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 an error with continuous subscription #49

Conversation

Swoorup
Copy link
Contributor

@Swoorup Swoorup commented Apr 28, 2022

Continuous subscription was not being updated correctly as it was never getting the offset correctly from the response. (Likely lost during the initial refactor).

Also added a test, which tries to request more records than present eventually timing out. This would originally return repeated records from the beginning as the offsets were never updated.

Also added xrange and xrevrange as a bonus.

Copy link
Collaborator

@ChristopherDavenport ChristopherDavenport left a comment

Choose a reason for hiding this comment

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

I swear I was consuming messages with this when I merged it. Maybe I was getting the wrong offset though?

@Swoorup
Copy link
Contributor Author

Swoorup commented Apr 29, 2022

I swear I was consuming messages with this when I merged it. Maybe I was getting the wrong offset though?

Yeah, it would have called xread with the initial offset repeatedly, since the offsetsByKey returned map of MessageId to offset, which would never merged with the map of Stream to offset

@ChristopherDavenport
Copy link
Collaborator

This is fully binary compatible right? Making sure I'm reading this right I think I can release this as a patch.

@ChristopherDavenport ChristopherDavenport merged commit 04a2490 into davenverse:main Apr 29, 2022
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