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

fix: xadd inconsistency with redis implementation #1343

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ullumullu
Copy link
Contributor

@ullumullu ullumullu commented Feb 5, 2024

This commit aligns id generation with the official "specification/docu":

Both quantities are 64-bit numbers. When an ID is auto-generated, the
first part is the Unix time in milliseconds of the Redis instance
generating the ID. The second part is just a sequence number and is
used in order to distinguish IDs generated in the same millisecond.

Main motivation is that the current implementation was flawed when using capped streams by creating invalid ids (e.g. generating the same id twice) as well as re-using mocked tests and run them in an e2e fashion.

Tests were added to cover the enhanced functionality.

Links:
https://redis.io/commands/xadd/

@ullumullu ullumullu changed the title fix xadd inconsistency with redis implementation fix: xadd inconsistency with redis implementation Feb 5, 2024
This commit aligns id generation with the official "specification/docu":

> Both quantities are 64-bit numbers. When an ID is auto-generated, the
> first part is the Unix time in milliseconds of the Redis instance
> generating the ID. The second part is just a sequence number and is
> used in order to distinguish IDs generated in the same millisecond.

Main motivation is that the current implementation was flawed when using
capped streams by creating invalid ids (e.g. generating the same id twice)
as well as re-using mocked tests and run them in an e2e fashion.

Tests were added to cover the additional funcionality.

Links:
https://redis.io/commands/xadd/
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.

None yet

1 participant