Skip to content

Commit

Permalink
Fix size check and update build script to fail on warnings (#35)
Browse files Browse the repository at this point in the history
Signed-off-by: Karthik Subbarao <karthikrs2021@gmail.com>
  • Loading branch information
KarthikSubbarao authored Dec 19, 2024
1 parent 9baf6fc commit fcaad77
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 15 deletions.
6 changes: 3 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ jobs:
cargo fmt --check
cargo clippy --profile release --all-targets -- -D clippy::all
- name: Release Build
run: cargo build --all --all-targets --release
run: RUSTFLAGS="-D warnings" cargo build --all --all-targets --release
- name: Run unit tests
run: cargo test --features enable-system-alloc
- name: Make valkey-server binary
Expand Down Expand Up @@ -58,7 +58,7 @@ jobs:
cargo fmt --check
cargo clippy --profile release --all-targets -- -D clippy::all
- name: Release Build
run: cargo build --all --all-targets --release
run: RUSTFLAGS="-D warnings" cargo build --all --all-targets --release
- name: Run unit tests
run: cargo test --features enable-system-alloc

Expand All @@ -77,7 +77,7 @@ jobs:
cargo fmt --check
cargo clippy --profile release --all-targets -- -D clippy::all
- name: Release Build
run: cargo build --all --all-targets --release
run: RUSTFLAGS="-D warnings" cargo build --all --all-targets --release
- name: Run unit tests
run: cargo test --features enable-system-alloc
- name: Make Valkey-server binary with asan
Expand Down
2 changes: 1 addition & 1 deletion build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ cargo fmt --check
cargo clippy --profile release --all-targets -- -D clippy::all

echo "Running cargo build release..."
cargo build --all --all-targets --release
RUSTFLAGS="-D warnings" cargo build --all --all-targets --release

echo "Running unit tests..."
cargo test --features enable-system-alloc
Expand Down
2 changes: 1 addition & 1 deletion src/bloom/command_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -407,7 +407,7 @@ pub fn bloom_filter_reserve(ctx: &Context, input_args: &[ValkeyString]) -> Valke
let validate_size_limit = !ctx.get_flags().contains(ContextFlags::REPLICATED);
let tightening_ratio = *configs::BLOOM_TIGHTENING_F64
.lock()
.expect("Failed to lock tightening ratio");
.expect("Unable to get a lock on tightening ratio static");
let bloom = match BloomFilterType::new_reserved(
fp_rate,
tightening_ratio,
Expand Down
11 changes: 8 additions & 3 deletions src/bloom/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -543,18 +543,23 @@ impl BloomFilter {
+ (self.bloom.len() / 8) as usize
}

/// Caculates the number of bytes that the bloom filter will require to be allocated.
/// This is used before actually creating the bloom filter when checking if the filter is within the allowed size.
/// Returns whether the bloom filter is of a valid size or not.
pub fn validate_size(capacity: i64, fp_rate: f64) -> bool {
let bytes = bloomfilter::Bloom::<[u8]>::compute_bitmap_size(capacity as usize, fp_rate)
+ std::mem::size_of::<BloomFilter>();
let bytes = Self::compute_size(capacity, fp_rate);
if bytes > configs::BLOOM_MEMORY_LIMIT_PER_FILTER.load(Ordering::Relaxed) as usize {
return false;
}
true
}

/// Caculates the number of bytes that the bloom filter will require to be allocated.
fn compute_size(capacity: i64, fp_rate: f64) -> usize {
std::mem::size_of::<BloomFilter>()
+ std::mem::size_of::<bloomfilter::Bloom<[u8]>>()
+ bloomfilter::Bloom::<[u8]>::compute_bitmap_size(capacity as usize, fp_rate)
}

pub fn check(&self, item: &[u8]) -> bool {
self.bloom.check(item)
}
Expand Down
7 changes: 2 additions & 5 deletions src/configs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,9 @@ use crate::bloom::utils;
use lazy_static::lazy_static;
use std::sync::atomic::{AtomicBool, AtomicI64};
use std::sync::Mutex;
use valkey_module::logging;
use valkey_module::{
configuration::{ConfigurationContext, ConfigurationFlags},
valkey_module, ConfigurationValue, Context, InfoContext, Status, ValkeyError, ValkeyGILGuard,
ValkeyResult, ValkeyString,
configuration::ConfigurationContext, ConfigurationValue, ValkeyError, ValkeyGILGuard,
ValkeyString,
};

/// Configurations
Expand Down Expand Up @@ -85,7 +83,6 @@ pub fn on_string_config_set(
return Err(ValkeyError::Str("Invalid floating-point value"));
}
};

match name {
"bloom-fp-rate" => {
if !(BLOOM_FP_RATE_MIN..BLOOM_FP_RATE_MAX).contains(&value) {
Expand Down
4 changes: 2 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use metrics::bloom_info_handler;
use valkey_module::{
configuration::ConfigurationFlags, valkey_module, Context, InfoContext, Status, ValkeyGILGuard,
ValkeyResult, ValkeyString,
configuration::ConfigurationFlags, valkey_module, Context, InfoContext, Status, ValkeyResult,
ValkeyString,
};
pub mod bloom;
pub mod configs;
Expand Down

0 comments on commit fcaad77

Please sign in to comment.