Skip to content

Conversation

@StevenLuMT
Copy link
Member

@StevenLuMT StevenLuMT commented Jun 26, 2025

Background

Upgrade bookkeeper to 4.17.2

Modifications

  1. [improve][broker] Upgrade bookkeeper to 4.17.2

  2. [improve][build] Migrate deprecated commons-configuration 1.x to commons-configuration 2.x, base on Migrate deprecated commons-configuration 1.x to commons-configuration 2.x bookkeeper#4604
    2.1. import change: org.apache.commons.configuration to org.apache.commons.configuration2
    2.2. impl change: setDelimiterParsingDisabled(true) to setListDelimiterHandler(new DisabledListDelimiterHandler())

  3. Enable ZooKeeper client to establish connection in read-only mode, base on Enable ZooKeeper client to establish connection in read-only mode bookkeeper#4244

  4. Upgrade gRPC to 1.72.0 : grpc version match to bookeeper 4.17.2, base on Upgrade to grpc 1.72.0 bookkeeper#4591

Verifying this change

  • Make sure that the change passes the CI checks.
    This change is a trivial rework / code cleanup without any test coverage.

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

Check the box below or label this PR directly.

Need to update docs?

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

Matching PR in forked repository

PR in forked repository: StevenLuMT#15

@github-actions github-actions bot added the doc-not-needed Your PR changes do not impact docs label Jun 26, 2025
@StevenLuMT StevenLuMT force-pushed the master-upgradeBookie4.17.2 branch from 605094a to 15b1d95 Compare June 27, 2025 01:16
@StevenLuMT StevenLuMT force-pushed the master-upgradeBookie4.17.2 branch 5 times, most recently from 110a3e9 to a78328e Compare June 28, 2025 23:44
@StevenLuMT StevenLuMT marked this pull request as ready for review June 29, 2025 01:58
@StevenLuMT StevenLuMT force-pushed the master-upgradeBookie4.17.2 branch 4 times, most recently from 9f63ce6 to 9592a84 Compare July 1, 2025 02:10
@codecov-commenter
Copy link

codecov-commenter commented Jul 1, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.25%. Comparing base (bbc6224) to head (2d025b0).
⚠️ Report is 1250 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #24468      +/-   ##
============================================
+ Coverage     73.57%   74.25%   +0.67%     
+ Complexity    32624    32411     -213     
============================================
  Files          1877     1868       -9     
  Lines        139502   145762    +6260     
  Branches      15299    16701    +1402     
============================================
+ Hits         102638   108232    +5594     
- Misses        28908    28943      +35     
- Partials       7956     8587     +631     
Flag Coverage Δ
inttests 26.66% <25.00%> (+2.07%) ⬆️
systests 23.32% <25.00%> (-1.00%) ⬇️
unittests 73.73% <100.00%> (+0.88%) ⬆️

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

Files with missing lines Coverage Δ
...ookie/rackawareness/BookieRackAffinityMapping.java 78.08% <ø> (-1.36%) ⬇️
...in/java/org/apache/pulsar/PulsarBrokerStarter.java 43.19% <ø> (+6.06%) ⬆️
.../org/apache/pulsar/PulsarClusterMetadataSetup.java 72.43% <100.00%> (-0.09%) ⬇️
...ache/pulsar/broker/ManagedLedgerClientFactory.java 82.65% <ø> (-1.50%) ⬇️
.../prometheus/metrics/PrometheusMetricsProvider.java 71.87% <ø> (ø)
...ache/pulsar/zookeeper/LocalBookkeeperEnsemble.java 72.20% <ø> (-0.81%) ⬇️
...he/pulsar/metadata/impl/PulsarZooKeeperClient.java 44.61% <100.00%> (+2.89%) ⬆️

... and 1085 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
Copy link
Member

lhotari commented Jul 1, 2025

Good work! However, I think we can split the common-lang -> commons-lang3 migration to a separate PR. In that separate PR it would also be useful to add check style rules that prevent using commons-lang for new code.

@StevenLuMT
Copy link
Member Author

Good work! However, I think we can split the common-lang -> commons-lang3 migration to a separate PR. In that separate PR it would also be useful to add check style rules that prevent using commons-lang for new code.

@lhotari Good suggestion, I will split this out and submit a separate PR

@StevenLuMT
Copy link
Member Author

Good work! However, I think we can split the common-lang -> commons-lang3 migration to a separate PR. In that separate PR it would also be useful to add check style rules that prevent using commons-lang for new code.

