[Bug] isCursorDataFullyPersistable ignores the batch-index persistence limit
- Dominant language
- Java
- Stars
- 15.3k
- Forks
- 3.8k
- Avg merge
- 1d 14h
- Merged PRs (30d)
- 160
Description
### Search before reporting
- [x] I searched in the [issues](https://github.com/apache/pulsar/issues) and found nothing similar.
### Read release policy
- [x] I understand that [unsupported versions](https://pulsar.apache.org/contribute/release-policy/#supported-versions) don't get bug fixes. I reproduced the issue on the `master` branch.
### User environment
- Version: `master` (`ce607988747`).
- Component: `managed-ledger`, `ManagedCursorImpl`.
### Issue Description
`ManagedCursorImpl.isCursorDataFullyPersistable()` can return `true` even when the configured batch-index persistence limit prevents some partial-batch ACK records from being saved. The method checks the whole-entry deletion-range limit, but not the separate batch-record limit.
#### Triggering scenario
A stored batch entry can contain several messages. If only some are acknowledged, the cursor keeps a bitmap of the messages still pending. The batch-index persistence limit counts one record per partially acknowledged batch entry, not messages or bits within a batch.
Consider three entries E1, E2 and E3, each containing an A message and a B message. Acknowledge each A message and leave each B message pending. The cursor has three batch ACK records and no whole-entry deletion ranges.
With `maxBatchDeletedIndexToPersist=2`, `isCursorDataFullyPersistable()` returns `true`, but an explicit cursor-state write saves only two of the three batch records. The same mismatch occurs when the whole-entry range collection is nonempty but within its own limit.
Expected: ACK state that exceeds the batch-record persistence limit should not be reported as fully persistable.
#### Impact
The batch persistence limit intentionally leaves excess ACK records in memory. If the cursor is recovered before the omitted state has been saved, affected messages may be delivered again. The issue here is not the configured truncation itself, but that the persistability check does not report it.
Both multiple-consumer dispatcher implementations consult this check when `dispatcherPauseOnAckStatePersistentEnabled` is enabled. Batch-record overflow alone leaves the result `true`, so it does not cause those checks to pause dispatching. Whether the existing pause policy should cover this additional limit is the policy question noted below.
### Reproducing the issue
These steps use the managed-ledger API on a durable `ManagedCursor`.
1. In `ManagedLedgerConfig`, set `deletionAtBatchIndexLevelEnabled=true`, `maxBatchDeletedIndexToPersist=2`, and `throttleMarkDelete=0`. The last setting prevents rate limiting from delaying the explicit cursor-state writes below.
2. Add an earlier entry E0 followed by batch entries E1, E2 and E3. Mark-delete E0 while the three batches remain unacknowledged, and wait for the cursor-state write to complete.
3. Individually acknowledge each A message, leaving each B message pending. There are now three batch ACK records in memory and no whole-entry deletion ranges.
4. Call `isCursorDataFullyPersistable()`. It returns `true`.
5. Mark-delete E0 again, without advancing the mark-delete position, and wait for completion. The latest stored cursor record (`PositionInfo`) contains only two batch ACK records.
Writing the same mark-delete position preserves all three partially acknowledged entries and checks serialization independently of the missing write trigger in #26498.
### Additional information
The corresponding broker configuration for the batch-record limit is `managedLedgerMaxBatchDeletedIndexToPersist`.
Relevant code: [persistability check](https://github.com/apache/pulsar/blob/ce6079887474043f4055ee6c22d2c72e456ac40f/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java#L411-L418), [batch-record serialization limit](https://github.com/apache/pulsar/blob/ce6079887474043f4055ee6c22d2c72e456ac40f/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java#L3441-L3494), and the dispatcher pause/resume checks ([current](https://github.com/apache/pulsar/blob/ce6079887474043f4055ee6c22d2c72e456ac40f/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumers.java#L1227-L1247), [classic](https://github.com/apache/pulsar/blob/ce6079887474043f4055ee6c22d2c72e456ac40f/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumersClassic.java#L1076-L1096)).
Policy scope to confirm: [PIP-299](https://github.com/apache/pulsar/blob/ce6079887474043f4055ee6c22d2c72e456ac40f/pip/pip-299.md#L48-L77) specifies pausing at the whole-entry range limit. Should the existing pause policy also cover batch-record overflow?
### Are you willing to submit a PR?
- [x] I'm willing to submit a PR!
Contributor guide
Research direction
Start in ManagedCursorImpl at isCursorDataFullyPersistable() and the batch-record serialization limit, then follow the current and classic multiple-consumer dispatcher checks. Reproduce the E1–E3 scenario with maxBatchDeletedIndexToPersist=2 and compare the persistability result with the saved PositionInfo; clarify the PIP-299 policy scope for batch-record overflow.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- backend, distributed-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100