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] KeyValue not found after server restarts #5054

Merged
merged 2 commits into from
Feb 9, 2024
Merged

Conversation

derekcollison
Copy link
Member

Do not force sequence mismatches and possible dataloss on first sequence move past a catchup request.

When a server restarts and does a stream catchup where the snapshot from the leader is for messages it no longer has, we would create a sequence mismatch reset on the follower.

Also on a catchup if the stream state has moved past the updated request range from a follower, meaning first for the new state is past the last of the request, extend and continue catchup.

Signed-off-by: Derek Collison derek@nats.io

…nce move past a catchup request.

When a server restarts and does a stream catchup where the snapshot from the leader is for messages it no longer has, we would create a sequence mismatch reset on the follower.
Also on a catchup if the stream state has moved past the updated request range from a follower, meaning first for the new state is past the last of the request, extend and continue catchup.

Signed-off-by: Derek Collison <derek@nats.io>
Signed-off-by: Derek Collison <derek@nats.io>
@derekcollison derekcollison requested a review from a team as a code owner February 9, 2024 20:01
Copy link
Member

@wallyqs wallyqs left a comment

Choose a reason for hiding this comment

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

LGTM

@derekcollison derekcollison merged commit eadab32 into main Feb 9, 2024
4 checks passed
@derekcollison derekcollison deleted the kv-not-found branch February 9, 2024 23:37
derekcollison added a commit that referenced this pull request Feb 10, 2024
Do not force sequence mismatches and possible dataloss on first sequence
move past a catchup request.

When a server restarts and does a stream catchup where the snapshot from
the leader is for messages it no longer has, we would create a sequence
mismatch reset on the follower.
    
Also on a catchup if the stream state has moved past the updated request
range from a follower, meaning first for the new state is past the last
of the request, extend and continue catchup.

Signed-off-by: Derek Collison <derek@nats.io>

---------

Signed-off-by: Derek Collison <derek@nats.io>
wallyqs pushed a commit that referenced this pull request Feb 13, 2024
Do not force sequence mismatches and possible dataloss on first sequence
move past a catchup request.

When a server restarts and does a stream catchup where the snapshot from
the leader is for messages it no longer has, we would create a sequence
mismatch reset on the follower.
    
Also on a catchup if the stream state has moved past the updated request
range from a follower, meaning first for the new state is past the last
of the request, extend and continue catchup.

Signed-off-by: Derek Collison <derek@nats.io>

---------

Signed-off-by: Derek Collison <derek@nats.io>
wallyqs added a commit that referenced this pull request Feb 14, 2024
derekcollison added a commit that referenced this pull request Oct 24, 2024
When we're being asked to provide data within a range during catchup, we
should not extend that range and provide more data. Especially since
that range was defined by a snapshot, which also specifies which RAFT
entries should be sent after processing that snapshot. This would just
result in duplicated work and possible desync for the follower, so these
lines can safely be removed.

**Timeline of these lines:**
Previously when receiving a catchup request the `FirstSeq` could be
moved up to match what the state says:
[[IMPROVED] Catchup improvements
#3348](https://github.com/nats-io/nats-server/pull/3348/files#diff-5cb252c37caef12e7027803018861c82724b120ddb62cfedc2f36addf57f6970R7132-R7138)
(August, 2022)

Afterward this was removed in favor of only extending the `LastSeq`:
[[FIXED] KeyValue not found after server restarts
#5054](https://github.com/nats-io/nats-server/pull/5054/files#diff-5cb252c37caef12e7027803018861c82724b120ddb62cfedc2f36addf57f6970R8579-R8588)
(February, 2024)

This was done to solve for KeyValue not found issues.
However this change would have also fixed that case:
[[FIXED] Do not bump clfs on seq mismatch when before stream LastSeq
#5821](https://github.com/nats-io/nats-server/pull/5821/files#diff-2f4991438bb868a8587303cde9107f83127e88ad70bd19d5c6a31c238a20c299R4694-R4699)
(August, 2024)

Signed-off-by: Maurice van Veen <github@mauricevanveen.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