Skip to content

chore: cms docs#516

Merged
kostasrim merged 2 commits intomainfrom
cms
Mar 20, 2026
Merged

chore: cms docs#516
kostasrim merged 2 commits intomainfrom
cms

Conversation

@kostasrim
Copy link
Contributor

add cms docs

resolves dragonflydb/dragonfly#5445

Signed-off-by: Kostas Kyrimis <kostas@dragonflydb.io>
@kostasrim kostasrim self-assigned this Mar 18, 2026
@vercel
Copy link

vercel bot commented Mar 18, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
documentation Ready Ready Preview, Comment Mar 20, 2026 8:04am

Request Review

Copy link
Contributor

@vyavdoshenko vyavdoshenko left a comment

Choose a reason for hiding this comment

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

  1. CMS.INCRBY - incorrect behavior when key does not exist
    The docs claim that a new sketch is created with default dimensions if the key does not exist. This is incorrect - the command returns an error "CMS: key does not exist". Also missing: increment must be a strictly positive integer (> 0); passing 0 or a negative value returns an error.

  2. CMS.INITBYPROB - inverted semantics of probability
    The docs describe probability as the probability that an estimate will fall within the error bounds. This is the opposite of the actual behavior - it is the failure probability (probability of exceeding the error bound). The source code confirms: depth = ceil(ln(1 / probability)), meaning a small value like 0.001 gives a deeper, more accurate sketch. The example value 0.999 produces depth = 1 (a single hash function), making the sketch practically useless.

  3. CMS.MERGE - destination must be pre-initialized
    The docs do not mention that destination must already exist before calling the command. If it does not, the command returns "CMS: key does not exist". Should explicitly state: "The destination key must be pre-initialized via CMS.INITBYDIM or CMS.INITBYPROB."

  4. CMS.MERGE - weights must be integers
    Weights are parsed as int64_t in the source code - fractional values are not supported. The docs describe them as "scaling factors" without specifying the type, which could mislead users into passing float values.

@kostasrim
Copy link
Contributor Author

  1. CMS.INCRBY - incorrect behavior when key does not exist
    The docs claim that a new sketch is created with default dimensions if the key does not exist. This is incorrect - the command returns an error "CMS: key does not exist". Also missing: increment must be a strictly positive integer (> 0); passing 0 or a negative value returns an error.
  2. CMS.INITBYPROB - inverted semantics of probability
    The docs describe probability as the probability that an estimate will fall within the error bounds. This is the opposite of the actual behavior - it is the failure probability (probability of exceeding the error bound). The source code confirms: depth = ceil(ln(1 / probability)), meaning a small value like 0.001 gives a deeper, more accurate sketch. The example value 0.999 produces depth = 1 (a single hash function), making the sketch practically useless.
  3. CMS.MERGE - destination must be pre-initialized
    The docs do not mention that destination must already exist before calling the command. If it does not, the command returns "CMS: key does not exist". Should explicitly state: "The destination key must be pre-initialized via CMS.INITBYDIM or CMS.INITBYPROB."
  4. CMS.MERGE - weights must be integers
    Weights are parsed as int64_t in the source code - fractional values are not supported. The docs describe them as "scaling factors" without specifying the type, which could mislead users into passing float values.

Done take a look :)

@kostasrim kostasrim merged commit ca7fffa into main Mar 20, 2026
3 checks passed
@kostasrim kostasrim deleted the cms branch March 20, 2026 08:32
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.

Implement CMS (Count Min Sketch)

2 participants