Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix][broker] Skip topic.close during unloading if the topic future fails with ownership check, and fix isBundleOwnedByAnyBroker to use ns.checkOwnershipPresentAsync for ExtensibleLoadBalancer #22379

Merged

Conversation

heesung-sn
Copy link
Contributor

@heesung-sn heesung-sn commented Mar 28, 2024

Fixes #20157

Motivation

#20157 found that the ExtensibleLoadManagerImplTest init step, unloading the test namespace often fails.

Modifications

  • Skip topic.close during unloading if the topic future fails with ownership check. If the unloading(not transfer) happens in the middle of topic creation, the topic creation might fail because of the ownership check failure. In this case, we need to skip unloading that topic, not to fail the unload command.
  • Fix isBundleOwnedByAnyBroker to use ns.checkOwnershipPresentAsync for ExtensibleLoadBalancer. Currently, isBundleOwnedByAnyBroker is unnecessarily selecting a new owner broker, if no owner is found. We should simply check ownership, instead.
  • Skip assignment command events in UnloadManager.handlEvent. UnloadManager should not complete the inflight jobs from the Owned event from assignment commands. This could result in returning unload command too fast, although the unloading is still in progress.

Verifying this change

  • Make sure that the change passes the CI checks.

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

Documentation

  • doc
  • doc-required
  • doc-not-needed
  • doc-complete

Matching PR in forked repository

PR in forked repository:

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Mar 28, 2024
@heesung-sn heesung-sn self-assigned this Mar 28, 2024
@heesung-sn heesung-sn added this to the 3.3.0 milestone Mar 28, 2024
@heesung-sn heesung-sn force-pushed the fix-topic-creation-failure-upon-unloading branch from e8c197a to 8b42318 Compare March 28, 2024 19:07
…ails with ownership check, and fix isBundleOwnedByAnyBroker to use ns.checkOwnershipPresentAsync for ExtensibleLoadBalancer
@heesung-sn heesung-sn force-pushed the fix-topic-creation-failure-upon-unloading branch from 8b42318 to e0cd7c2 Compare March 28, 2024 19:45
@heesung-sn heesung-sn closed this Mar 30, 2024
@heesung-sn heesung-sn reopened this Mar 30, 2024
@heesung-sn heesung-sn removed the request for review from lhotari March 30, 2024 17:12
@Technoboy- Technoboy- merged commit 3eb3b1c into apache:master Apr 1, 2024
69 of 99 checks passed
Technoboy- pushed a commit to Technoboy-/pulsar that referenced this pull request Apr 1, 2024
…ails with ownership check, and fix isBundleOwnedByAnyBroker to use ns.checkOwnershipPresentAsync for ExtensibleLoadBalancer (apache#22379)
@heesung-sn heesung-sn deleted the fix-topic-creation-failure-upon-unloading branch April 2, 2024 17:43
heesung-sn added a commit to heesung-sn/pulsar that referenced this pull request Apr 2, 2024
…ails with ownership check, and fix isBundleOwnedByAnyBroker to use ns.checkOwnershipPresentAsync for ExtensibleLoadBalancer (apache#22379)

(cherry picked from commit 3eb3b1c)
heesung-sn added a commit to heesung-sn/pulsar that referenced this pull request Apr 2, 2024
…ails with ownership check, and fix isBundleOwnedByAnyBroker to use ns.checkOwnershipPresentAsync for ExtensibleLoadBalancer (apache#22379)

(cherry picked from commit 3eb3b1c)
heesung-sn added a commit to heesung-sn/pulsar that referenced this pull request Apr 2, 2024
…ails with ownership check, and fix isBundleOwnedByAnyBroker to use ns.checkOwnershipPresentAsync for ExtensibleLoadBalancer (apache#22379)

(cherry picked from commit 3eb3b1c)
heesung-sn added a commit that referenced this pull request Apr 3, 2024
…ails with ownership check, and fix isBundleOwnedByAnyBroker to use ns.checkOwnershipPresentAsync for ExtensibleLoadBalancer (#22379) (#22407)
heesung-sn added a commit that referenced this pull request Apr 3, 2024
…ails with ownership check, and fix isBundleOwnedByAnyBroker to use ns.checkOwnershipPresentAsync for ExtensibleLoadBalancer (#22379) (#22406)
heesung-sn added a commit that referenced this pull request Apr 3, 2024
…ails with ownership check, and fix isBundleOwnedByAnyBroker to use ns.checkOwnershipPresentAsync for ExtensibleLoadBalancer (#22379) (#22403)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Apr 3, 2024
…ails with ownership check, and fix isBundleOwnedByAnyBroker to use ns.checkOwnershipPresentAsync for ExtensibleLoadBalancer (apache#22379) (apache#22406)

(cherry picked from commit e664432)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Apr 4, 2024
…ails with ownership check, and fix isBundleOwnedByAnyBroker to use ns.checkOwnershipPresentAsync for ExtensibleLoadBalancer (apache#22379) (apache#22406)

(cherry picked from commit e664432)
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.

Flaky-test: ExtensibleLoadManagerImplTest.initializeState
4 participants