Skip to content

[fix][broker] Avoid blocking the dispatcher close path on delayed-delivery tracker close - #26053

Merged
lhotari merged 1 commit into
apache:masterfrom
merlimat:mmerli/fix-delayed-delivery-tracker-close-blocking
Jun 18, 2026
Merged

[fix][broker] Avoid blocking the dispatcher close path on delayed-delivery tracker close#26053
lhotari merged 1 commit into
apache:masterfrom
merlimat:mmerli/fix-delayed-delivery-tracker-close-blocking

Conversation

@merlimat

Copy link
Copy Markdown
Contributor

Motivation

BucketDelayedDeliveryTracker.close() blocks up to AsyncOperationTimeoutSeconds (60s) on FutureUtil.waitForAll(snapshotCreateFutures).get(...), hidden behind the void close() of DelayedDeliveryTracker. It is invoked by PersistentDispatcherMultipleConsumers.close() (and the Classic variant) via delayedDeliveryTracker.ifPresent(DelayedDeliveryTracker::close), which runs inside the topic/subscription close chain on a metadata / CompletableFuture-callback thread (e.g. the ExtensibleLoadManager getAssignedBrokerLookupData(...).thenAccept(...) continuation during unload). A pending snapshot generation could therefore stall that callback thread for up to 60s.

Modifications

  • Add CompletableFuture<Void> closeAsync() to the DelayedDeliveryTracker interface as a default that delegates to close(). Because the interface extends AutoCloseable, close() must stay void; the default keeps InMemoryDelayedDeliveryTracker and the DISABLE no-op working unchanged.
  • BucketDelayedDeliveryTracker overrides closeAsync() to perform the cleanup (super.close() + the bucket queue closes) under its monitor and then return waitForAll(snapshotCreateFutures) (with .exceptionally(...) logging, as before) instead of blocking on get(60s). Its AutoCloseable close() now delegates to closeAsync().join() for synchronous callers (try-with-resources, tests).
  • PersistentDispatcherMultipleConsumers and its Classic variant chain closeAsync() into the returned close future, so the tracker still finishes closing before consumers are disconnected — but without blocking the callback thread.

Verifying this change

This change is covered by existing tests, which pass: BucketDelayedDeliveryTrackerTest, BucketDelayedDeliveryTrackerThreadSafetyTest, InMemoryDeliveryTrackerTest, and DelayedDeliveryTrackerFactoryTest.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

  • Dependencies (add or upgrade a dependency)
  • The public API
  • The schema
  • The default values of configurations
  • The threading model
  • The binary protocol
  • The REST endpoints
  • The admin CLI options
  • The metrics
  • Anything that affects deployment

…ivery tracker close

BucketDelayedDeliveryTracker.close() blocks up to AsyncOperationTimeoutSeconds
(60s) on FutureUtil.waitForAll(snapshotCreateFutures).get(...), behind the void
close() of DelayedDeliveryTracker. It is invoked by
PersistentDispatcherMultipleConsumers.close() (and the Classic variant) via
delayedDeliveryTracker.ifPresent(DelayedDeliveryTracker::close), which runs in
the topic/subscription close chain on a metadata/CompletableFuture-callback
thread (e.g. the ExtensibleLoadManager getAssignedBrokerLookupData(...)
.thenAccept(...) continuation during unload). A pending snapshot generation could
therefore stall that callback thread for up to 60s.

Add CompletableFuture<Void> closeAsync() to the DelayedDeliveryTracker interface
(a default that delegates to close(), so InMemoryDelayedDeliveryTracker and
DISABLE keep working). BucketDelayedDeliveryTracker overrides closeAsync() to run
the cleanup under its monitor and return waitForAll(snapshotCreateFutures) (with
.exceptionally to log, as before) instead of blocking on get(60s); its
AutoCloseable close() now delegates to closeAsync().join() for synchronous
callers. Both PersistentDispatcherMultipleConsumers and its Classic variant chain
closeAsync() into the returned close future, so the tracker still finishes
closing before consumers are disconnected, without blocking the callback thread.
@lhotari
lhotari merged commit 82074aa into apache:master Jun 18, 2026
44 checks passed
@lhotari lhotari added this to the 5.0.0-M2 milestone Jun 18, 2026
lhotari pushed a commit that referenced this pull request Jun 22, 2026
lhotari pushed a commit that referenced this pull request Jun 22, 2026
sandeep-ctds pushed a commit to datastax/pulsar that referenced this pull request Jul 31, 2026
nodece pushed a commit to ascentstream/pulsar that referenced this pull request Aug 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants