[improve][broker] Use atomic counter for ongoing transaction count
Motivation
Currently, txnMetaMap.size() is used to determine if a new transaction can be added and for metrics reporting. However, txnMetaMap is a ConcurrentSkipListMap, and calling size() is an O(n) operation, which can consume significant system resources, especially under high transaction loads.
This PR replaces the use of txnMetaMap.size() with a dedicated atomic counter (onGoingTxnCount) to track the number of ongoing transactions in constant time (O(1)). This improves performance and reduces overhead in transaction management.
Modifications
- Introduced a
LongAdderfieldonGoingTxnCountinMLTransactionMetadataStoreto maintain the ongoing transaction count. - Updated all relevant methods to increment/decrement the counter when transactions are added or removed.
- Refactored test cases to use
getOnGoingTxnCount()instead of accessing the map via reflection, improving test clarity and maintainability. - Ensured metrics and coordinator stats now use the atomic counter instead of
txnMetaMap.size().
Verifying this change
- [x] Make sure that the change passes the CI checks.
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 - [x]
doc-not-needed - [ ]
doc-complete
Matching PR in forked repository
PR in forked repository: https://github.com/3pacccccc/pulsar/pull/34
Codecov Report
:x: Patch coverage is 84.61538% with 2 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 74.37%. Comparing base (270120c) to head (2b4c3c8).
:warning: Report is 8 commits behind head on master.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| ...n/coordinator/impl/MLTransactionMetadataStore.java | 84.61% | 0 Missing and 2 partials :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #25053 +/- ##
============================================
- Coverage 74.48% 74.37% -0.11%
+ Complexity 34186 34116 -70
============================================
Files 1921 1921
Lines 150376 150394 +18
Branches 17469 17472 +3
============================================
- Hits 112005 111857 -148
- Misses 29499 29619 +120
- Partials 8872 8918 +46
| Flag | Coverage Δ | |
|---|---|---|
| inttests | 26.46% <23.07%> (+0.11%) |
:arrow_up: |
| systests | 22.87% <0.00%> (-0.05%) |
:arrow_down: |
| unittests | 73.89% <84.61%> (-0.12%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
| Files with missing lines | Coverage Δ | |
|---|---|---|
| ...n/coordinator/impl/MLTransactionMetadataStore.java | 81.51% <84.61%> (-0.22%) |
:arrow_down: |
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
- :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.