[fix][ml] Preserve cursor properties when recovering from an unreadable cursor ledger - #26512
[fix][ml] Preserve cursor properties when recovering from an unreadable cursor ledger#26512dao-jun wants to merge 2 commits into
Conversation
|
Thanks for the fix! I am wondering whether
On topic loading, Is there an existing guarantee that keeps |
lhotari
left a comment
There was a problem hiding this comment.
Preserving snapshot properties addresses the marker loss, but restoring a deleted compacted-ledger pointer can leave readCompacted reads failing. The rollover tests also need to wait for the intended snapshot before injecting recovery failures.
Fixes #26483
Motivation
ManagedCursorImpl#recoverFromLedger()has three error paths that rebuild the cursor fromthe metadata-store snapshot after failing to read the cursor ledger:
ledgerForceRecoveryenabled, any error)lastAddConfirmed < 0)initialize()does not treat that map as in-memory state — it persists it:persistPositionMetaStore()writes it into the durableManagedCursorInfo.propertieslist,so the properties are overwritten with an empty list. The position rollback itself is
defensible (
getRollbackPosition()returns the snapshotted mark-delete position clamped upto the first existing ledger), but the properties are not rolled back to an older value:
they are gone, which is not a rollback to a consistent earlier state.
These properties are load-bearing. Two in-tree consumers:
Compactor.COMPACTED_TOPIC_LEDGER_PROPERTYis the only durable pointer to a topic'scompacted ledger. If the key is absent after recovery,
PulsarCompactorSubscriptionnever registers the compacted ledger, so the topic serves uncompacted data until the next
successful compaction — and the previously compacted ledger stays unreferenced in
BookKeeper forever, since its only deletion site is the
previousContextreplacement.PersistentSubscription.REPLICATED_SUBSCRIPTION_PROPERTY: a replicated subscriptionsilently stops being treated as replicated after recovery.
The sibling recovery branch already does the right thing: when
cursorsLedgerId == -1,the same method decodes and restores the properties from the
ManagedCursorInfo. Only thecursor-ledger error paths drop them. An ERROR is logged about the ledger, but nothing
indicates that the compacted-topic pointer or the replicated-subscription marker was just
discarded, so the loss is invisible in the logs.
Modifications
In
ManagedCursorImpl#recoverFromLedger(), decode the properties from the sameManagedCursorInfosnapshot that provides the rollback position, and pass them toinitialize()in all three error paths instead ofCollections.emptyMap(). Position andproperties now come from the same snapshot, so the rollback is to a consistent earlier
state: if the last cursor-ledger entry carried newer values, they are rolled back together
with the position, which is safe — e.g. an older compacted-topic pointer references a
ledger that is only deleted once a newer pointer has been registered.
The decode is extracted into a small static
recoverProperties(count, accessor)helper(same shape as the existing
recoverIndividualDeletedMessages(count, accessor)); thecursorsLedgerId == -1branch and the successful-recovery path now use it too — a purelymechanical deduplication with no behavior change. When the snapshot contains no
properties, the helper returns
Collections.emptyMap(), exactly as before, soproperty-less cursors are unaffected.
Verifying this change
(Please pick either of the following options)
This change is a trivial rework / code cleanup without any test coverage.
(or)
This change is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)
Does this pull request potentially affect one of the following parts:
If the box was checked, please highlight the changes