Skip to content

Conversation

@lhotari
Copy link
Member

@lhotari lhotari commented Sep 25, 2025

Motivation

When brokers are exposed directly to clients, there might be a need to make the advertisedAddress absolute so that clients with unoptimal ndots configuration won't do unnecessary DNS lookups on the client side when an advertisedAddress with less dots than ndots is returned to the client from topic lookup.

Modifications

  • remove extra dot from brokerId so that the brokerId format doesn't change when advertisedAddress is made absolute
  • add tests for brokerId
  • add integration tests to ensure that client can connect when TLS is enabled. This ensures that TLS hostname validation doesn't fail with the extra trailing dot.

Documentation

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

@lhotari lhotari added this to the 4.2.0 milestone Sep 25, 2025
@lhotari lhotari requested review from dao-jun and nodece September 25, 2025 18:50
@lhotari lhotari self-assigned this Sep 25, 2025
@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Sep 25, 2025
Copy link
Member

@nodece nodece left a comment

Choose a reason for hiding this comment

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

LGTM

@codecov-commenter
Copy link

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.28%. Comparing base (676ba07) to head (9dbd600).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #24787       +/-   ##
=============================================
+ Coverage     38.56%   74.28%   +35.72%     
- Complexity    13262    33908    +20646     
=============================================
  Files          1856     1913       +57     
  Lines        145287   149502     +4215     
  Branches      16877    17371      +494     
=============================================
+ Hits          56025   111065    +55040     
+ Misses        81696    29552    -52144     
- Partials       7566     8885     +1319     
Flag Coverage Δ
inttests 26.12% <100.00%> (-0.06%) ⬇️
systests 22.76% <100.00%> (-0.01%) ⬇️
unittests 73.80% <100.00%> (+39.07%) ⬆️

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

Files with missing lines Coverage Δ
...n/java/org/apache/pulsar/broker/PulsarService.java 83.81% <100.00%> (+13.34%) ⬆️

... and 1409 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.

@lhotari lhotari merged commit a1ed5c4 into apache:master Nov 4, 2025
52 checks passed
lhotari added a commit that referenced this pull request Nov 4, 2025
…ress and remove extra dot from brokerId (#24787)

(cherry picked from commit a1ed5c4)
lhotari added a commit that referenced this pull request Nov 4, 2025
…ress and remove extra dot from brokerId (#24787)

(cherry picked from commit a1ed5c4)
ganesh-ctds pushed a commit to datastax/pulsar that referenced this pull request Nov 6, 2025
…ress and remove extra dot from brokerId (apache#24787)

(cherry picked from commit a1ed5c4)
(cherry picked from commit a3d9c53)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Nov 6, 2025
…ress and remove extra dot from brokerId (apache#24787)

(cherry picked from commit a1ed5c4)
(cherry picked from commit a3d9c53)
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