Siyao Meng created HDDS-17742:
---------------------------------
Summary: Optimized full snapshot diff omits changes once deep
cleaning or a snapshot purge advances the from snapshot's lastTransactionInfo
Key: HDDS-17742
URL: https://issues.apache.org/jira/browse/HDDS-17742
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments: CR-4-gate-full-diff-on-snapshot-create-transaction.patch,
TestBugCR4FullDiffGateAfterPurge.java
h3. Mechanism
In full diff mode {{SnapshotDiffManager.resolveFullDiffUpdateIdGate}} takes the
transaction index of the from snapshot's {{lastTransactionInfo}}, and
{{FullDiffSequentialReader}} treats every to side row whose updateID is at or
below it as unchanged; {{MergeJoinSnapDiffWriter}} then emits nothing for it.
{{lastTransactionInfo}} does not stay at the creation index:
{{OMKeyPurgeRequest}} and {{OMDirectoriesPurgeRequestWithFSO}} (deep cleaning
of the snapshot), {{OMSnapshotMoveUtils}} and {{OMSnapshotPurgeRequest}} (for
the snapshots after a purged one) rewrite it later. Changes made after the from
snapshot was created but before such a rewrite then fall under the gate.
{{createTransactionInfo}} records the creation index and is never rewritten.
The gate also assumes that the from snapshot is the older one. Nothing enforces
that, and in a diff from a newer to an older snapshot every to side row is at
or below the from snapshot's index, so renames, creates and modifications are
dropped there too, while the legacy diff reports them.
h3. Trigger
{{ozone.om.snapshot.diff.optimized.enabled=true}} and a full diff: forced by
the client or by {{ozone.om.snapshot.force.full.diff}}, or, with the native
library loaded, the automatic fallback when the compaction DAG cannot serve the
diff. Then either:
* the from snapshot's {{lastTransactionInfo}} has moved: deep cleaning
({{ozone.snapshot.deep.cleaning.enabled}}, on by default) purges anything from
the snapshot, or an older snapshot is deleted and purged. Example run: create
{{/d/k1}}, take snapshots snap0 and snap1, rename {{/d/k1}} to {{/d/k1r}},
create {{/d/k2}}, take snapshot snap2, delete snap0 and let
{{SnapshotDeletingService}} purge it, then request a full diff snap1 to snap2;
* or the diff goes from a newer to an older snapshot, for example snap2 to
snap1.
h3. Impact
Deep cleaning moves the gate without any snapshot being deleted, so a from
snapshot that held reclaimable deleted keys when it was created is enough
(argued from the code; the purge route was run). In the purge example the DONE
report is empty instead of {{[RENAME d/k1 -> d/k1r, CREATE d/k2]}}, for ordered
and unordered reports, while the same diff before the purge is correct. In a
reverse diff the report keeps only the deletes ({{[DELETE k2]}} instead of
{{[RENAME k1r -> k1, DELETE k2]}}). The report is persisted and returned for
every later read of the pair.
h3. Reproduction
[^TestBugCR4FullDiffGateAfterPurge.java] ({{hadoop-ozone/integration-test}}, no
native library needed) runs the purge example on a mini cluster through the
FileSystem, snapshot and diff APIs, with the real {{SnapshotDeletingService}}
run once instead of on its interval. A control diff before the purge reports
the rename and the create. After the purge the unfixed code fails with
{{Expecting actual: [] to contain exactly in any order: [R ./d/k1 -> ./d/k1r, +
./d/k2]}}; with the patch it passes. The reverse case is covered by the
regression test below.
h3. Patch
[^CR-4-gate-full-diff-on-snapshot-create-transaction.patch], against
d3b75c75bf9ad475551202477c3a21a3f4b968b7. It also applies to master at
bbc8505e00d.
The gate now uses the from snapshot's {{createTransactionInfo}}, and only when
both snapshots have it and the from index is lower than the to index. Otherwise
the gate is disabled and every row is compared, which is correct but slower:
snapshots created before 2.1.0 have no {{createTransactionInfo}}, and a reverse
diff has no rows after the from snapshot's creation. It does not fall back to
{{lastTransactionInfo}}. The {{FullDiffSequentialReader}} Javadoc of the gate
is updated to match.
Covered by two tests in the existing {{TestOptimizedSnapshotDiff}} (native
suite), using a new overload of the {{OmSnapshotTests}} helper that takes the
force full diff flag: {{testForcedFullDiffAfterSnapshotPurge}} (purge example,
also asserts that the purge moved {{lastTransactionInfo}} past
{{createTransactionInfo}}) and {{testForcedFullDiffFromNewerToOlderSnapshot}}
(reverse diff). On the unfixed code the first gets an empty report and the
second {{[- ./reverse/k2]}}. With the patch {{TestOptimizedSnapshotDiff}}, the
snapshot diff unit suites and {{TestSnapshotDiffManager}} (191 tests) pass, and
checkstyle is clean.
Found by code review of the OM optimized snapshot diff paths, as part of the
TLA+ verification effort under HDDS-15926, on commit
d3b75c75bf9ad475551202477c3a21a3f4b968b7. Checked against HDDS issues and
apache/ozone pull requests for duplicates before filing. The attached patch is
a proposal for review. Generated with Specula (Claude Opus 5.5).
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
