Skip to content

[fix][managed-ledger] Honor batch-index cap in isCursorDataFullyPersistable - #26553

Open
arimu1 wants to merge 1 commit into
apache:masterfrom
arimu1:fix/26499-isCursorDataFullyPersistable-batch-index
Open

[fix][managed-ledger] Honor batch-index cap in isCursorDataFullyPersistable#26553
arimu1 wants to merge 1 commit into
apache:masterfrom
arimu1:fix/26499-isCursorDataFullyPersistable-batch-index

Conversation

@arimu1

@arimu1 arimu1 commented Sep 12, 2026

Copy link
Copy Markdown

Fixes #26499

Motivation

ManagedCursorImpl.isCursorDataFullyPersistable() only compared the whole-entry
deletion range count to maxUnackedRangesToPersist. When partial batch ACK state
exceeded maxBatchDeletedIndexToPersist, the method still returned true even
though buildBatchEntryDeletionIndexInfoList() truncates on persist. That
mismatch can leave dispatchers running while some batch ACK records would not
survive a cursor-state write.

Modifications

  • Extend isCursorDataFullyPersistable() to return false when
    batchDeletedIndexes exceeds maxBatchDeletedIndexToPersist.
  • Add a managed-ledger test reproducing the issue scenario (three partial batch
    ACK records with limit 2).

No change to PIP-299 dispatcher pause policy beyond making the persistability
check accurate.

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

  • ./gradlew :managed-ledger:test --tests org.apache.bookkeeper.mledger.impl.ManagedCursorBatchAckRecoveryTest.testIsCursorDataFullyPersistableReflectsBatchDeletedIndexLimit

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

…stable

Also compare batchDeletedIndexes size to maxBatchDeletedIndexToPersist so
dispatchers see non-persistable ACK state when batch records exceed the limit.

Assisted-by: Cursor (Composer)
@arimu1
arimu1 force-pushed the fix/26499-isCursorDataFullyPersistable-batch-index branch from 8d00f5b to 6da992c Compare September 12, 2026 13:24
@void-ptr974

Copy link
Copy Markdown
Contributor

One coordination note for future contributions: I opened #26499 and indicated that I was willing to submit a PR, although I had not opened one yet. In this situation, it would be helpful to leave a short comment before starting work, so contributors can avoid duplicating effort. No problem this time—just please coordinate on the issue first in similar cases.

One change is required: the new org.awaitility.Awaitility import is out of order. I verified that ./gradlew :managed-ledger:checkstyleTest fails with an ImportOrder violation. Please move it after the org.apache... imports.

I did not find another blocker in the current diff.

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.

[Bug] isCursorDataFullyPersistable ignores the batch-index persistence limit

2 participants