[
https://issues.apache.org/jira/browse/HDDS-17734?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17734:
------------------------------
Attachment: CR-1-obs-report-not-dependency-ordered.patch
> Optimized snapshot diff of an OBS bucket is labelled "dependency ordered"
> although its entries are not
> ------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17734
> URL: https://issues.apache.org/jira/browse/HDDS-17734
> Project: Apache Ozone
> Issue Type: Bug
> Affects Versions: 2.3.0
> Reporter: Siyao Meng
> Priority: Minor
> Attachments: CR-1-obs-report-not-dependency-ordered.patch,
> TestBugCR1ObsReportLabeledOrdered.java
>
>
> h3. Mechanism
> For OBS (and LEGACY) buckets, that is every bucket that is not FSO,
> {{MergeJoinSnapDiffWriter.writeReport}} writes each entry as the merge join
> produces it. The report key orders the entries by diff type (DELETE, MODIFY,
> RENAME, CREATE), then in merge join (object ID) order, and there is no
> dependency ordering step (the {{writeReport}} Javadoc, the {{--ordered}} help
> and the {{ObjectStore.submitSnapshotDiff}} Javadoc all say ordering applies
> to FSO buckets). The OBS branch still sets the ordered outcome to true
> unconditionally, so {{SnapshotDiffManager}} finishes every optimized OBS job,
> ordered request or not, with the reason "Report is dependency ordered.".
> The label was set on purpose: the diff type order puts deletes before renames
> and creates, and renames before creates, which meets the path conflict rules
> the original design relied on for OBS buckets, and
> {{TestOptimizedSnapshotDiff#testOptimizedObsDiff}} expected "ordered" even
> for an unordered request. It does not order a rename into a key that another
> rename frees, and the OBS path has no equivalent of the FSO graph rule for
> that case ("RENAME(source) before RENAME(target) that reuses the same path"
> in {{SnapDiffDependencyGraph}}).
> h3. Trigger
> Optimized snapshot diff enabled
> ({{ozone.om.snapshot.diff.optimized.enabled}}, default false) and an OBS
> bucket. Create key {{a}}, then key {{b}}, take snapshot 1, rename {{b}} to
> {{c}}, then {{a}} to {{b}}, take snapshot 2, and request a snapshot diff.
> h3. Impact
> The report is {{RENAME a -> b, RENAME b -> c}} with the reason "Report is
> dependency ordered.". Applying it in that order to the keys of snapshot 1
> fails at the first entry with KEY_ALREADY_EXISTS, because {{b}} still exists.
> A client that trusts the label to replay the report fails or has to reorder
> it itself. No data in the bucket is changed.
> h3. Reproduction
> [^TestBugCR1ObsReportLabeledOrdered.java] is a self contained mini cluster
> class (target path and Maven command in the class comment) that enables the
> optimized diff with a forced full diff and native diff disabled and uses only
> public client calls. On unmodified source the ordered request ends DONE
> "Report is dependency ordered." with {{RENAME a -> b, RENAME b -> c}}, and
> renaming the keys of a second OBS bucket that holds the snapshot 1 keys in
> report order fails at {{RENAME a -> b}} with KEY_ALREADY_EXISTS. A passing
> test means the defect is present.
> h3. Patch
> [^CR-1-obs-report-not-dependency-ordered.patch], against
> d3b75c75bf9ad475551202477c3a21a3f4b968b7. It also applies to master at
> bbc8505e00d.
> The OBS branch now reports the outcome as not ordered, so optimized OBS jobs
> end with "Report is dependency unordered.", the reason FSO already uses when
> it cannot order a report. The entry order and the request handling are
> unchanged. The new label is conservative on purpose: a report without
> renames, whose type order already replays, is now labelled unordered too. An
> exact label (ordered unless a rename targets another rename's source) would
> need a rename path index that grows with the report, and a swap of two keys
> cannot be ordered anyway. Ordering OBS reports for real (a path dependency
> graph for renames, deletes and creates) would be a feature and is not
> attempted.
> Covered by {{testObsReportIsNotDependencyOrdered}} in the existing
> {{TestMergeJoinSnapDiffWriter}}, which writes the rename chain above and
> expects the rows in object ID order and an unordered result.
> {{TestOptimizedSnapshotDiff#testOptimizedObsDiff}} asserted the ordered
> reason for an OBS diff and now expects "Report is dependency unordered.";
> that class needs the native library and was compiled but not run. Without the
> change the new test fails with "expected: <false> but was: <true>"; with it
> {{TestSnapDiffDependencyGraph}}, {{TestMergeJoinSnapDiffWriter}} and
> {{TestSnapshotDiffManager}} pass (122 tests) and checkstyle is clean. With
> the patch the reproduction fails as intended, with the reason "Report is
> dependency unordered.".
> 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]
