Siyao Meng created HDDS-17737:
---------------------------------
Summary: Ordered snapshot diff served by the legacy diff is stored
as ordered, so a later ordered request never replaces it
Key: HDDS-17737
URL: https://issues.apache.org/jira/browse/HDDS-17737
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments: MC-10-legacy-diff-job-not-recorded-ordered.patch,
TestBugMC10LegacyFallbackRecordedOrdered.java
h3. Mechanism
{{SnapshotDiffManager.submitSnapshotDiff}} stores the job's ordered flag as
"optimized diff enabled and ordered requested". The job then runs the legacy
diff, which does not order the report, whenever the optimized diff is enabled
but the native SST diff is unavailable (native library not loaded or
{{ozone.om.snapshot.diff.disable.native.libs}}) and a full diff is not forced.
That routing is decided from the same request flags and server settings, so it
is known at submit time.
A QUEUED or DONE previous job is replaced by a new request only if {{isFso &&
effectiveOrderedReport && !snapDiffJob.isOrderedReport()}}, that is, only if
the stored job is not ordered. Because the legacy job was stored as ordered, a
later ordered request that would run the optimized diff (for example with a
forced full diff) gets the legacy report back until the job expires.
h3. Trigger
Optimized snapshot diff enabled ({{ozone.om.snapshot.diff.optimized.enabled}},
default false), native SST diff unavailable, full diff not forced on the
server, an FSO bucket.
# An ordered snapshot diff request without a forced full diff. It runs the
legacy diff.
# Another ordered request for the same pair with a forced full diff.
h3. Impact
The second request is answered "Previous snapshot diff attempt found. Status:
DONE." with the legacy, type ordered report, for example {{DELETE dir1/dir2}}
before {{RENAME dir1/dir2/key1 -> dir1/dir3/renamed_key}}, which does not
replay in that order. The response reason is empty, so the client is not told
the report is ordered, but it cannot get the ordered report for that pair until
the job expires. The job records an ordered request that the legacy diff never
attempts, so the resubmit check treats the legacy report as an ordered one.
h3. Reproduction
[^TestBugMC10LegacyFallbackRecordedOrdered.java] is a self contained mini
cluster class (target path and Maven command in the class comment) with the
optimized diff enabled, {{ozone.om.snapshot.diff.disable.native.libs=true}} and
full diff not forced on the server, using only public client calls. History:
{{dir1/dir2/key1, dir1/dir3/key1; snapshot 1; mv dir1/dir2/key1
dir1/dir3/renamed_key; rm -r dir1/dir2; snapshot 2}}. On unmodified source the
first ordered request ends DONE with an empty reason and {{DELETE dir1/dir2,
MODIFY dir1/dir3, RENAME dir1/dir2/key1 -> dir1/dir3/renamed_key}}, and the
second ordered request with a forced full diff returns the same job and report.
A passing test means the defect is present.
h3. Patch
[^MC-10-legacy-diff-job-not-recorded-ordered.patch], against
d3b75c75bf9ad475551202477c3a21a3f4b968b7. It also applies to master at
bbc8505e00d.
The existing routing condition moves into a private helper
{{SnapshotDiffManager.isLegacyDiff}}, used by {{generateSnapshotDiffReport}}
(unchanged behaviour) and by {{submitSnapshotDiff}}, which now stores the job
as ordered only if the optimized diff will serve it, and logs that the ordered
request is ignored otherwise (the existing log for a disabled optimized diff,
widened). The job's ordered field now means the optimized diff attempts
ordering, and a one line comment says so where it is computed; whether the
report is ordered is in the DONE reason, as for the over the limit fallback. A
legacy job is then stored like a request made while the optimized diff is
disabled. A later ordered request that runs the optimized diff passes the
existing resubmit check; one that would run the legacy diff again does not, so
there is no recompute loop. Storing the outcome after the run instead was not
taken, because then every ordered request with the same flags would rerun the
legacy diff.
Covered by {{testSubmitOrderedServedByLegacyDiffIsNotRecordedOrdered}} in the
existing {{TestSnapshotDiffManager}}. Behaviour change visible in two existing
tests: {{testSubmitDoneResubmitsFsoOrdered}} and
{{testSubmitQueuedResubmitsFsoOrdered}} passed before although the unit test
JVM has no native library, so their requests were served by the legacy diff: an
ordered request served by the legacy diff replaced an unordered job and reran
the legacy diff. That no longer happens, so they now force a full diff to keep
testing the optimized resubmit. Without the change the new test fails with
"expected: <false> but was: <true>"; with it {{TestSnapshotDiffManager}},
{{TestSnapshotDiffManagerMXBean}}, {{TestSnapDiffDependencyGraph}} and
{{TestMergeJoinSnapDiffWriter}} pass (123 tests) and checkstyle is clean. With
the patch the reproduction fails as intended: the forced request is resubmitted
("Previous snapshot diff attempt found with an unordered report") and ends DONE
"Report is dependency ordered." with {{RENAME dir1/dir2/key1 ->
dir1/dir3/renamed_key, DELETE dir1/dir2}}. Jobs already stored with the flag
expire as before.
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]