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] Fixed ServiceUnitStateChannel monitor to tombstone only inactive bundle states #21721

Merged

Conversation

heesung-sn
Copy link
Contributor

@heesung-sn heesung-sn commented Dec 13, 2023

Motivation

There is a regression in the previous PR, https://github.com/apache/pulsar/pull/21213/files#r1425693449.

We should tombstone only inactive bundle states by the bundle state channel monitor thread. Currently, active bundle states(including the Owned state) are tombstoned(cleaned), which incurs new bundle assignments.

Modifications

Verifying this change

  • Make sure that the change passes the CI checks.

This change is already covered by existing tests. Please refer to the test changes in the PR.

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 Dec 13, 2023
@heesung-sn heesung-sn self-assigned this Dec 13, 2023
@dragosvictor
Copy link
Contributor

Looks good to me!

@codecov-commenter
Copy link

codecov-commenter commented Dec 14, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.38%. Comparing base (50007c3) to head (f799009).
Report is 761 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #21721      +/-   ##
============================================
- Coverage     73.50%   73.38%   -0.12%     
+ Complexity    32818    32757      -61     
============================================
  Files          1893     1893              
  Lines        140721   140723       +2     
  Branches      15502    15503       +1     
============================================
- Hits         103432   103270     -162     
- Misses        29200    29345     +145     
- Partials       8089     8108      +19     
Flag Coverage Δ
inttests 24.11% <0.00%> (-0.05%) ⬇️
systests 24.66% <0.00%> (-0.19%) ⬇️
unittests 72.68% <100.00%> (-0.10%) ⬇️

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

Files with missing lines Coverage Δ
...xtensions/channel/ServiceUnitStateChannelImpl.java 84.26% <100.00%> (-1.27%) ⬇️

... and 75 files with indirect coverage changes

@heesung-sn heesung-sn merged commit 8d16580 into apache:master Dec 14, 2023
74 of 77 checks passed
Demogorgon314 pushed a commit that referenced this pull request Dec 25, 2023
… inactive bundle states (#21721)

(cherry picked from commit 8d16580)
heesung-sn added a commit to heesung-sn/pulsar that referenced this pull request Jan 4, 2024
heesung-sn added a commit that referenced this pull request Jan 4, 2024
… inactive bundle states (#21721)

(cherry picked from commit 8d16580)
nikhil-ctds pushed a commit to datastax/pulsar that referenced this pull request Jan 4, 2024
… inactive bundle states (apache#21721)

(cherry picked from commit 8d16580)
(cherry picked from commit 30fe564)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Jan 8, 2024
… inactive bundle states (apache#21721)

(cherry picked from commit 8d16580)
(cherry picked from commit 30fe564)
@heesung-sn heesung-sn deleted the fix-bundle-state-monitor-tombstone branch April 2, 2024 17:45
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.

7 participants