[SaketaChalamchala]

Found by TLA+ model checking and code review of the OM optimized snapshot diff paths under HDDS-15926, on commit d3b75c7.
The patch is generated with Specula (Claude Opus 5.5).

What changes were proposed in this pull request?

For optimized FSO snapshot diff with dependency ordering enabled, a cyclic dependency graph (e.g. two bucket-root files swapping names) used to fail report generation with IOException (Dependency graph cycle for job …). The job ended FAILED, and each subsequent ordered request for the same snapshot pair recomputed the diff and failed again. Unordered requests for the same pair could still complete.

This change aligns cycle handling with the existing in-memory dependency node limit fallback: when topological sort detects a cycle, MergeJoinSnapDiffWriter writes the report via flushUnorderedDependencyNodes in dependency-node index order, returns a successful write with isReportDependencyOrdered() == false, and SnapshotDiffManager marks the job DONE with reason "Report is dependency unordered." The job’s ordered-request flag is unchanged so a later ordered request can reuse the completed job instead of recomputing indefinitely.

Implementation (OM / MergeJoinSnapDiffWriter):

• buildOrderedNodeIds returns null on a dependency-graph cycle (WARN log) instead of throwing.
• orderAndWriteReport returns boolean, flushes unordered nodes on null, and propagates the outcome through dependencyOrderingEnabledRef in writeReport.
• Other graph IllegalStateExceptions (non-cycle) are still propagated.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-17738

How was this patch tested?

Unit Test added:

• testDependencyOrderingFallsBackOnCycle in TestMergeJoinSnapDiffWriter (swap-rename scenario), asserting successful write, unordered outcome / DONE-style reason, RENAME a→b and RENAME b→a, same row order as explicit unordered writeReport, and no putOrderedReportEntries.

[smengcl]

Thanks @SaketaChalamchala for the patch

[smengcl]

This one builds the reason string inside the test, so it passes whenever the assertFalse above passes and does not touch SnapshotDiffManager. Could we drop it? assertFalse(store.isReportDependencyOrdered()) already checks the outcome, and the reason text itself comes from updateJobStatusToDone.

[smengcl]

nit: the path checks above already pin the order (a -> b, then b -> a). If we keep the comparison with the unordered run, assertEquals(explicitUnordered, entries) might be simpler, since DiffReportEntry implements equals over the type and both paths. It would also catch a size mismatch, which this loop over entries does not. The four put calls repeated for unorderedStore could go into a small helper shared by both stores.