Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Package review PR #1

Open
wants to merge 84 commits into
base: test
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
84 commits
Select commit Hold shift + click to select a range
5237a10
Initial commit
KarthikSubbarao Apr 26, 2024
a96c08a
Add support for BF.ADD, BF.EXISTS, BF.CARD
KarthikSubbarao Apr 26, 2024
be97a25
Update README
KarthikSubbarao Apr 26, 2024
5ebc1cb
Optimize bloom operations to use a reference to already created Bloom…
KarthikSubbarao Apr 27, 2024
8557b55
Remove older Bloom APIs using prev serialization apporach
KarthikSubbarao Apr 27, 2024
1729520
Add Dev Profile in Cargo.toml for debugging
KarthikSubbarao Apr 27, 2024
be80d9d
Create Module Datatype and support RDB load, save, free
KarthikSubbarao Apr 27, 2024
ed4d0f9
Update README.md
KarthikSubbarao Apr 27, 2024
2db0e4b
Add support for BF.RESERVE and expansion config
KarthikSubbarao Apr 27, 2024
778c219
Add support for BF.INFO
KarthikSubbarao Apr 27, 2024
b11970e
Add support for BF.MEXISTS
KarthikSubbarao Apr 27, 2024
bdf5f0d
Add support for BF.MADD
KarthikSubbarao Apr 27, 2024
8f5e3fc
Update error handling / messages and update expansion logic
KarthikSubbarao Apr 28, 2024
5545680
Add auto scaling support for bloom filters
KarthikSubbarao Apr 28, 2024
42d015c
Fix RDB Save for scaled filters
KarthikSubbarao Apr 28, 2024
481ecac
Refactoring
KarthikSubbarao Apr 28, 2024
ff60817
Update TODOs
KarthikSubbarao Apr 28, 2024
1c1b6ab
Fix mem_usage calculation
KarthikSubbarao Apr 28, 2024
d00fa99
Update Cargo.toml
KarthikSubbarao Apr 28, 2024
053d5e9
minor refactoring
KarthikSubbarao Apr 28, 2024
fbdfb5b
Add support for BF.INSERT and fix multi add logic
KarthikSubbarao Apr 29, 2024
a2343b7
Update error messages
KarthikSubbarao Apr 29, 2024
753575a
Type conversions + struct updates to use less memory
KarthikSubbarao Apr 29, 2024
3f85cae
fixed few clippy warnings
KarthikSubbarao Apr 29, 2024
159c938
fix clippy warnings
KarthikSubbarao Apr 29, 2024
bebf72a
Update README and address all clippy warnings
KarthikSubbarao Apr 30, 2024
ef4e215
Specify package license
KarthikSubbarao Apr 30, 2024
6ca68c5
Cargo fmt
KarthikSubbarao Apr 30, 2024
b7504e9
Add support for COPY commands on BloomFilter datatypes
KarthikSubbarao May 1, 2024
3ebb15b
Add more documentation
KarthikSubbarao Jul 2, 2024
ec72262
Migrate to valkeymodule-rs
KarthikSubbarao Jul 2, 2024
2816474
Implement free_effort datatype callback, Make aux_load cb ignore invo…
KarthikSubbarao Jul 16, 2024
3c285d5
Defrag Bloom Object callback
KarthikSubbarao Jul 17, 2024
ffb722f
Minor Refactoring + Handle Non Scaling Filters which are filled to ca…
KarthikSubbarao Aug 21, 2024
5b0df15
Replication support + Update Module/Datatype name + Refactor
KarthikSubbarao Aug 23, 2024
c66243d
Update data type name and use static str for errors
KarthikSubbarao Aug 23, 2024
0d2ad8a
Support keyspace notifications for write operations
KarthikSubbarao Aug 26, 2024
85e20fd
Add Python testing framework to support Integration testing of the bl…
KarthikSubbarao Sep 10, 2024
0b9a90d
Types, Ranges, limit updates and overflow handling
KarthikSubbarao Sep 13, 2024
a180f42
Add unit testing support using the valkeymodule-rs enable-system-allo…
KarthikSubbarao Sep 16, 2024
736a48a
Add unit testing for scaling & non scaling filters for behavior and f…
KarthikSubbarao Sep 17, 2024
6d0a4b6
Merge pull request #3 from KarthikSubbarao/unstable
KarthikSubbarao Sep 18, 2024
dee8a7e
Adding github workflow for building, running format checks, unit test…
zackcam Sep 19, 2024
145f9b2
Merge pull request #5 from zackcam/unstable
KarthikSubbarao Sep 19, 2024
282599d
RDB format optimization: Using a fixed seed for bloom filters (#2)
YueTang-Vanessa Sep 19, 2024
687bec7
Update build.sh and fix import in save/restore pytest
KarthikSubbarao Sep 19, 2024
f299216
Adding replication ability to valkey test case, changing waiter funct…
zackcam Oct 2, 2024
4d38649
Integration tests for commands, delete operations, expiry, compatibil…
YueTang-Vanessa Oct 9, 2024
fde4d37
Updating _ to -in lib.rs. Also updating loading from rdb to use a tra…
zackcam Oct 10, 2024
f5aa39e
Add Integration Testing for correctness of scaling and non scaling fi…
KarthikSubbarao Oct 15, 2024
57a7901
Add integration tests for maxmemory scenarios and replication correct…
KarthikSubbarao Oct 17, 2024
12031b2
Handle bloom object max allowed size limit, switch fp rate to f64 (#18)
KarthikSubbarao Oct 29, 2024
31d21c0
Adding info handler, that contains three fields, num_objects, num_fil…
zackcam Nov 5, 2024
b301baa
add bf.load to support bloom filter aofrewrite. (#17)
wuranxx Nov 15, 2024
a33e0e3
Add new metrics to show capacity and items across objects (#20)
YueTang-Vanessa Nov 19, 2024
7c25468
Support for DEBUG DIGEST module data type callback (#21)
nnmehta Nov 30, 2024
34d6fb0
Updating how we create BloomFilter from rdb loads and upgrading bloom…
zackcam Dec 4, 2024
1b0d819
Support random seed per bloom object by default (configurable) (#27)
KarthikSubbarao Dec 5, 2024
f10a4e5
Add tightening_ratio support in BloomFilterType structure (#26)
nnmehta Dec 6, 2024
eaa218f
Adding a wrong type error message when performing a bloom operation o…
zackcam Dec 6, 2024
d8a0508
Updating defragmentation to defrag both bloomfiltertype and bloomfile…
zackcam Dec 10, 2024
a5ec8de
Adding bloom acl category to commands. Added tests to check that the …
zackcam Dec 10, 2024
4a8af80
Update default configuration and related test (#28)
YueTang-Vanessa Dec 11, 2024
8b94c17
Deterministic replication - bloom object creation should have the sam…
KarthikSubbarao Dec 12, 2024
3589d30
Adding bloom defrag hits and misses to info, added check these metric…
zackcam Dec 13, 2024
98ccdc6
Deflaking tests to remove chance of false positives causing tests to …
zackcam Dec 17, 2024
9baf6fc
Add TIGHTENING Ratio and FP_Rate configs support (#31)
nnmehta Dec 19, 2024
6a514bf
Fix size check and update build script to fail on warnings
KarthikSubbarao Dec 19, 2024
1c949fb
Add test for loading rdb onto non-bloom cluster (#36)
YueTang-Vanessa Jan 3, 2025
ab158bb
Large bloom object handling + Rename to BloomFilterType to BloomObjec…
KarthikSubbarao Jan 11, 2025
a9b578e
ACL Updates, BF.INSERT updates, other misc updates (#38)
KarthikSubbarao Jan 12, 2025
1bb7d25
Update README.md (#39)
KarthikSubbarao Jan 12, 2025
591ab10
Adding a bloom command keyspace test (#40)
zackcam Jan 17, 2025
20efd95
Add dump and restore test (#42)
nnmehta Jan 27, 2025
14ad3d0
Adding optional arg to BF.INSERT to allow users to check if their blo…
zackcam Jan 30, 2025
56f631f
Fixing capacity calcualtion and adding a unit test to make sure capac…
zackcam Jan 30, 2025
8077d58
Code cleanup: removing unused dependencies in tests, removing digest …
zackcam Feb 3, 2025
8f1c9f4
Add License file following Valkey (#46)
KarthikSubbarao Feb 14, 2025
1e6c23b
Add server version validation during module loading (#48)
YueTang-Vanessa Mar 10, 2025
e94a2e7
Making it so github workflows can see if we have memory leaks as well…
zackcam Mar 10, 2025
12fa935
Adding usage of must_obey_client in wrapper to improve performace (#45)
zackcam Mar 11, 2025
e308fed
Adding tightening ratio to bf.info. Making BF.BLOOM-EXPANSION also (#50)
zackcam Mar 12, 2025
d23399a
Update development script's tests use module extensions from differen…
KarthikSubbarao Mar 21, 2025
60513c7
Removing unnecessary code form valkey_test_case (#51)
zackcam Mar 27, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 131 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
name: ci
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I am a new developer looking at this file, how do I understand the intent of this file without documentation? Are these configurations obvious? What documentation did you read to write this file?


on:
push:
pull_request:

env:
CARGO_TERM_COLOR: always
VALKEY_REPO_URL: https://github.com/valkey-io/valkey.git

jobs:
build-ubuntu-latest:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
server_version: ['unstable', '8.0.0']
steps:
- uses: actions/checkout@v4
- name: Set the server verison for python integeration tests
run: echo "SERVER_VERSION=${{ matrix.server_version }}" >> $GITHUB_ENV
- name: Run cargo and clippy format check
run: |
cargo fmt --check
cargo clippy --profile release --all-targets -- -D clippy::all
- name: Release Build
run: |
if [ "${{ matrix.server_version }}" = "8.0.0" ]; then
RUSTFLAGS="-D warnings" cargo build --all --all-targets --release --features valkey_8_0
else
RUSTFLAGS="-D warnings" cargo build --all --all-targets --release
fi
- name: Run unit tests
run: cargo test --features enable-system-alloc
- name: Make valkey-server binary
run: |
mkdir -p "tests/.build/binaries/${{ matrix.server_version }}"
cd tests/.build
git clone "${{ env.VALKEY_REPO_URL }}"
cd valkey
git checkout ${{ matrix.server_version }}
make -j
cp src/valkey-server ../binaries/${{ matrix.server_version }}/
- name: Set up Python
uses: actions/setup-python@v3
with:
python-version: '3.8'
- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install -r requirements.txt
- name: Update module path
run: echo "MODULE_PATH=$(realpath target/release/libvalkey_bloom.so)" >> $GITHUB_ENV
- name: Run integration tests
run: python -m pytest --cache-clear -v "tests/"

build-macos-latest:
runs-on: macos-latest
steps:
- uses: actions/checkout@v4
- name: Run cargo and clippy format check
run: |
cargo fmt --check
cargo clippy --profile release --all-targets -- -D clippy::all
- name: Release Build
run: RUSTFLAGS="-D warnings" cargo build --all --all-targets --release
- name: Run unit tests
run: cargo test --features enable-system-alloc

asan-build:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
server_version: ['unstable', '8.0.0']
steps:
- uses: actions/checkout@v4
- name: Set the server verison for python integeration tests
run: echo "SERVER_VERSION=${{ matrix.server_version }}" >> $GITHUB_ENV
- name: Run cargo and clippy format check
run: |
cargo fmt --check
cargo clippy --profile release --all-targets -- -D clippy::all
- name: Release Build
run: |
if [ "${{ matrix.server_version }}" = "8.0.0" ]; then
RUSTFLAGS="-D warnings" cargo build --all --all-targets --release --features valkey_8_0
else
RUSTFLAGS="-D warnings" cargo build --all --all-targets --release
fi
- name: Run unit tests
run: cargo test --features enable-system-alloc
- name: Make Valkey-server binary with asan
run: |
mkdir -p "tests/.build/binaries/${{ matrix.server_version }}"
cd tests/.build
git clone "${{ env.VALKEY_REPO_URL }}"
cd valkey
git checkout ${{ matrix.server_version }}
make distclean
make -j SANITIZER=address SERVER_CFLAGS='-Werror' BUILD_TLS=module
cp src/valkey-server ../binaries/${{ matrix.server_version }}/
- name: Set up Python
uses: actions/setup-python@v3
with:
python-version: '3.8'
- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install -r requirements.txt
- name: Update module path
run: echo "MODULE_PATH=$(realpath target/release/libvalkey_bloom.so)" >> $GITHUB_ENV
- name: Run integration tests
run: |
python -m pytest --capture=sys --cache-clear -v "tests/" -m "not skip_for_asan" 2>&1 | tee test_output.tmp

if grep -q "LeakSanitizer: detected memory leaks" test_output.tmp; then
RED='\033[0;31m'
echo -e "${RED}Memory leaks detected in the following tests:"
LEAKING_TESTS=$(grep -B 2 "LeakSanitizer: detected memory leaks" test_output.tmp | \
grep -v "LeakSanitizer" | \
grep ".*\.py::")
LEAK_COUNT=$(echo "$LEAKING_TESTS" | wc -l)
echo "$LEAKING_TESTS" | while read -r line; do
echo "::error::Test with leak: $line"
done
echo -e "\n$LEAK_COUNT python integration tests have leaks detected in them"
rm test_output.tmp
exit 1
fi
rm test_output.tmp
6 changes: 6 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
Cargo.lock
target
tests/.build
__pycache__
test-data
.attach_pid*
14 changes: 14 additions & 0 deletions 00-RELEASENOTES
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
Hello! This file is just a placeholder, since this is the "unstable" branch
of Valkey-Bloom, the place where all the development happens.

There is no release notes for this branch, it gets forked into another branch
every time there is a partial feature freeze in order to eventually create
a new stable release.

Usually "unstable" is stable enough for you to use it in development environments
however you should never use it in production environments. You can find the
latest stable release here:

https://github.com/valkey-io/valkey-bloom/releases

Happy hacking!
42 changes: 42 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
[package]
name = "valkey-bloom"
authors = ["Karthik Subbarao"]
version = "0.1.0-dev"
edition = "2021"
license = "BSD-3-Clause"
repository = "https://github.com/valkey-io/valkey-bloom"
readme = "README.md"
description = "A bloom filter module for Valkey"
homepage = "https://github.com/valkey-io/valkey-bloom"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
valkey-module = { version = "0.1.5", features = ["min-valkey-compatibility-version-8-0", "min-redis-compatibility-version-7-2"]}
valkey-module-macros = "0"
linkme = "0"
bloomfilter = { version = "3.0.1", features = ["serde"] }
lazy_static = "1.4.0"
libc = "0.2"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need libc?
Why do we not depend on "valkey-module-macros" package? (valkey-module-rs github repo)

serde = { version = "1.0", features = ["derive"] }
bincode = "1.3"

[dev-dependencies]
rand = "0.8"
rstest = "0.23.0"

[lib]
crate-type = ["cdylib"]
name = "valkey_bloom"

[profile.dev]
opt-level = 0
debug = 2
debug-assertions = true

[features]
default = ["min-valkey-compatibility-version-8-0"]
enable-system-alloc = ["valkey-module/enable-system-alloc"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the intention with this configuration?

min-valkey-compatibility-version-8-0 = []
valkey_8_0 = [] # Valkey-bloom is intended to be loaded on server versions >= Valkey 8.1 and by default it is built this way (unless this flag is provided). It is however compatible with Valkey version 8.0 if the user explicitly provides this feature flag in their cargo build command.
use-redismodule-api = [] # We don't support this feature flag which is why it is empty.
12 changes: 12 additions & 0 deletions LICENSE
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
BSD 3-Clause License

Copyright (c) 2024-present, Valkey contributors
All rights reserved.

Redistribution and use in source and binary forms, with or without modification, are permitted provided that the following conditions are met:

* Redistributions of source code must retain the above copyright notice, this list of conditions and the following disclaimer.
* Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the following disclaimer in the documentation and/or other materials provided with the distribution.
* Neither the name of the copyright holder nor the names of its contributors may be used to endorse or promote products derived from this software without specific prior written permission.

THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
75 changes: 74 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1 +1,74 @@
# valkey-bloom
# valkey-bloom

Valkey-Bloom (BSD-3-Clause) is a Rust Valkey-Module which brings a native and space efficient probabilistic Module data type to Valkey. With this, users can create filters (space-efficient probabilistic Module data type) to add elements, perform “check” operation to test whether an element exists, auto scale their filters, perform RDB Save and load operations, etc.

Valkey-Bloom is built using `bloomfilter::Bloom` (https://crates.io/crates/bloomfilter which has a BSD-2-Clause license).

It is compatible with the BloomFilter (BF.*) command APIs in Redis offerings.

## Supported commands
```
BF.EXISTS
BF.ADD
BF.MEXISTS
BF.MADD
BF.CARD
BF.RESERVE
BF.INFO
BF.INSERT
BF.LOAD
```

## Build instructions
```
curl https://sh.rustup.rs -sSf | sh
sudo yum install clang
git clone https://github.com/valkey-io/valkey-bloom.git
cd valkey-bloom
cargo build --all --all-targets --release
valkey-server --loadmodule ./target/release/libvalkey_bloom.so
```

#### Local development script to build, run format checks, run unit / integration tests, and for cargo release:
```
# Builds the valkey-server (unstable) for integration testing.
SERVER_VERSION=unstable
./build.sh
# Same as above, but uses valkey-server (8.0.0) for integration testing.
SERVER_VERSION=8.0.0
./build.sh
# Build with asan, you may need to remove the old valkey binary if you have used ./build.sh before. You can do this by deleting the `.build` folder in the `tests` folder
ASAN_BUILD=true
./build.sh
```

## Load the Module
To test the module with a Valkey, you can load the module in the following ways:

#### Using valkey.conf:
```
1. Add the following to valkey.conf:
loadmodule /path/to/libvalkey_bloom.so
2. Start valkey-server:
valkey-server /path/to/valkey.conf
```

#### Starting Valkey with the `--loadmodule` option:
```text
valkey-server --loadmodule /path/to/libvalkey_bloom.so
```

#### Using the Valkey command `MODULE LOAD`:
```
1. Connect to a running Valkey instance using valkey-cli
2. Execute Valkey command:
MODULE LOAD /path/to/libvalkey_bloom.so
```
## Feature Flags

* valkey_8_0: valkey-bloom is intended to be loaded on server versions >= Valkey 8.1 and by default it is built this way (unless this flag is provided). It is however compatible with Valkey version 8.0 if the user explicitly provides this feature flag in their cargo build command.
```
cargo build --release --features valkey_8_0
```

This can also be done by specifiyng SERVER_VERSION=8.0.0 and then running `./build.sh`
Loading