Skip to content

[fix][broker] fix ExtensibleLoadManager to override the ownerships concurrently without blocking load manager thread #24156

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

Merged
merged 4 commits into from
Apr 9, 2025

Conversation

heesung-sn
Copy link
Contributor

@heesung-sn heesung-sn commented Apr 7, 2025

Motivation

Currently, ExtensibleLoadManager overrides orphan ownerships one by one while holding the monitor lock. This can delay the synchronized serviceUnitStateChannel.started() call(required for SystemTopicPoliciesService.getTopicPoliciesAsync and others) when the metadata service is slow(is overloaded).

Modifications

  • override orphan ownership concurrently (max 10 bundle ownerships by default and configurable by loadBalancerServiceUnitStateMaxConcurrentOverrides).
  • removed unnecessary monitor lock from doCleanup
  • minor change: put TopBundlesLoadData is empty log under debug flag

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:

…ncurrently without blocking load manager thread
Copy link

github-actions bot commented Apr 7, 2025

@heesung-sn Please add the following content to your PR description and select a checkbox:

- [ ] `doc` <!-- Your PR contains doc changes -->
- [ ] `doc-required` <!-- Your PR changes impact docs and you will update later -->
- [ ] `doc-not-needed` <!-- Your PR changes do not impact docs -->
- [ ] `doc-complete` <!-- Docs have been already added -->

@github-actions github-actions bot added doc-not-needed Your PR changes do not impact docs and removed doc-label-missing labels Apr 7, 2025
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some review comments

Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@heesung-sn heesung-sn closed this Apr 8, 2025
@heesung-sn heesung-sn reopened this Apr 8, 2025
@heesung-sn heesung-sn merged commit 066a20c into master Apr 9, 2025
49 of 52 checks passed
@heesung-sn heesung-sn deleted the elb-deadlock branch April 9, 2025 14:04
@lhotari lhotari added this to the 4.1.0 milestone Apr 9, 2025
heesung-sn added a commit that referenced this pull request Apr 9, 2025
…ncurrently without blocking load manager thread (#24156)

(cherry picked from commit 066a20c)
heesung-sn added a commit that referenced this pull request Apr 9, 2025
…ncurrently without blocking load manager thread (#24156)

(cherry picked from commit 066a20c)
poorbarcode pushed a commit to poorbarcode/pulsar that referenced this pull request Apr 15, 2025
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Apr 15, 2025
…ncurrently without blocking load manager thread (apache#24156)

(cherry picked from commit 066a20c)
(cherry picked from commit 970df5f)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Apr 16, 2025
…ncurrently without blocking load manager thread (apache#24156)

(cherry picked from commit 066a20c)
(cherry picked from commit 970df5f)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Apr 16, 2025
…ncurrently without blocking load manager thread (apache#24156)

(cherry picked from commit 066a20c)
(cherry picked from commit 970df5f)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Apr 16, 2025
…ncurrently without blocking load manager thread (apache#24156)

(cherry picked from commit 066a20c)
(cherry picked from commit 970df5f)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Apr 16, 2025
…ncurrently without blocking load manager thread (apache#24156)

(cherry picked from commit 066a20c)
(cherry picked from commit 970df5f)
@heesung-sn
Copy link
Contributor Author

This fix has this bug. please pull this PR as well. #24196

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