Add Cuckoo Filter (CF.*) support#3774
Conversation
Adds client support for the RedisBloom Cuckoo Filter commands across the synchronous, asynchronous, reactive, cluster and Kotlin coroutine APIs, following the same module pattern as Bloom Filter (BF.*). Commands: CF.RESERVE, CF.ADD, CF.ADDNX, CF.INSERT, CF.INSERTNX, CF.EXISTS, CF.MEXISTS, CF.DEL, CF.COUNT, CF.SCANDUMP, CF.LOADCHUNK, CF.INFO Read-only commands (CF.EXISTS, CF.MEXISTS, CF.COUNT, CF.INFO, CF.SCANDUMP) are registered for cluster read routing. CF.INSERT/CF.INSERTNX map the per-item -1 (filter full) result to null via ErrorTolerantBooleanListOutput, consistent with BF.INSERT. Resolves redis#2659, redis#2660, redis#2661, redis#2662, redis#2663, redis#2664, redis#2665, redis#2666, redis#2667, redis#2668, redis#2669, redis#2670
|
Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset. In case there are security findings, they will be communicated to you as a comment inside the PR. Hope you’ll enjoy using Jit. Questions? Comments? Want to learn more? Get in touch with us. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Reviewed by Cursor Bugbot for commit 7896a0b. Configure here.
ErrorTolerantBooleanListOutput mapped any non-1 integer to false, conflating
the per-item -1 (filter full) returned by CF.INSERT/CF.INSERTNX with 0
("already exists"). Add CF-specific outputs mapping 1->true, 0->false,
-1->null (reactive: Value.empty()), keeping the three states distinct.
Verified end-to-end against redis-stack (filter-full case asserts null /
Value.empty()).
The RedisBloom server returns the per-item -1 (filter full) as an integer only over RESP2; over RESP3 it returns boolean false instead, so the -1 vs 0 distinction is not observable on the client. Move the -1 -> null integration assertion into RedisCuckooFilterResp2IntegrationTests and drop the filter-full case from the RESP3 sync/reactive tests. The output mapping is independently covered by CuckooInsertBooleanListOutput/ValueListOutput unit tests.
- cfInsert/cfInsertNx Javadoc now document the per-item null (RESP2) / Value.empty() (reactive) result for a full filter, and drop the incorrect "false = already exists" note from cfInsert (CF.INSERT never returns 0). Fix the broken reactive @return text, add cfAdd @throws and a RESP3 note. - Restore bfInsert/bfMAdd in CreateReactiveApi VALUE_WRAP; they were accidentally dropped while adding the CF entries. Without them an api-generator rerun would regenerate the Bloom reactive insert as Flux<Boolean> and break BF reactive signatures, wiring and tests.
|
Thank you for the contribution @HwangRock |
|
@a-TODO-rov |
…terfaces - Set @author to the contributor on the new Cuckoo Filter files. - Drop CF.SCANDUMP from ReadOnlyCommands to match BF.SCANDUMP, which is not registered there, and adjust the read-only command count test accordingly. - Declare RedisBloomFilterCoroutinesCommands and RedisCuckooFilterCoroutinesCommands on the RedisClusterCoroutinesCommands interface; they were previously only implemented in the impl class.
…NSERT CF.INSERT returns integers on RESP2 and booleans on RESP3, never null, so the Value wrapper wasn't needed. cfInsert now returns List<Boolean> (Flux<Boolean> reactive) with a full filter mapped to false. CF.INSERTNX stays an integer on both RESP2 and RESP3, so it now returns List<Long> (1 = added, 0 = already exists, -1 = filter full) instead of being squeezed into booleans, which keeps the three states distinct without a wrapper. Also adds the single-value overloads requested in review and removes the now-unused CuckooInsertBoolean* outputs.
Both are registered with the readonly flag in RedisBloom and don't mutate the filter, so they're safe to route to replicas. Adds both to ReadOnlyCommands (BF.SCANDUMP was missing) and updates the read-only count test to 123.
I think that's a great approach. Merging the two dump value classes into one will definitely clean up the code. Moving them into a probabilistic package also makes sense for future features like Top-K and T-Digest. |
I think it is fine to do both in this pull request (merging into one class and moving arguments and values to a single |
BfScanDumpValue and CfScanDumpValue were identical, so consolidate them into a single io.lettuce.core.probabilistic.ScanDumpValue (with ScanDumpValueParser) shared by both BF.SCANDUMP and CF.SCANDUMP.
Group the RedisBloom value, parser, and argument types that were spread across io.lettuce.core.bf and io.lettuce.core.cf into a shared io.lettuce.core.probabilistic package (arguments under io.lettuce.core.probabilistic.arguments), so the Bloom Filter and Cuckoo Filter features sit together for future probabilistic commands.
Relocate the Bloom Filter and Cuckoo Filter test classes from io.lettuce.core.bf and io.lettuce.core.cf into io.lettuce.core.probabilistic to match the production layout and keep the probabilistic features together.
Done. I merged the two dump value classes into a single ScanDumpValue, and moved the values, arguments, and tests from both bf and cf into a shared probabilistic package (arguments under probabilistic.arguments). |

Adds support for the RedisBloom Cuckoo Filter (CF.) commands, wired the same way as the existing Bloom Filter (BF.) support across the synchronous, asynchronous, reactive, cluster node-selection, and Kotlin coroutine APIs.
Commands: CF.RESERVE, CF.ADD, CF.ADDNX, CF.INSERT, CF.INSERTNX, CF.EXISTS, CF.MEXISTS, CF.DEL, CF.COUNT, CF.SCANDUMP, CF.LOADCHUNK, CF.INFO
CF.INSERT returns
List<Boolean>(true= added,false= filter full), and CF.INSERTNX returnsList<Long>(1= added,0= already exists,-1= filter full) since it stays an integer on both RESP2 and RESP3. CF.INSERT itself comes back as integers on RESP2 and booleans on RESP3, with a full filter mapping tofalse.CF.EXISTS, CF.MEXISTS, CF.COUNT, CF.INFO and CF.SCANDUMP are registered as read-only for cluster read routing (this also adds the missing BF.SCANDUMP). CF.INFO returns the field names
Number of filtersandMax iterations(plural), which differs from what the command reference currently shows.Resolves #2659, #2660, #2661, #2662, #2663, #2664, #2665, #2666, #2667, #2668, #2669, #2670
Testing
Unit tests:
RedisCuckooFilterCommandBuilderUnitTests— RESP encoding for the 12 commands, including the single-value overloadsCfInfoValueParserUnitTests— CF.INFO field parsingIntegration tests ran against redis-stack-server (Redis 7.4.7, RedisBloom
bfmodule) over the sync and reactive APIs, and again over RESP2:The filter-full case is asserted in the integration tests —
cfInsertReturnsFalseWhenFilterIsFullchecks the overflow items come back asfalse, and CF.INSERTNX results are asserted asList<Long>.Skipped: 0shows the@EnabledOnCommand("CF.ADD")guard ran against a CF-capable server. The cluster integration test mirrors the BF one but needs a running cluster, so it was not part of this run.Make sure that:
mvn formatter:formattarget. Don’t submit any formatting related changes.Note
Medium Risk
Large API surface addition plus package/type renames (
io.lettuce.core.bf→probabilistic,BfScanDumpValue→ScanDumpValue) are breaking for downstream imports; new command encoding and cluster read-only lists need correct routing behavior.Overview
Adds Redis Cuckoo Filter (
CF.*) support end-to-end (sync/async/reactive, cluster node-selection, Kotlin coroutines), mirroring the existing Bloom Filter wiring viaRedisCuckooFilterCommandBuilderand newcf*command surfaces.Bloom-related types move from
io.lettuce.core.bftoio.lettuce.core.probabilistic;BfScanDumpValueis renamed to sharedScanDumpValue(used by bothBF.SCANDUMPandCF.SCANDUMP). NewCfInfoValue,CfInsertArgs,CfReserveArgs, and protocol entries (CF_*commands,BUCKETSIZE/MAXITERATIONSkeywords).Cluster read-only routing gains
BF.SCANDUMPand the CF read commands (CF.EXISTS,CF.MEXISTS,CF.COUNT,CF.INFO,CF.SCANDUMP).Reviewed by Cursor Bugbot for commit 64735ac. Bugbot is set up for automated code reviews on this repo. Configure here.