Skip to content

[improve][broker] Part 2: Pack PendingAcksMap values into long#26030

Open
void-ptr974 wants to merge 2 commits into
apache:masterfrom
void-ptr974:pending-acks-pack-value-part2
Open

[improve][broker] Part 2: Pack PendingAcksMap values into long#26030
void-ptr974 wants to merge 2 commits into
apache:masterfrom
void-ptr974:pending-acks-pack-value-part2

Conversation

@void-ptr974

Copy link
Copy Markdown
Contributor

Motivation

Part of #26027.

PendingAcksMap stores per-entry pending ack metadata as an IntIntPair containing the remaining unacked count and sticky key hash. That keeps one value object per pending ack entry. The main consumer ack paths only need the remaining unacked count after lookup or removal, so returning an IntIntPair there also creates short-lived objects that are not needed by the caller.

Modifications

  • Store the pending ack value as a packed long inside the existing TreeMap<Long, TreeMap<Long, ...>> structure.
    • high 32 bits: remaining unacked count
    • low 32 bits: sticky key hash
  • Keep the existing ledger and entry ordering unchanged. This patch does not replace the inner map implementation.
  • Add primitive accessors for the consumer hot paths:
    • getRemainingUnacked
    • removeAndGetRemainingUnacked
  • Update Consumer to use the primitive accessors for ack owner lookup, full ack completion, and redelivery removal.
  • Keep the existing get and removeAndGet methods that return IntIntPair for compatibility with existing callers.
  • Add coverage for zero remaining count, signed sticky key hashes, integer boundaries, value-based removal, updateRemainingUnacked, and removeAllUpTo callbacks.

Verifying this change

  • ./gradlew :pulsar-broker:test --tests org.apache.pulsar.broker.service.PendingAcksMapTest --max-workers=1 -PtestRetryCount=0
  • ./gradlew :pulsar-broker:checkstyleMain :pulsar-broker:checkstyleTest --max-workers=1

@void-ptr974

void-ptr974 commented Jun 14, 2026

Copy link
Copy Markdown
Contributor Author

After this PR is merged, the next PR will switch the per-ledger entry map in PendingAcksMap to the primitive Long2LongOpenHashMap added in #26028.

That follow-up will keep the packed-value shape from this PR and focus only on removing the boxed inner-map key/value overhead. The BitSet prefix index for same-ledger removeAllUpTo cleanup will be a separate PR after that.

…alue-part2

# Conflicts:
#	pulsar-broker/src/main/java/org/apache/pulsar/broker/service/PendingAcksMap.java
#	pulsar-broker/src/test/java/org/apache/pulsar/broker/service/PendingAcksMapTest.java

@lhotari lhotari left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Related to #26028 (review). I have the fastutil minification working now and this change can be implemented on top of fastutil once the restoration of fastutil has been merged.

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.

3 participants