Siyao Meng created HDDS-17740:
---------------------------------
Summary: Ordered snapshot diff resubmit skips the cleanup grace
period, so a concurrent read of the DONE report fails
Key: HDDS-17740
URL: https://issues.apache.org/jira/browse/HDDS-17740
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments:
CR-6-keep-resubmit-queued-report-for-one-cleanup-run.patch,
TestBugCR6ResubmitSkipsCleanupGrace.java
h3. Mechanism
{{SnapshotDiffCleanupService.run}} first deletes the reports of jobs in the
purged job table and only then moves expired or failed jobs there. The comment
in {{run}} explains why: a job moved in one run keeps its report until the next
run, so that a request that already saw the job as DONE can still read the
report. With {{ozone.om.snapshot.diff.optimized.enabled}} = true (default
false), an ordered request for a pair on an FSO bucket whose job is DONE with
an unordered report replaces that job, and
{{SnapshotDiffManager.createSnapDiffJob}} writes the old job ID straight into
the purged job table. The very next cleanup run then deletes the old report.
{{getSnapshotDiffReport}} takes no lock, so a call that read the DONE job
before the resubmit can open the report after it was deleted.
h3. Trigger
# An unordered snapshot diff of a pair finishes DONE.
# A client calls {{getSnapshotDiffReport}} and reads the DONE job.
# Before that call reads the report, another request submits the pair with an
ordered report, and a cleanup run happens (default every 60 minutes).
The resubmit and the cleanup run must both fall inside one read call. The next
page call reads the job table again and gets the new job's IN_PROGRESS status,
so a client paging through a report is not affected beyond the one call.
h3. Impact
That read call fails instead of returning the page: with an empty report the
integrity check in {{checkReportsIntegrity}} fails, and at the moment the call
ends in the NullPointerException of HDDS-16442 (with the open fix for
HDDS-16442 it would be an IOException "Report integrity check failed", read
from that change, not run). A retry returns the new job's status.
h3. Reproduction
PASS, deterministic, unmodified source.
[^TestBugCR6ResubmitSkipsCleanupGrace.java] ({{hadoop-ozone/ozone-manager}})
runs the real {{SnapshotDiffManager}} and {{SnapshotDiffCleanupService}} on a
real RocksDB with the rest of the OM mocked. A stand in for the report
generation task writes five report rows and publishes DONE through the real
{{updateJobStatusToDone}}. The only timing aid holds the reader in
{{getSnapshotRootPath}}, the call right after it read the DONE job; meanwhile
the test submits an ordered request and runs the cleanup once. Output on
d3b75c75bf9ad475551202477c3a21a3f4b968b7: 0 of 5 report rows left after one
cleanup run, and the reader's call fails with a NullPointerException from
{{updateJobStatus}}. A passing test means the defect is present.
h3. Patch
[^CR-6-keep-resubmit-queued-report-for-one-cleanup-run.patch], against
d3b75c75bf9ad475551202477c3a21a3f4b968b7. It also applies to master at
bbc8505e00d.
{{SnapshotDiffCleanupService}} remembers which purged entries it saw in the
previous run. {{removeOlderJobReport}} deletes only the reports of entries that
were already there in the previous run and keeps an entry it sees for the first
time for one more run. Jobs the cleanup moves itself are deleted on the next
run as before, so the existing grace period is unchanged and a resubmit now
gets the same one. The change is local to the cleanup service, with no change
to the stored format. After an OM restart the first run only records the
existing entries, so their reports are deleted one interval later than today.
Covered by {{testReportQueuedBetweenRunsIsKeptForOneRun}} in the existing
{{TestSnapshotDiffCleanupService}}: after one cleanup run, a report and its
purged entry are written; they must survive the next run and be gone after the
one after it. Without the change it fails because the run after the entry was
written deletes the report. With it {{TestSnapshotDiffManager}},
{{TestSnapshotDiffCleanupService}} and {{TestSnapshotDiffManagerMXBean}} (88
tests) pass, checkstyle is clean, and the reproduction fails because all five
rows remain after its cleanup run.
Found by code review of the OM optimized snapshot diff paths, as part of the
TLA+ verification effort 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]
