Skip to content

[fix] [broker] do not filter system topic while shedding.#18949

Merged
congbobo184 merged 3 commits into
apache:masterfrom
thetumbled:fixbug_doNotFilterSystemTopic
May 31, 2023
Merged

[fix] [broker] do not filter system topic while shedding.#18949
congbobo184 merged 3 commits into
apache:masterfrom
thetumbled:fixbug_doNotFilterSystemTopic

Conversation

@thetumbled

@thetumbled thetumbled commented Dec 16, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

See this PR for detailed discussion
#18936

Motivation

Currently, topics/bundles in pulsar/system will be filter while doing shedding, which is introduced by mistake by pr #15252.
But we need to unload topics/bundles in pulsar/system for load balancing.

Modifications

do not filter topics/bundles in pulsar/system.

Verifying this change

  • Make sure that the change passes the CI checks.

This change is already covered by existing tests, such as (please describe tests).

Documentation

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

Matching PR in forked repository

PR in forked repository: thetumbled#10

@github-actions github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Dec 16, 2022
@thetumbled thetumbled mentioned this pull request Dec 16, 2022
Closed
@thetumbled

thetumbled commented Dec 16, 2022

Copy link
Copy Markdown
Member Author

PTAL, thanks. @congbobo184

@codecov-commenter

codecov-commenter commented Dec 16, 2022 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.92%. Comparing base (fb05c45) to head (aa2a814).
⚠️ Report is 2472 commits behind head on master.

Files with missing lines Patch % Lines
...ache/pulsar/broker/namespace/NamespaceService.java 0.00% 0 Missing and 3 partials ⚠️
...org/apache/pulsar/broker/loadbalance/LoadData.java 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #18949      +/-   ##
============================================
+ Coverage     72.52%   72.92%   +0.39%     
+ Complexity    31937    31907      -30     
============================================
  Files          1854     1867      +13     
  Lines        138273   138503     +230     
  Branches      15178    15197      +19     
============================================
+ Hits         100282   101002     +720     
+ Misses        30014    29487     -527     
- Partials       7977     8014      +37
Flag Coverage Δ
inttests 24.30% <0.00%> (+0.09%) ⬆️
systests 24.93% <0.00%> (?)
unittests 72.21% <0.00%> (-0.07%) ⬇️

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

Files with missing lines Coverage Δ
...org/apache/pulsar/broker/loadbalance/LoadData.java 91.66% <0.00%> (ø)
...ache/pulsar/broker/namespace/NamespaceService.java 69.67% <0.00%> (-0.27%) ⬇️

... and 158 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

eolivelli
eolivelli requested changes Dec 19, 2022
View reviewed changes
Comment thread pulsar-broker/src/main/java/org/apache/pulsar/broker/loadbalance/LoadData.java Outdated
@thetumbled thetumbled force-pushed the fixbug_doNotFilterSystemTopic branch from 5c3dce8 to b30ae90 Compare December 21, 2022 02:33
@thetumbled

thetumbled commented Dec 28, 2022

Copy link
Copy Markdown
Member Author

PTAL, thanks. @codelipenghui @eolivelli

@github-actions

github-actions Bot commented Jan 28, 2023

Copy link
Copy Markdown

The pr had no activity for 30 days, mark with Stale label.

@github-actions github-actions Bot added the Stale label Jan 28, 2023
@thetumbled

thetumbled commented Feb 28, 2023

Copy link
Copy Markdown
Member Author

PTAL, thanks. @codelipenghui @eolivelli

eolivelli
eolivelli approved these changes Feb 28, 2023
View reviewed changes

@eolivelli eolivelli left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1

@github-actions github-actions Bot removed the Stale label Mar 1, 2023
@github-actions

github-actions Bot commented Apr 1, 2023

Copy link
Copy Markdown

The pr had no activity for 30 days, mark with Stale label.

@github-actions github-actions Bot added the Stale label Apr 1, 2023
@congbobo184 congbobo184 closed this Apr 3, 2023
@congbobo184 congbobo184 reopened this Apr 3, 2023
@congbobo184 congbobo184 reopened this May 30, 2023
@congbobo184 congbobo184 merged commit 5c74d20 into apache:master May 31, 2023
Technoboy- pushed a commit that referenced this pull request Jun 5, 2023
@thetumbled @Technoboy-
[fix] [broker] do not filter system topic while shedding. (#18949)
76b6e30 
### Motivation

Currently, topics/bundles in `pulsar/system` will be filter while doing shedding, which is introduced by mistake by pr #15252.
But we need to unload topics/bundles in `pulsar/system` for load balancing. 

### Modifications

do not filter topics/bundles in `pulsar/system`.
@Technoboy- Technoboy- added this to the 3.1.0 milestone Jun 5, 2023
RobertIndie pushed a commit that referenced this pull request Jun 7, 2023
@thetumbled @RobertIndie
[fix] [broker] do not filter system topic while shedding. (#18949)
ec7b89e 
### Motivation

Currently, topics/bundles in `pulsar/system` will be filter while doing shedding, which is introduced by mistake by pr #15252.
But we need to unload topics/bundles in `pulsar/system` for load balancing.

### Modifications

do not filter topics/bundles in `pulsar/system`.

(cherry picked from commit 5c74d20)
Technoboy- pushed a commit that referenced this pull request Dec 4, 2023
@thetumbled @Technoboy-
[fix] [broker] do not filter system topic while shedding. (#18949)
5e6c39a 
### Motivation

Currently, topics/bundles in `pulsar/system` will be filter while doing shedding, which is introduced by mistake by pr #15252.
But we need to unload topics/bundles in `pulsar/system` for load balancing. 

### Modifications

do not filter topics/bundles in `pulsar/system`.
nodece pushed a commit to nodece/pulsar that referenced this pull request Feb 23, 2024
@thetumbled @nodece
[fix] [broker] do not filter system topic while shedding. (apache#18949)
d8d3561 
### Motivation

Currently, topics/bundles in `pulsar/system` will be filter while doing shedding, which is introduced by mistake by pr apache#15252.
But we need to unload topics/bundles in `pulsar/system` for load balancing. 

### Modifications

do not filter topics/bundles in `pulsar/system`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@codelipenghui codelipenghui codelipenghui left review comments

@eolivelli eolivelli eolivelli approved these changes

@poorbarcode poorbarcode poorbarcode approved these changes

@congbobo184 congbobo184 congbobo184 approved these changes

@liangyepianzhou liangyepianzhou liangyepianzhou approved these changes

@Technoboy- Technoboy- Awaiting requested review from Technoboy-

@gaoran10 gaoran10 Awaiting requested review from gaoran10

@mattisonchao mattisonchao Awaiting requested review from mattisonchao

Assignees

@thetumbled thetumbled

Labels

area/broker area/transaction cherry-picked/branch-2.10 cherry-picked/branch-2.11 cherry-picked/branch-3.0 doc-not-needed Your PR changes do not impact docs release/2.10.6 release/2.11.2 release/3.0.1

Projects

None yet

Milestone

3.1.0

Development

Successfully merging this pull request may close these issues.

9 participants

@thetumbled @codecov-commenter @eolivelli @codelipenghui @poorbarcode @congbobo184 @liangyepianzhou @Technoboy- @RobertIndie