[
https://issues.apache.org/jira/browse/HDDS-17738?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17738:
------------------------------
Attachment: MC-4-unordered-report-on-dependency-cycle.patch
> Ordered snapshot diff fails on every request when the changes contain a
> rename cycle, instead of returning an unordered report
> ------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17738
> URL: https://issues.apache.org/jira/browse/HDDS-17738
> Project: Apache Ozone
> Issue Type: Bug
> Affects Versions: 2.3.0
> Reporter: Siyao Meng
> Priority: Minor
> Attachments: MC-4-unordered-report-on-dependency-cycle.patch,
> TestBugMC4RenameCycleFailsOrderedJob.java
>
>
> h3. Mechanism
> For an ordered FSO snapshot diff,
> {{MergeJoinSnapDiffWriter.buildOrderedNodeIds}} sorts the entries with
> {{SnapDiffDependencyGraph}}. When the graph has a cycle it throws
> {{IOException("Dependency graph cycle for job ...")}} and the job ends
> FAILED. A FAILED job is always replaced on the next request, and the diff is
> deterministic, so every ordered request for the pair recomputes the whole
> diff and fails again.
> The same writer already handles the other case where it cannot order the
> report: when the entry count is over the in memory ordering limit it writes
> the entries unordered with {{flushUnorderedDependencyNodes}} and the job ends
> DONE with "Report is dependency unordered.". The writer fails the job on a
> cycle on purpose, but the reason is not stated, and it does not match the
> over the limit case, where the same writer cannot order the report either and
> returns it unordered.
> h3. Trigger
> Optimized snapshot diff enabled
> ({{ozone.om.snapshot.diff.optimized.enabled}}, default false), an FSO bucket,
> an ordered snapshot diff request served by the optimized diff (native SST
> diff available, or a full diff forced on the server or by the request), and
> changes between the snapshots that no order can replay. The simplest is two
> files that swap names through a temporary name: {{create a; create b;
> snapshot 1; mv a t; mv b a; mv t b; snapshot 2}}, which gives {{RENAME a ->
> b}} and {{RENAME b -> a}}.
> h3. Impact
> The ordered request never returns a report for that snapshot pair, and each
> retry recomputes the full diff. An unordered request for the same pair
> returns the report. No data is changed.
> h3. Reproduction
> [^TestBugMC4RenameCycleFailsOrderedJob.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 two ordered requests each create a
> new job that ends FAILED "Dependency graph cycle for job ...", and an
> unordered request then ends DONE "Report is dependency unordered." with
> {{RENAME a -> b, RENAME b -> a}}. A passing test means the defect is present.
> h3. Patch
> [^MC-4-unordered-report-on-dependency-cycle.patch], against
> d3b75c75bf9ad475551202477c3a21a3f4b968b7. It also applies to master at
> bbc8505e00d.
> {{buildOrderedNodeIds}} returns null on a cycle and logs a WARN with the
> unresolved entries, and {{orderAndWriteReport}} then writes all entries with
> the existing {{flushUnorderedDependencyNodes}} and reports that the result is
> not ordered. The job ends DONE with "Report is dependency unordered.", as for
> the over the limit case. The job keeps its ordered request flag, as the over
> the limit fallback does, so a later ordered request reuses the job instead of
> recomputing a diff that would fall back again. Other graph errors still fail
> the job. The job's ordered field records that the optimized diff attempted
> ordering; whether the report is ordered is in the reason.
> Covered by {{testDependencyOrderingFallsBackOnCycle}} in the existing
> {{TestMergeJoinSnapDiffWriter}}, next to
> {{testDependencyOrderingFallsBackWhenOverLimit}}: two files swap names, and
> the test expects no exception, an unordered result, both RENAME rows written
> and no ordered rows. Without the change it fails with "Dependency graph cycle
> for job job9"; with it {{TestSnapDiffDependencyGraph}},
> {{TestMergeJoinSnapDiffWriter}} and {{TestSnapshotDiffManager}} pass (122
> tests) and checkstyle is clean. With the patch the reproduction fails as
> intended: the first ordered job ends DONE "Report is dependency unordered."
> with {{RENAME a -> b, RENAME b -> a}}, and the second ordered request reuses
> it. {{TestOptimizedSnapshotDiff}} needs the native library and was not run.
> The fallback writes the entries in the same order as an unordered request,
> grouped by diff type, as the over the limit fallback does.
> Found by TLA+ model checking and code review of the OM optimized snapshot
> diff paths 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]
