feat(cms): add Count-Min Sketch data structure by Tangruilin · Pull Request #3405 · apache/kvrocks

Tangruilin · 2026-03-29T11:05:33Z

Implement CMS (Count-Min Sketch) probabilistic data structure with the following commands:

CMS.INITBYDIM key width depth
CMS.INITBYPROB key error probability
CMS.INCRBY key item increment [...]
CMS.QUERY key item [...]
CMS.MERGE dest numkeys src... [WEIGHTS ...]
CMS.INFO key

Storage design:

Metadata stored in Metadata CF
Count Matrix stored in PrimarySubkey CF (per-bucket storage)
Uses MurmurHash64 for layer-specific hashing

Related: #2425

Tangruilin · 2026-03-29T14:20:37Z

I noticed that operations like ZINCRBY (ZSet) and HINCRBY (Hash) don't use any locks, even though they are read-modify-write operations. From my testing with concurrent unit tests, I
haven't observed any race conditions.

Questions about Kvrocks threading model:

Is Kvrocks single-threaded when executing commands?
How does Kvrocks ensure thread safety for read-modify-write operations like HINCRBY and ZINCRBY without explicit locking?
Do we need locks for CMS operations (INCRBY, MERGE) similar to how TDigest uses LockGuard?

Current locking status across types:

| Type | INCRBY has lock | Notes |
|------|-----------------|------ -|
| Hash | ❌ | HINCRBY - read-modify-write |
| ZSet | ❌ | ZINCRBY - read-modify-write + dual index update |
| TDigest | ✅ | Uses LockGuard |
| CMS (this PR) | ❌ | Currently no lock |

I've included concurrent tests for CMS in tests/gocase/unit/type/cms/cms_test.go to verify correctness. Would appreciate clarification on whether locks are needed for CMS operations.

Tangruilin · 2026-03-30T03:28:32Z

AI was used in this patch for specific code implementation and approach analysis. The main design was evaluated by humans, discussion refer:#3404

Tangruilin · 2026-03-30T03:29:09Z

tests/cppunit/types/cms_test.cc

