Copilot commented on code in PR #25502:
URL: https://github.com/apache/pulsar/pull/25502#discussion_r3056453427
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/PublishRateLimiterImpl.java:
##########
@@ -173,6 +175,12 @@ public void update(PublishRate maxPublishRate) {
} else {
tokenBucketOnMessage = null;
tokenBucketOnByte = null;
+ ScheduledExecutorService executor = lastUnthrottleExecutor;
+ if (executor != null) {
+ // Wake the existing unthrottle path without waiting on an old
delay.
+ scheduleUnthrottling(executor, 0L);
+ }
+ // If executor is null, no throttle has happened yet on this
limiter
}
Review Comment:
This adds new behavior (immediate unthrottling on `update(null)`) that is
timing- and race-sensitive. Please add a test that (1) throttles at least one
producer enough to schedule a long unthrottle delay, then (2) disables rate
limiting via `update(null)`, and (3) asserts queued producers are unthrottled
promptly (without waiting for the originally scheduled delay).
##########
pulsar-broker/src/main/java/org/apache/pulsar/broker/service/PublishRateLimiterImpl.java:
##########
@@ -173,6 +175,12 @@ public void update(PublishRate maxPublishRate) {
} else {
tokenBucketOnMessage = null;
tokenBucketOnByte = null;
+ ScheduledExecutorService executor = lastUnthrottleExecutor;
+ if (executor != null) {
+ // Wake the existing unthrottle path without waiting on an old
delay.
+ scheduleUnthrottling(executor, 0L);
+ }
Review Comment:
`lastUnthrottleExecutor` can become stale (e.g., executor shutdown during
broker/namespace lifecycle changes). If `scheduleUnthrottling(...)` ultimately
calls `ScheduledExecutorService#schedule`, this may throw
`RejectedExecutionException` and break the update path when disabling rate
limiting. Consider handling task rejection (catch `RejectedExecutionException`,
clear `lastUnthrottleExecutor`, and/or fall back to a known-live executor) so
disabling limits is best-effort and resilient.
--
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]
