[Bug] Cursor mark-delete falls back to the metadata store only when the cursor is caught up
- 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 current `master` branch.
### User environment
- Broker version: 4.2.4
- Present on `master` (`9ba61bd95de`); line numbers below are `master`.
### Issue Description
#20935 ("Persist mark deleted ops to ZK if create cursor ledger was failed", commit `843b8307f44`) added a metadata-store fallback for the case where a cursor ledger cannot be created. Its stated motivation lists the trigger explicitly:
> There are two case may cause switch ledger fails.
> 1. No enough BKs; BKs are in read-only mode...
> 2. Write ZK fails.
The fallback is gated so that it does not actually apply to a cursor that is behind the tip:
```java
if (state == State.NoLedger) {
if (ledger.isNoMessagesAfterPos(mdEntry.newPosition)) {
log.error("[{}][{}] Metadata ledger creation failed, try to persist the position in the metadata store.", ...);
persistPositionToMetaStore(mdEntry, cb);
} else {
cb.operationFailed(new ManagedLedgerException("Switch new cursor ledger failed"));
}
} else {
persistPositionToLedger(cursorLedger, mdEntry, cb, false);
}
```
`ManagedCursorImpl.java:2501-2510`, and `ManagedLedgerImpl.isNoMessagesAfterPos(pos)` is `pos >= LAC` (`ManagedLedgerImpl.java:4166-4169`).
So when BookKeeper cannot give the cursor a new ledger, the position is persisted to the metadata store **only if the cursor happens to be caught up to the last confirmed entry**. Any cursor with a backlog — and the `__compaction` cursor on a live topic, always — takes the `else` branch and the position is discarded, even though the metadata store is perfectly healthy. That is the opposite of what the change set out to do.
Note the asymmetry with the sibling path: an *add* failure on an **existing** cursor ledger falls back to the metadata store **unconditionally** (`ManagedCursorImpl.java:3548-3553`). Only the "could not create a new cursor ledger" path is gated.
The guard is presumably protecting individually-deleted ranges, which live only in the cursor ledger and are not written to the metadata store on this path. That is a real concern — but a cursor with no such ranges has nothing to lose, and the compaction cursor never has any (it acknowledges cumulatively).
### Error messages
```text
Failed to mark delete position (WARN, with attr position=)
```
from the `operationFailed` callback in `internalMarkDelete` (`ManagedCursorImpl.java:2481-2490`), reached via `ManagedLedgerException("Switch new cursor ledger failed")`.
### Reproducing the issue
Analysis is from code; a managed-ledger unit test can drive it:
1. Open a cursor and let it fall behind the managed ledger's last confirmed entry.
2. Make cursor-ledger creation fail (e.g. no writable bookies) so the cursor enters `State.NoLedger`.
3. Mark-delete on that cursor.
4. Observe that `persistPositionToMetaStore()` is never called and the callback fails with `Switch new cursor ledger failed`, even though the metadata store is available.
5. Repeat with the cursor caught up to LAC and observe the metadata-store fallback working as intended.
### Additional information
Suggested fix — take the fallback whenever there is nothing in the cursor ledger that the metadata store would not capture:
```java
if (ledger.isNoMessagesAfterPos(mdEntry.newPosition)
|| getTotalNonContiguousDeletedMessagesRange() == 0) {
persistPositionToMetaStore(mdEntry, cb);
} else {
cb.operationFailed(new ManagedLedgerException("Switch new cursor ledger failed"));
}
```
`isCompactionCursor()` already exists in the class if an explicit exemption is preferred.
### Are you willing to submit a PR?
- [X] I'm willing to submit a PR!
Contributor guide
Assessment
This issue has not been assessed yet.