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

[IMPROVED] Pre-acks handling, detecting ack for out of bounds sequence. #6109

Closed derekcollison closed 1 week ago

derekcollison commented 1 week ago

Detect if we receive an ack past our last stream sequence.

We also no longer register pre-acks when we detect this from a consumer snapshot since we properly handle this now and this could lead to excessive memory usage.

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

derekcollison commented 1 week ago

Question for the team, currently I do return if the ack is waiting for a reply. Currently we do not respond with any errors in 2.10.x so that will need to wait for 2.11 (@jnmoyne) but should we not return and let the ack timeout instead?

@ripienaar interested in your thoughts here.

derekcollison commented 1 week ago

ok after thinking I believe we should timeout on an AckSync and err when we hit 2.11.

I force pushed the change to the code and the test.

jnmoyne commented 1 week ago

ok after thinking I believe we should timeout on an AckSync and err when we hit 2.11.

I agree that it should error out in 2.11 (e.g. especially if at the same time we start erroring out also in case it's the message is already ack'd).