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

[FIXED] Fix consumer start sequence when sequence not yet in stream #5785

Merged
merged 1 commit into from
Aug 13, 2024

Conversation

neilalexander
Copy link
Member

The consumer sseq was being clipped back to the range between the stream FirstSeq and LastSeq even when the consumer start sequence was provided, so it wasn't possible to provide a start sequence that didn't exist in the stream yet. This PR changes that to now obey the start sequence even in that case.

Note that this problem will still exist with the consumer start timestamp, as that's predicated on being able to match the timestamp to a sequence number in the stream, which it won't be able to do if the stream doesn't contain any sequences for that time.

Signed-off-by: Neil Twigg [email protected]

@neilalexander neilalexander requested a review from a team as a code owner August 13, 2024 10:47
Copy link
Member

@derekcollison derekcollison left a comment

Choose a reason for hiding this comment

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

LGTM - Thanks

@derekcollison derekcollison merged commit 1758e0a into main Aug 13, 2024
5 checks passed
@derekcollison derekcollison deleted the neil/optstartseq branch August 13, 2024 14:55
@wallyqs wallyqs changed the title Fix consumer start sequence when sequence not yet in stream [FIXED] Fix consumer start sequence when sequence not yet in stream Aug 23, 2024
wallyqs added a commit that referenced this pull request Aug 23, 2024
derekcollison added a commit that referenced this pull request Oct 17, 2024
This PR backs out #5785, which was released in v2.10.19, as it has a
negative impact on sourcing and mirroring. See discussion at #6005.

Two new unit tests, `TestJetStreamSourcingClipStartSeq` and
`TestJetStreamMirroringClipStartSeq`, are added to show this behaviour
in action.

We will probably want to revisit this for 2.11 so that we can come up
with something that works for both use-cases.

Signed-off-by: Neil Twigg <[email protected]>
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