+  }
+
+  void TearDown() override {
+    TestBase::SetUp();


error. should be TearDown

Tangruilin · 2026-03-30T03:45:27Z

src/types/redis_cms.cc

+
+/// CMS.INCRBY - Increment counters for given items
+/// Usage: CMS.INCRBY key item increment [item increment ...]
+rocksdb::Status CMS::IncrBy(engine::Context &ctx, const Slice &key,


I assessed that a lock is needed here. However, I ran concurrent tests and also reviewed Kvrocks' HSET, HASH and other modules. I found that their write operations don't use locks. Is this by design? Is concurrency handled at some other level?

any suggestion？ @PragmaTwice @git-hulk

@Tangruilin Thanks for your PR. I will take a look at your proposal first, you could read Guidelines for AI-assisted Contributions.

I have alread read it before do this.

and I have a proposal already: #3404

Ths for look

Hi @git-hulk,

Thank you for taking the time to review this PR.

For this patch, you could focus on reviewing the proposal-related parts first. I'll address any feedback you have on the design and implementation approach.

Regarding the concurrency concern I mentioned earlier — I noticed that commands like HSET, HINCRBY, and ZINCRBY in Kvrocks don't use explicit locks, even though they involve
read-modify-write operations. Is this by design? Is thread safety handled at a different level (e.g., command execution model)?

Tangruilin · 2026-03-30T05:45:42Z

src/types/redis_cms.cc

+  // Formula from Count-Min Sketch paper:
+  // width = ceil(e / error_rate)
+  // depth = ceil(ln(1 - probability) / ln(1 - 1/e))
+  auto width = static_cast<uint32_t>(std::ceil(2.718281828 / error_rate));


use std::numbers::e

Tangruilin · 2026-03-30T05:58:12Z

src/types/redis_cms.cc

+  return static_cast<uint32_t>(hash % width);
+}
+
+std::pair<uint32_t, uint32_t> CMS::calcDimFromProb(double error_rate, double probability) {


probability use success probability here.
should be fail_probability

refer to: https://redis.io/docs/latest/commands/cms.initbyprob/

Tangruilin · 2026-03-30T06:02:07Z

src/types/redis_cms.cc

+  // Formula from Count-Min Sketch paper:
+  // width = ceil(e / error_rate)
+  // depth = ceil(ln(1 - probability) / ln(1 - 1/e))
+  auto width = static_cast<uint32_t>(std::ceil(2.718281828 / error_rate));


RedisBloom uses ceil(2 / error) for the width calculation. I believe this is because C doesn't have a built-in constant for e (Euler's number). In our C++ implementation, using e would
be more mathematically accurate according to the original CMS paper.

Should we follow the paper's formula with e, or should we fully match Redis's implementation for compatibility?

@PragmaTwice

Implement CMS (Count-Min Sketch) probabilistic data structure with the following commands: - CMS.INITBYDIM key width depth - CMS.INITBYPROB key error probability - CMS.INCRBY key item increment [...] - CMS.QUERY key item [...] - CMS.MERGE dest numkeys src... [WEIGHTS ...] - CMS.INFO key Storage design: - Metadata stored in Metadata CF - Count Matrix stored in PrimarySubkey CF (per-bucket storage) - Uses MurmurHash64 for layer-specific hashing Related: apache#2425

jihuayu · 2026-03-31T07:06:36Z

Hi @Tangruilin
This PR is a bit too massive. Could you implement only the minimum viable features in the first PR?
Please make sure the clang-tidy and format checks pass.

Kvrocks' write commands have key-level locks at the framework level.

Tangruilin · 2026-03-31T12:59:50Z

Hi @Tangruilin This PR is a bit too massive. Could you implement only the minimum viable features in the first PR? Please make sure the clang-tidy and format checks pass.

Kvrocks' write commands have key-level locks at the framework level.

Thanks for the review! I'll split this PR. The first PR will focus on the basic CMS and INIT commands only.

I've marked this PR as draft for now. Once the split is complete, I'll change it back to ready for review.

In the meantime, you could review the proposal first: #3404

Tangruilin marked this pull request as draft March 29, 2026 11:08

Tangruilin force-pushed the feature#2425 branch 2 times, most recently from ce091bf to 71b0ba8 Compare March 29, 2026 12:24

Tangruilin marked this pull request as ready for review March 29, 2026 12:25

Tangruilin force-pushed the feature#2425 branch from 71b0ba8 to a80940f Compare March 29, 2026 12:34

Tangruilin force-pushed the feature#2425 branch 2 times, most recently from 6808595 to 4b0fa17 Compare March 29, 2026 15:21

Tangruilin commented Mar 30, 2026

View reviewed changes

Tangruilin force-pushed the feature#2425 branch from 4b0fa17 to c11c84e Compare March 30, 2026 15:10

Tangruilin marked this pull request as draft March 31, 2026 12:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(cms): add Count-Min Sketch data structure#3405

feat(cms): add Count-Min Sketch data structure#3405
Tangruilin wants to merge 1 commit intoapache:unstablefrom
Tangruilin:feature#2425

Tangruilin commented Mar 29, 2026

Uh oh!

Tangruilin commented Mar 29, 2026

Uh oh!

Tangruilin commented Mar 30, 2026

Uh oh!

Tangruilin Mar 30, 2026

Uh oh!

Tangruilin Mar 30, 2026 •

edited

Loading

Uh oh!

git-hulk Mar 30, 2026

Uh oh!

Tangruilin Mar 30, 2026

Uh oh!

Tangruilin Mar 30, 2026

Uh oh!

Tangruilin Mar 30, 2026

Uh oh!

Tangruilin Mar 30, 2026

Uh oh!

Tangruilin Mar 30, 2026

Uh oh!

jihuayu commented Mar 31, 2026

Uh oh!

Tangruilin commented Mar 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

Tangruilin commented Mar 29, 2026

Uh oh!

Tangruilin commented Mar 29, 2026

Uh oh!

Tangruilin commented Mar 30, 2026

Uh oh!

Tangruilin Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Tangruilin Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

git-hulk Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Tangruilin Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Tangruilin Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Tangruilin Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Tangruilin Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Tangruilin Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

jihuayu commented Mar 31, 2026

Uh oh!

Tangruilin commented Mar 31, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Tangruilin Mar 30, 2026 •

edited

Loading