Skip to content

[fix][broker] Preserve topic initialization failures - #26775

Merged
nodece merged 1 commit into
apache:masterfrom
lhotari:improve-topic-initialization-checks
Sep 30, 2026
Merged

nodece merged 1 commit into
apache:masterfrom
lhotari:improve-topic-initialization-checks

Conversation

@lhotari

@lhotari lhotari commented Sep 30, 2026

Copy link
Copy Markdown
Member

Motivation

PersistentTopic.initialize() loads the namespace policies, then the topic policies, and then removes orphan replication cursors. When any of these stages failed, the failure was logged and ignored, isEncryptionRequired was set to false, and the topic was loaded anyway. The topic then served clients with settings that did not match its configured policies, and it stayed loaded until it was unloaded.

A topic should only become available after its initialization succeeds.

Modifications

  • Remove the exceptionally handler in PersistentTopic.initialize(), so a failure in loading namespace policies, loading topic policies or removing orphan replication cursors fails the initialization.
  • The existing handling in BrokerService then closes the partially initialized topic and removes it from the topic cache, so a later load retries the initialization. Its log message now describes this as a topic initialization failure.
  • Correct the topic policies mock in PersistentTopicTest#testGetReplicationClusters (in org.apache.pulsar.broker.service). The test no longer relies on an ignored initialization failure, and it now verifies that topic-level replication clusters override the namespace-level ones.

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

  • Added PersistentTopicTest#testInitializationFailureClosesTopicAndAllowsRetry (in org.apache.pulsar.broker.service.persistent), which simulates a failure in loading namespace policies, loading topic policies, or removing an orphan replication cursor. It checks that the topic load fails, the topic is removed from the cache and its managed ledger is closed, and that a retry loads the topic with the configured policies applied.

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: a topic load now fails when loading its policies or removing orphan replication cursors fails. The load can be retried once the underlying condition recovers.

PersistentTopic.initialize() loads the namespace policies, then the topic
policies, and then removes orphan replication cursors. A failure in any of
these stages was logged and ignored, isEncryptionRequired was set to false,
and the topic was loaded anyway with settings that did not match its
configured policies.

Remove the exceptionally handler so these failures fail the initialization.
The existing handling in BrokerService then closes the partially
initialized topic and removes it from the topic cache, so a later load
retries. Its log message now describes this as a topic initialization
failure.

Add a test that simulates a failure in each stage and verifies that the
load fails, the topic is closed and removed from the cache, and a retry applies
the configured policies. Correct the topic policies mock in
testGetReplicationClusters, which relied on the ignored failure; it now
verifies that topic-level replication clusters override namespace-level
ones.

Assisted-by: Claude Code
@nodece
nodece merged commit c623cfd into apache:master Sep 30, 2026
44 checks passed
ascentstream-bot pushed a commit to ascentstream/pulsar that referenced this pull request Oct 1, 2026
ascentstream-bot pushed a commit to ascentstream/pulsar that referenced this pull request Oct 2, 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.

3 participants