From fcaad772f2865b19a79e8a80b5b7c0a78ade5849 Mon Sep 17 00:00:00 2001 From: KarthikSubbarao Date: Thu, 19 Dec 2024 03:15:38 -0800 Subject: [PATCH] Fix size check and update build script to fail on warnings (#35) Signed-off-by: Karthik Subbarao --- .github/workflows/ci.yml | 6 +++--- build.sh | 2 +- src/bloom/command_handler.rs | 2 +- src/bloom/utils.rs | 11 ++++++++--- src/configs.rs | 7 ++----- src/lib.rs | 4 ++-- 6 files changed, 17 insertions(+), 15 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index da72f7a..bb5cea8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -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 @@ -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 @@ -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 diff --git a/build.sh b/build.sh index d8d6781..f250c9d 100755 --- a/build.sh +++ b/build.sh @@ -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 diff --git a/src/bloom/command_handler.rs b/src/bloom/command_handler.rs index e12a40f..19a0fa4 100644 --- a/src/bloom/command_handler.rs +++ b/src/bloom/command_handler.rs @@ -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, diff --git a/src/bloom/utils.rs b/src/bloom/utils.rs index f64c553..9971a14 100644 --- a/src/bloom/utils.rs +++ b/src/bloom/utils.rs @@ -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::(); + 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::() + + std::mem::size_of::>() + + bloomfilter::Bloom::<[u8]>::compute_bitmap_size(capacity as usize, fp_rate) + } + pub fn check(&self, item: &[u8]) -> bool { self.bloom.check(item) } diff --git a/src/configs.rs b/src/configs.rs index 12764b7..6936fff 100644 --- a/src/configs.rs +++ b/src/configs.rs @@ -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 @@ -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) { diff --git a/src/lib.rs b/src/lib.rs index c9b2b50..cf5eb64 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -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;