Skip to content

[fix][broker]Incorrect source side topic permission deletion when disabling replication - #26479

Open
poorbarcode wants to merge 1 commit into
apache:masterfrom
poorbarcode:permission_replication
Open

[fix][broker]Incorrect source side topic permission deletion when disabling replication#26479
poorbarcode wants to merge 1 commit into
apache:masterfrom
poorbarcode:permission_replication

Conversation

@poorbarcode

Copy link
Copy Markdown
Contributor

Motivation

Env

  • two clsuters [c1, c2] with shared metadata store

Issue

  • disabled replication by set topic level replication [c1]
    • then c2 can not access the topic anymore
  • Issue: c1 cluster topic permission configuration was deleted

Modifications

fix the issue

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

@void-ptr974

Copy link
Copy Markdown
Contributor

Thanks for addressing the shared topic-permission issue during cluster-removal cleanup. The added test covers the intended scenario well. I noticed two additional cases introduced by the new asynchronous check:

  1. checkAllowedCluster() also returns false for ordinary non-replicated namespaces, where both the effective replication clusters and allowed_clusters are empty. Normal topic deletion therefore skips authentication cleanup. The topic ACL remains in metadata and may be applied again if a topic with the same name is recreated. This path also causes the current PersistentTopicTest.testDeleteTopicDeleteOnMetadataStoreFailed CI failure, because topic.delete() succeeds after bypassing the expected permission-removal failure.

  2. When getPoliciesAsync() completes exceptionally, the thenAccept callback is not invoked and its returned stage is ignored. deleteTopicAuthenticationFuture consequently never completes. The public deletion future can then remain pending indefinitely after the topic has been fenced, leaving the topic unavailable until external recovery.

Both cases affect deletion behavior outside the successful cluster-removal scenario covered by the new test and should be addressed before merging.

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