Skip to content

[improve][ml] Avoid cursor lock for read scheduling hints - #296

Open
lhotari wants to merge 1 commit into
lh-improve-ack-owner-allocationfrom
lh-perfopt-cursor-deletion-lookup
Open

lhotari wants to merge 1 commit into
lh-improve-ack-owner-allocationfrom
lh-perfopt-cursor-deletion-lookup

Conversation

@lhotari

@lhotari lhotari commented Sep 19, 2026

Copy link
Copy Markdown
Owner

Motivation

Managed-ledger read scheduling checks every candidate position against the cursor mark-delete position and individual-deletion ranges. Although the check is only a hint before I/O, it currently acquires the cursor read lock for every position. Grouped acknowledgments hold the same cursor's write lock while applying a list, so active consumers and ACK processing contend on the outer cursor lock.

Holding the read lock across a larger scan is not a good alternative because it delays ACK writers. The scheduling check can instead tolerate an acknowledgment racing with it: the existing post-I/O filter checks the authoritative deletion state before entries are returned. A stale-negative scheduling result therefore causes only an extra read.

Modifications

  • Publish a read-only primitive ledger-to-bitmap directory from PositionRangeSet when ledger membership changes.
  • Query the existing thread-safe per-ledger bitmap without acquiring the cursor lock for the normal read-scheduling predicate.
  • Keep the existing locked post-I/O filtering as the correctness boundary.
  • Guard cursor reset with an epoch because reset can move mark-delete backward. A lookup overlapping reset falls back to the cursor lock.
  • Preserve the virtual locked deletion check for cursor subclasses.
  • Extend the read-skip microbenchmark and add lookup publication/invalidation tests.

The client already groups acknowledgments and the broker applies each received list under one cursor writer critical section. This change lets scheduling readers continue without contending on that outer lock while retaining the bitmap's own short read lock.

Performance

A short JMH screen measured the scheduling lookup as follows:

Individual-deletion ranges Existing primitive lookup Published-directory lookup Change
0 16.598 ns/op 1.897 ns/op -88.6%
128 28.394 ns/op 18.081 ns/op -36.3%
10,000 27.641 ns/op 15.190 ns/op -45.0%

In an end-to-end scenario with 500 producer clients, one topic, and twenty Key_Shared subscriptions with ten isolated consumers each, two interleaved parent/candidate pairs produced these ingress rates:

Pair Parent Change Improvement
1 30,053 msg/s 32,329 msg/s +7.6%
2 31,545 msg/s 33,955 msg/s +7.6%

Each run delivered all 20 million fanout messages with zero duplicates, ordering violations, or invalid messages.

A separate one-subscription/50-consumer Key_Shared profile pair concentrated scheduling and ACK activity on one cursor. Wall-clock sampling was disabled:

Recording Parent Change Improvement
Broker CPU/allocation/lock profile 72,086 msg/s 76,799 msg/s +6.5%

Both profile runs delivered all two million messages with zero duplicates, ordering violations, or invalid messages. An unprofiled pair differed by -2.4%, within the scenario's run variance, so the profile result is supporting evidence rather than a precise standalone throughput claim.

Verifying this change

This change added tests and was verified with:

  • ./gradlew :managed-ledger:test --tests org.apache.bookkeeper.mledger.impl.ManagedCursorTest --tests org.apache.bookkeeper.mledger.impl.PositionRangeSetTest
  • ./gradlew quickCheck
  • ./gradlew spotlessCheck checkstyleMain checkstyleTest
  • ReadSkipPredicateBenchmark
  • the end-to-end IoT Key_Shared scenario described above

Does this pull request potentially affect one of the following parts:

  • 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant