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

[improve][broker] Should notify bundle ownership listener onLoad event when ServiceUnitState start (ExtensibleLoadManagerImpl only) #23152

Conversation

Demogorgon314
Copy link
Member

Motivation

The ExtensibleLoadManagerImpl behavior is not the same as the old LB, the old LB’s ownership will be empty when the broker starts.

When ExtensibleLoadManagerImpl starts, it may have some owners will be loaded into table view, and those ownerships will not trigger the bundle ownership listener, which may cause some issues, for example, the kop relies on the load event to load the offset topics.

Modifications

Notify bundle ownership listener onLoad event when ServiceUnitState starts.

Documentation

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

@Demogorgon314 Demogorgon314 self-assigned this Aug 12, 2024
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Aug 12, 2024
heesung-sn
heesung-sn previously approved these changes Aug 12, 2024
@codecov-commenter
Copy link

codecov-commenter commented Aug 12, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.57%. Comparing base (bbc6224) to head (8b02802).
Report is 527 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #23152      +/-   ##
============================================
+ Coverage     73.57%   74.57%   +0.99%     
- Complexity    32624    33670    +1046     
============================================
  Files          1877     1921      +44     
  Lines        139502   144494    +4992     
  Branches      15299    15810     +511     
============================================
+ Hits         102638   107750    +5112     
+ Misses        28908    28502     -406     
- Partials       7956     8242     +286     
Flag Coverage Δ
inttests 27.76% <22.22%> (+3.17%) ⬆️
systests 24.71% <0.00%> (+0.39%) ⬆️
unittests 73.92% <100.00%> (+1.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...xtensions/channel/ServiceUnitStateChannelImpl.java 87.04% <100.00%> (+1.73%) ⬆️

... and 507 files with indirect coverage changes

@heesung-sn heesung-sn dismissed their stale review August 12, 2024 15:05

Change is expected

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

@lhotari lhotari merged commit 3053b64 into apache:master Aug 16, 2024
51 checks passed
@lhotari lhotari added this to the 4.0.0 milestone Aug 16, 2024
lhotari pushed a commit that referenced this pull request Aug 16, 2024
…t when ServiceUnitState start (ExtensibleLoadManagerImpl only) (#23152)

(cherry picked from commit 3053b64)
lhotari pushed a commit that referenced this pull request Aug 16, 2024
…t when ServiceUnitState start (ExtensibleLoadManagerImpl only) (#23152)

(cherry picked from commit 3053b64)
heesung-sn pushed a commit that referenced this pull request Aug 16, 2024
…t when ServiceUnitState start (ExtensibleLoadManagerImpl only) (#23152)

(cherry picked from commit 3053b64)
heesung-sn pushed a commit that referenced this pull request Aug 16, 2024
…t when ServiceUnitState start (ExtensibleLoadManagerImpl only) (#23152)

(cherry picked from commit 3053b64)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Aug 22, 2024
…t when ServiceUnitState start (ExtensibleLoadManagerImpl only) (apache#23152)

(cherry picked from commit 3053b64)
(cherry picked from commit 9a090f7)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Aug 23, 2024
…t when ServiceUnitState start (ExtensibleLoadManagerImpl only) (apache#23152)

(cherry picked from commit 3053b64)
(cherry picked from commit 9a090f7)
@lhotari
Copy link
Member

lhotari commented Aug 30, 2024

This test is very flaky. Would it be possible to fix it or disable it until it works? Reported #23240 for this

grssam pushed a commit to grssam/pulsar that referenced this pull request Sep 4, 2024
…t when ServiceUnitState start (ExtensibleLoadManagerImpl only) (apache#23152)
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.

4 participants