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
16k stars 1.41k forks source link

[FIXED] only `UpdateDelivered` with quorum when clustered #6139

Closed MauriceVanVeen closed 1 day ago

MauriceVanVeen commented 3 days ago

store.UpdateDelivered would always be called, without checking quorum. Which meant that we could get a consumer to be ahead on delivered after switching leader. This change makes o.updateDelivered consistent with o.updateAcks, which does the same; if clustered propose through RAFT, if not just update delivered directly.

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

neilalexander commented 2 days ago

For what it's worth I think this type of change makes sense. Quorum is what aligns the leader with the followers. If we're making persistent changes to the state of an asset without going through the Raft apply queue, then we've undermined the point of running Raft in the first place and then all bets are completely off when the leader changes.

derekcollison commented 2 days ago

Like I said better to discuss on a call / video.

MauriceVanVeen commented 1 day ago

@derekcollison , I've marked this PR ready for review as well. We've discussed that only the store delivered state requires quorum so it's consistent on all nodes, and it doesn't block/wait for quorum before sending a message to the client.