Siyao Meng created HDDS-17738:
---------------------------------
Summary: 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
Reporter: Siyao Meng
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]