[Bug] Cursor mark-delete falls back to the metadata store only when the cursor is caught up

[lhotari]

Search before reporting

• I searched in the issues and found nothing similar.

Read release policy

• I understand that unsupported 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:

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

Failed to mark delete position   (WARN, with attr position=<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:

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?

• I'm willing to submit a PR!