[
https://issues.apache.org/jira/browse/HDDS-17742?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17742:
------------------------------
Attachment: CR-4-gate-full-diff-on-snapshot-create-transaction.patch
> 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
> Affects Versions: 2.3.0
> Reporter: Siyao Meng
> Priority: Major
> 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]
