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]

Reply via email to