Ritesh Shukla created HDDS-16092:
------------------------------------
Summary: takeSnapshotImpl's unbatched TransactionInfo put can
regress the persisted transaction index below the DB's content
Key: HDDS-16092
URL: https://issues.apache.org/jira/browse/HDDS-16092
Project: Apache Ozone
Issue Type: Bug
Components: Ozone Manager
Reporter: Ritesh Shukla
h2. Summary
{{OzoneManagerStateMachine.takeSnapshotImpl}} writes {{TRANSACTION_INFO_KEY}}
with a direct, unbatched {{put}} while the double buffer writes the same key
inside its atomic batch. A snapshot that lands in the gap between the batch
commit and the applied-index advance computes its index from a stale value and
overwrites the batch's higher {{TransactionInfo}} with a lower one, leaving the
DB holding data for transactions its own watermark disclaims.
h2. The two writers
* Double buffer: {{putWithBatch}} at {{OzoneManagerDoubleBuffer.java:375-376}},
committed together with the transaction data at {{:379-381}}. The state
machine's applied index advances only *afterwards*, at {{:396}}
({{updateLastAppliedIndex.accept}}, wired to
{{OzoneManagerStateMachine::updateLastAppliedTermIndex}} at
{{OzoneManagerStateMachine.java:562}}).
* Snapshot: direct {{put}} at {{OzoneManagerStateMachine.java:604}}, using the
index computed at {{:599}} as {{max(applied, notified)}}.
h2. Interleaving
# *OMDoubleBufferFlushThread* commits transactions 101..105. The DB now
atomically holds their data and {{TransactionInfo = 105}}. It proceeds toward
{{accept(105)}}.
# *Ratis StateMachineUpdater* enters {{takeSnapshotImpl}} first. That method is
{{synchronized}} ({{:596}}) and so is {{updateLastAppliedTermIndex}} ({{:262}})
-- the same monitor -- so step 1's {{accept(105)}} blocks. The window is
therefore held open across the put and the {{flushDB}} that follows, rather
than being a momentary read blip.
# It reads {{applied = 100}}, computes {{snapshot = 100}}, and at {{:604}} puts
{{TransactionInfo = 100}}. That write is sequenced after the already-committed
batch, so *100 overwrites 105*.
End state: the DB contains the effects of 101..105 under a watermark of 100.
Ratis's snapshot index is also 100, so the log purge stops there and entries
101..105 survive in the raft log. On an idle cluster the inconsistency persists
until the next flush.
Note the asymmetry: {{updateLastAppliedTermIndex}} guards the *in-memory* index
against moving backwards ({{assertUpdateIncreasingly}}, {{:264}}). The
persisted twin has no equivalent guard, which is exactly the direction this
moves.
h2. Consequence
On restart, {{loadSnapshotInfoFromDB}} seeds the applied index at 100 and Ratis
replays 101..105 through full request execution, with no replay suppression at
any layer.
This is *not* silent data corruption. If any object in the window was touched
more than once, {{WithObjectID.Builder.validate()}} throws
{{IllegalArgumentException}} on the updateID regression before anything is
persisted ({{WithObjectID.java:121-131}}), and {{runCommand}}'s {{catch
(Throwable)}} terminates the OM. The reachable outcome is a deterministic
startup crash-loop -- persisting until manual repair on a single-node
deployment, or until the leader's log purge passes the window on an HA node,
after which restart heals via install-snapshot. If no object in the window was
multi-touched, the replay is benign and the next flush silently heals the
watermark.
h2. Trigger
The auto-snapshot path, enabled unconditionally at
{{OzoneManagerRatisServer.java:869}} with the threshold from
{{ozone.om.ratis.snapshot.auto.trigger.threshold}}.
h2. Suggested fix
Make the snapshot's write unable to lower the stored value: either skip the put
when the persisted index already exceeds the snapshot index, or route it
through the same batch/commit path the flush uses so the two writers are
ordered rather than racing.
h2. Provenance and caveats
Found while reviewing HDDS-16086 / [PR
#10943|https://github.com/apache/ozone/pull/10943], which touches the other
writer of this field. Line references are against master at commit
{{8415f3b876}}.
Every load-bearing citation above was read directly. The one assumption not
re-derived from source is standard RocksDB write ordering in step 3 -- that a
{{put}} issued after {{commitBatchOperation}} returns is sequenced after that
batch. No reproducer has been written yet; the interleaving is derived from the
code, not observed.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