@lhotari Good suggestion, I will split this out and submit a separate PR

I have proposed a new one. After #24473 is merged, I will delete the duplicate code, rebase the master and push it again.

@StevenLuMT StevenLuMT force-pushed the master-upgradeBookie4.17.2 branch from 9592a84 to dee7dee Compare July 2, 2025 06:20
@StevenLuMT StevenLuMT force-pushed the master-upgradeBookie4.17.2 branch from dee7dee to 5902d5f Compare July 3, 2025 01:34
@StevenLuMT StevenLuMT force-pushed the master-upgradeBookie4.17.2 branch from 5902d5f to 2d025b0 Compare July 4, 2025 06:23
@StevenLuMT
Copy link
Member Author

/pulsarbot run-failure-checks

@codelipenghui codelipenghui added this to the 4.1.0 milestone Jul 7, 2025
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

LGTM

Please make it clear in the title that we are also updating other libraries

It would be better to split the patch, but I understand it is not possible

@StevenLuMT StevenLuMT changed the title [improve][broker] Upgrade bookkeeper to 4.17.2 [improve][broker] Upgrade bookkeeper to 4.17.2/commons-configuration to 2.x/grpc to 1.72.0 Jul 7, 2025
@StevenLuMT StevenLuMT changed the title [improve][broker] Upgrade bookkeeper to 4.17.2/commons-configuration to 2.x/grpc to 1.72.0 [improve][broker] Upgrade bookkeeper to 4.17.2/commons-configuration to 2.x/grpc to 1.72.0 and enable ZooKeeper client to establish connection in read-only mode Jul 7, 2025
@frankjkelly
Copy link
Contributor

Will this be backported to 3.3.X stream? If so can it receive those labels? Thanks!

@StevenLuMT
Copy link
Member Author

Will this be backported to 3.3.X stream? If so can it receive those labels? Thanks!
Can the 3.3.x label be added directly, or do I need to initiate a vote? @lhotari @codelipenghui

@codelipenghui codelipenghui merged commit 8d1ec6d into apache:master Jul 8, 2025
100 of 104 checks passed
@StevenLuMT
Copy link
Member Author

Will this be backported to 3.3.X stream? If so can it receive those labels? Thanks!

Finished: two labels have been added, release/3.3.8 and release/4.0.6
Thanks @codelipenghui @frankjkelly

@StevenLuMT
Copy link
Member Author

StevenLuMT commented Jul 8, 2025

After bookeeper is upgraded, all org.apache.commons.lang references are removed, so the PR #24473 is also given the same label. Thanks @codelipenghui

codelipenghui pushed a commit to codelipenghui/incubator-pulsar that referenced this pull request Jul 15, 2025
…to 2.x/grpc to 1.72.0 and enable ZooKeeper client to establish connection in read-only mode (apache#24468)
BewareMyPower pushed a commit that referenced this pull request Jul 17, 2025
…to 2.x/grpc to 1.72.0 and enable ZooKeeper client to establish connection in read-only mode (#24468)

(cherry picked from commit 8d1ec6d)
lhotari pushed a commit that referenced this pull request Jul 17, 2025
…to 2.x/grpc to 1.72.0 and enable ZooKeeper client to establish connection in read-only mode (#24468)

(cherry picked from commit 8d1ec6d)
priyanshu-ctds pushed a commit to datastax/pulsar that referenced this pull request Jul 22, 2025
…to 2.x/grpc to 1.72.0 and enable ZooKeeper client to establish connection in read-only mode (apache#24468)

(cherry picked from commit 8d1ec6d)
(cherry picked from commit 22ff023)
srinath-ctds pushed a commit to datastax/pulsar that referenced this pull request Jul 24, 2025
…to 2.x/grpc to 1.72.0 and enable ZooKeeper client to establish connection in read-only mode (apache#24468)

(cherry picked from commit 8d1ec6d)
(cherry picked from commit 22ff023)
KannarFr pushed a commit to CleverCloud/pulsar that referenced this pull request Sep 22, 2025
…to 2.x/grpc to 1.72.0 and enable ZooKeeper client to establish connection in read-only mode (apache#24468)
walkinggo pushed a commit to walkinggo/pulsar that referenced this pull request Oct 8, 2025
…to 2.x/grpc to 1.72.0 and enable ZooKeeper client to establish connection in read-only mode (apache#24468)
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.

9 participants