Skip to content

[fix][broker] Run the message expiry check off the topic policy update path - #26040

Merged
lhotari merged 2 commits into
apache:masterfrom
lhotari:lh-fix-prevent-message-expiry-blocking-broker-client
Jun 17, 2026
Merged

[fix][broker] Run the message expiry check off the topic policy update path#26040
lhotari merged 2 commits into
apache:masterfrom
lhotari:lh-fix-prevent-message-expiry-blocking-broker-client

Conversation

@lhotari

@lhotari lhotari commented Jun 17, 2026

Copy link
Copy Markdown
Member

Main Issue: #26037

Motivation

When topic policies are applied (PersistentTopic.onUpdate / onPoliciesUpdateapplyUpdatedTopicPolicies),
the message-expiry check (checkMessageExpiry) was run inline on the calling thread. That check can block: it
walks subscription cursors and can wait on metadata operations
(...getNthEntrycounter.await(metadataOperationsTimeoutSeconds), default 60s).

For topic-policy updates this inline work runs on the single, process-wide shared
broker-client-shared-internal-executor reader thread (the thread reported in #26037), so a single topic whose
oldest ledger is temporarily inaccessible could block topic-policy loading/updates. The check was also executed
on every policy update, even when the message TTL had not changed.

Modifications

  • applyUpdatedTopicPolicies no longer runs checkMessageExpiry inline. Instead,
    maybeCheckMessageExpiryOnPolicyUpdateInBackground schedules it on the dedicated messageExpiryMonitor
    thread — the same thread the periodic expiry check already uses — so it never blocks applying or loading
    topic policies. The check is idempotent and the periodic monitor still provides eventual coverage, so it is
    no longer part of the futures returned by applyUpdatedTopicPolicies (callers no longer wait for it).
  • The check now runs only when the effective message TTL actually changed since the last run (tracked via a
    new volatile int lastMessageTtlInSeconds), avoiding redundant scans on unrelated policy updates.
  • Exposed BrokerService.messageExpiryMonitor via @Getter so the topic can schedule on it.

Verifying this change

This change is already covered by existing tests:

  • MessageTTLTesttestTTLPoliciesUpdate was updated to wait for the now-asynchronous expiry check using
    Mockito timeout verification (the previous synchronous verify(times(N)) would otherwise race the
    background execution).
  • PersistentMessageExpiryMonitorMockTest.

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

  • The threading model

The message-expiry check triggered by a topic-policy update now runs asynchronously on the
messageExpiryMonitor thread instead of synchronously on the policy-application thread.

lhotari added 2 commits June 17, 2026 10:51
… changed since the last run, schedule on messageExpiryMonitor thread
…expiry check

checkMessageExpiry now runs asynchronously on the messageExpiryMonitor thread, so the synchronous verify(times(N)) right after the policy update could race the background execution. Use Mockito timeout verification to wait for the invocations, and drop the now-unused static times import.

Assisted-by: Claude Opus 4.8
@lhotari
lhotari merged commit 4763128 into apache:master Jun 17, 2026
44 checks passed
lhotari added a commit that referenced this pull request Jun 22, 2026
lhotari added 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
@lhotari lhotari added this to the 5.0.0-M2 milestone Aug 14, 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