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.

Sign up for GitHub

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

Jump to bottom

[FIXED] Don't replace leader's snapshot during shutdown #6053

Merged
merged 1 commit into from
Oct 29, 2024
Merged

[FIXED] Don't replace leader's snapshot during shutdown #6053

merged 1 commit into from
Oct 29, 2024

Conversation

MauriceVanVeen
Copy link
Member

When a follower received a snapshot from the leader and then restarts (before it can complete the catchup), it overwrites the leader's snapshot with its own. Resulting in a desynced stream, since it doesn't retry catchup.

Signed-off-by: Maurice van Veen github@mauricevanveen.com

derekcollison
derekcollison approved these changes Oct 29, 2024
View reviewed changes
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

@derekcollison
Copy link
Member

Let me know when you move off of draft.

@MauriceVanVeen
[FIXED] Don't replace leader's snapshot during shutdown
1e218f0 
Signed-off-by: Maurice van Veen <github@mauricevanveen.com>
@MauriceVanVeen MauriceVanVeen force-pushed the maurice/restart-replaces-leader-snapshot branch from 00cda7a to 1e218f0 Compare October 29, 2024 11:21
@MauriceVanVeen
Copy link
Member Author

@derekcollison, marked ready for review.

As a heads up, TestJetStreamClusterDeleteConsumerWhileServerDown was failing and changing n.observer to n.paused in the condition fixed it. Not sure if it's still LGTM under that change?

@MauriceVanVeen MauriceVanVeen marked this pull request as ready for review October 29, 2024 12:36
@MauriceVanVeen MauriceVanVeen requested a review from a team as a code owner October 29, 2024 12:36
@derekcollison derekcollison merged commit 4428dd9 into main Oct 29, 2024
5 checks passed
@derekcollison derekcollison deleted the maurice/restart-replaces-leader-snapshot branch October 29, 2024 15:06
@neilalexander neilalexander mentioned this pull request Oct 29, 2024
Merged
neilalexander added a commit that referenced this pull request Oct 29, 2024
@neilalexander
Cherry-picks for 2.10.23-RC.1 (#6054)
8f6dd32 
Includes the following:

- #5115
- #6019
- #6039
- #6034
- #6043
- #6042
- #6047
- #6049
- #6050 
- #6052
- #6053

Signed-off-by: Neil Twigg <neil@nats.io>
@MauriceVanVeen MauriceVanVeen mentioned this pull request Oct 30, 2024
Merged
@neilalexander neilalexander mentioned this pull request Nov 25, 2024
Merged
neilalexander added a commit that referenced this pull request Nov 25, 2024
@neilalexander
Cherry-picks for 2.10.23-RC.5 (#6171)
24acb45 
Includes the following:

- #5661
- #5666
- #5671
- #5344
- #5684
- #5689
- #5691
- #5714
- #5717
- #5707
- #5792
- #5912
- #5957
- #5700
- #5975
- #5991
- #5987
- #6027
- #6038
- #6053
- #5848
- #6055
- #6056
- #6060
- #6061
- #6072
- #5832
- #6073
- #6107

Signed-off-by: Neil Twigg <neil@nats.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derekcollison derekcollison derekcollison approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@MauriceVanVeen @derekcollison