lhotari commented on code in PR #26012:
URL: https://github.com/apache/pulsar/pull/26012#discussion_r3405183509
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/AbstractDelayedDeliveryTracker.java:
##########
@@ -133,7 +133,11 @@ protected void updateTimer() {
// either not connected or slow.
// We don't need to keep retriggering the timer. When the consumer
// catches up, the dispatcher will do the readMoreEntries() and
- // get these messages
+ // get these messages.
+ // Reset the tracked state so a subsequent updateTimer() call
cannot short-circuit on a stale
+ // currentTimeoutTarget (the timeout was just cancelled above and
no live timer remains). See #25996.
+ currentTimeoutTarget = -1;
+ timeout = null;
Review Comment:
this is correct, but it would be logical to set it after `timeout.cancel()`
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/delayed/AbstractDelayedDeliveryTracker.java:
##########
@@ -133,7 +133,11 @@ protected void updateTimer() {
// either not connected or slow.
// We don't need to keep retriggering the timer. When the consumer
// catches up, the dispatcher will do the readMoreEntries() and
- // get these messages
+ // get these messages.
+ // Reset the tracked state so a subsequent updateTimer() call
cannot short-circuit on a stale
+ // currentTimeoutTarget (the timeout was just cancelled above and
no live timer remains). See #25996.
+ currentTimeoutTarget = -1;
Review Comment:
this is correct, but it would be logical to set this after `if (timestamp ==
currentTimeoutTarget) { ... }`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
