search

nats-io / nats-server

High-Performance server for NATS.io, the cloud and edge native messaging system.
https://nats.io
Apache License 2.0
16.03k stars 1.41k forks source link

[FIXED] Catchup must not extend past requested sequence range #6038

Closed MauriceVanVeen closed 1 month ago

MauriceVanVeen commented 1 month ago

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 (August, 2022)

Afterward this was removed in favor of only extending the LastSeq: [FIXED] KeyValue not found after server restarts #5054 (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 (August, 2024)

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

derekcollison commented 1 month ago

This is till marked draft, lmk if this should be merged.

MauriceVanVeen commented 1 month ago

This is till marked draft, lmk if this should be merged.

Marked ready for review/merge now :slightly_smiling_face:

I've put these PRs up as draft so these could first be discussed with some colleagues before opening up.