-
Notifications
You must be signed in to change notification settings - Fork 207
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
fix: ACK deadline set for received messages can be too low #416
Conversation
When a message is received, the initial MODACK value used should be the same value currently used by leaser. This makes sure the leaser will wake up and extend the ACK deadline before the initial one expires.
If flow_control.max_duration_per_lease_extension is set to too low a value, it is adjusted to the minimum ACK deadline.
Hmm... this PR fixes the issue caused by directly using the Consider - When yet more messages arrive, the shortened ACK deadline will be used for them while the leaser is still asleep (due to the previous "long" deadline). The leaser will not wake in time to extend the deadlines of these new messages. Bottom line - the ACK deadline should probably not change while the leaser is asleep so that |
The ACK deadline is dynamically being adjusted based on ACK data, but an updated deadline value must not be used for the new messages that arrive while the leaser thread is still sleeping. Doing so could result in the messages' ACK deadlines expiring before the leaser wakes up and has a chance of extending the deadlines for these messages. An updated ACK deadline value for new messages is thus only used when the leaser starts using it, too.
Fixed, only the leaser now updates the |
Fixes #413.
This PR makes sure that the ACK deadline set for the received messages is always consistent with what the leaser uses internally when extending the ACK deadlines for the leased messages.
See the issue description and a comment explaining a possible sequence of events that lead to a bug.
PR checklist