Siyao Meng created HDDS-17736:
---------------------------------
Summary: Report rows written by a cancelled snapshot diff task
after a resubmit are never deleted
Key: HDDS-17736
URL: https://issues.apache.org/jira/browse/HDDS-17736
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments: MC-11-requeue-report-of-superseded-diff-task.patch,
TestBugMC11CancelledTaskReportRowsLeak.java
h3. Mechanism
{{cancelSnapshotDiff}} only sets the job to CANCELLED; the task keeps running
and keeps writing report rows under its job ID (HDDS-17401). After a resubmit
even its liveness checks pass, because {{areDiffJobAndSnapshotsActive}} looks
up the job of the snapshot pair, which is now the new job. A resubmit of the
same pair puts the old job ID into the purged job table at once
({{SnapshotDiffManager.enqueueSnapshotDiffReportCleanup}}). The next
{{SnapshotDiffCleanupService}} run deletes the rows that exist at that moment
and removes the purged entry in the same batch. Rows the old task writes after
that run belong to a job ID that is in neither the job table nor the purged job
table, and nothing deletes them later, also not an OM restart
({{loadJobsOnStartUp}} only handles IN_PROGRESS jobs in the job table). The fix
of HDDS-16441 stops the old task from publishing DONE but not from writing
rows. This holds for both the default (legacy) diff path and the optimized one.
h3. Trigger
# Submit a snapshot diff.
# Cancel it while its task is generating the report, then submit the same pair
again.
# A cleanup run happens before the old task has written its whole report.
No fault is needed. With the default cleanup interval of 60 minutes this mostly
concerns large diffs that take long to write.
h3. Impact
Up to one full report of rows per such cancel and resubmit stays in the OM's
local snapshot diff DB for good. No client reads these rows, so no wrong result
is returned; the cost is disk space.
Without a resubmit (cancel only, or expiry) the window is much smaller: the
task stops at its next update of the missing job row, and only rows written
after the second cleanup run and before that update stay behind. The patch
covers this case too.
h3. Reproduction
PASS, deterministic, unmodified source.
[^TestBugMC11CancelledTaskReportRowsLeak.java] ({{hadoop-ozone/ozone-manager}})
runs the real {{SnapshotDiffManager}} and {{SnapshotDiffCleanupService}} on a
real RocksDB with the rest of the OM mocked. The cancelled job's task body
({{generateSnapshotDiffReport}}) runs for real; at its first liveness check a
hook cancels and resubmits the pair through the public calls, runs the cleanup
once, and writes three report rows under the cancelled job's ID, standing in
for the report writer, which does not check for cancellation. Output on
d3b75c75bf9ad475551202477c3a21a3f4b968b7 after the task ended and three more
cleanup runs: 3 report rows left, job ID not queued for purge. A passing test
means the defect is present.
h3. Patch
[^MC-11-requeue-report-of-superseded-diff-task.patch], against
d3b75c75bf9ad475551202477c3a21a3f4b968b7. It also applies to master at
bbc8505e00d.
When {{generateSnapshotDiffReport}} ends, after it has written its last report
row, it reads the job row of its key; if the row is gone or holds another job
ID, it queues its own job ID for cleanup again. All its rows are written by
then, so the next cleanup run deletes them, whatever an earlier run already
consumed. This needs neither stopping the task (HDDS-17401) nor a liveness
check in the writer, which would race with a resubmit unless every row write
took the manager lock. Stopping the task as HDDS-17401 proposes would still
save the wasted work. Queuing a job ID that is already queued is harmless. The
same change also purges the report rows of a job that the cleanup deleted while
it was being resubmitted (the related issue on the cleanup scan), since that
job's task also ends with its job row gone or replaced.
Covered by {{testSupersededTaskQueuesItsReportForCleanup}} in the existing
{{TestSnapshotDiffManager}}: a task whose job row is replaced by a new job ID
must leave its own job ID queued for purge, and a task whose cancelled row
still holds its own job ID must not. Without the change the first part fails.
With it {{TestSnapshotDiffManager}}, {{TestSnapshotDiffCleanupService}} and
{{TestSnapshotDiffManagerMXBean}} (88 tests) pass, checkstyle is clean, and the
reproduction fails because no rows remain. Rows leaked before the patch, or by
a task that an OM restart stops after the cleanup has consumed its entry, are
not removed.
The patch and the open PR for HDDS-16442 both change
{{SnapshotDiffManager.generateSnapshotDiffReport}} and
{{TestSnapshotDiffManager}}; whichever lands second needs a small rebase.
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]
