Skip to content

CMS : Merge Command - #120

Open
zmccoy wants to merge 7 commits into
valkey-io:count-min-sketchfrom
zmccoy:count-min-sketch-merge-picked
Open

zmccoy wants to merge 7 commits into
valkey-io:count-min-sketchfrom
zmccoy:count-min-sketch-merge-picked

Conversation

@zmccoy

@zmccoy zmccoy commented Aug 31, 2026

Copy link
Copy Markdown

Cherry picked from the older branches that were in a rebase stack, this adds the merge command and testing around it for the Count Min Sketch. This is the last command we were missing.

Questions:

  1. I'm a bit unclear on replication, and it seems like I'd not want to do verbatim, but am curious as to how to think about that.

Notes:

  1. We are still missing the replication testing as a whole, I'm happy to add that here or in another PR after this is done and fix up from there.

…se branch

Signed-off-by: Zach McCoy <zmccoy@jackhenry.com>
Comment thread src/cms/cms_command_handler.rs Outdated
zmccoy added 2 commits August 31, 2026 12:08
Signed-off-by: Zach McCoy <zmccoy@jackhenry.com>
Signed-off-by: Zach McCoy <zmccoy@jackhenry.com>
@zmccoy zmccoy changed the title Count Min Sketch : Merge Command CMS : Merge Command Aug 31, 2026
zmccoy added 2 commits August 31, 2026 12:30
Signed-off-by: Zach McCoy <zmccoy@jackhenry.com>
Signed-off-by: Zach McCoy <zmccoy@jackhenry.com>
@zackcam

zackcam commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

I owuld think verbatim replication is what we want as consistency is key across replica and primaries. Was there a place where you saw this wasn't the case?

@zmccoy

zmccoy commented Sep 9, 2026

Copy link
Copy Markdown
Author

@zackcam Nothing specific to why it wouldn't work. I think I was pattern matching off the previously done INITBYDIM and INITBYPROB which were not verbatim and followed a similar structure of not having to omit anything.

For my own edification, would INITBYDIM and INITBYPROB also be able to be verbatim? For example: https://github.com/valkey-io/valkey-bloom/blob/count-min-sketch/src/cms/cms_command_handler.rs#L46-L47 ends up recreating the CMS.INITBYPROB key error probability

Signed-off-by: Zach McCoy <zmccoy@jackhenry.com>
Signed-off-by: Zach McCoy <zmccoy@jackhenry.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants