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] Restore the broker id to match the format used in existing Pulsar releases #21937

Conversation

lhotari
Copy link
Member

@lhotari lhotari commented Jan 20, 2024

Motivation

The "lookup service address" format was changed in PR #21015 based on an invalid issue report #21012.
"lookup service address" has been renamed to "brokerId" in #21894 to reduce confusion.
The value is not a service address to be used by any connections to the broker. It is used in the Pulsar load manager as
a unique runtime identifier for a broker instance.

It's better to not change the broker id format since there isn't a reason to do this. The format used in existing Pulsar releases should be restored and that's the motivation for this PR.

There was a misconception that PR #17136 would have changed the format in Pulsar 2.11 release. That PR didn't change the format. It can be verified from the diff.

Modifications

  • restore the broker id format to match the format used in existing Pulsar releases
  • fix tests that are using incorrect ways to find the broker id for a broker instance

Documentation

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

@lhotari lhotari added the release/blocker Indicate the PR or issue that should block the release until it gets resolved label Jan 20, 2024
@lhotari lhotari added this to the 3.2.0 milestone Jan 20, 2024
@lhotari lhotari self-assigned this Jan 20, 2024
@lhotari lhotari changed the title [fix][broker] Restore the broker id format used in existing Pulsar releases [fix][broker] Restore the broker id to match the format used in existing Pulsar releases Jan 20, 2024
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jan 20, 2024
@codecov-commenter
Copy link

Codecov Report

Attention: 1 lines in your changes are missing coverage. Please review.

Comparison is base (5cb12a5) 73.76% compared to head (9fbef92) 73.59%.
Report is 2 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #21937      +/-   ##
============================================
- Coverage     73.76%   73.59%   -0.17%     
+ Complexity    32476    32411      -65     
============================================
  Files          1861     1861              
  Lines        138605   138677      +72     
  Branches      15186    15187       +1     
============================================
- Hits         102238   102066     -172     
- Misses        28505    28711     +206     
- Partials       7862     7900      +38     
Flag Coverage Δ
inttests 24.09% <33.33%> (-0.11%) ⬇️
systests 23.67% <33.33%> (-0.08%) ⬇️
unittests 72.86% <83.33%> (-0.18%) ⬇️

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

Files Coverage Δ
...n/java/org/apache/pulsar/broker/PulsarService.java 82.29% <100.00%> (-1.77%) ⬇️
...ache/pulsar/client/admin/internal/BrokersImpl.java 87.50% <100.00%> (ø)
...g/apache/pulsar/broker/admin/impl/BrokersBase.java 70.86% <50.00%> (ø)

... and 87 files with indirect coverage changes

@merlimat merlimat merged commit 6347315 into apache:master Jan 21, 2024
47 checks passed
lhotari added a commit that referenced this pull request Jan 21, 2024
lhotari added a commit that referenced this pull request Jan 26, 2024
lhotari added a commit that referenced this pull request Jan 26, 2024
mukesh-ctds pushed a commit to datastax/pulsar that referenced this pull request Mar 1, 2024
…ing Pulsar releases (apache#21937)

(cherry picked from commit 6347315)
(cherry picked from commit 89f722f)
mukesh-ctds pushed a commit to datastax/pulsar that referenced this pull request Mar 6, 2024
…ing Pulsar releases (apache#21937)

(cherry picked from commit 6347315)
(cherry picked from commit 89f722f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-picked/branch-3.0 cherry-picked/branch-3.1 doc-not-needed Your PR changes do not impact docs ready-to-test release/blocker Indicate the PR or issue that should block the release until it gets resolved release/3.0.3 release/3.1.3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants