[
https://issues.apache.org/jira/browse/HDDS-17736?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17736:
------------------------------
Attachment: MC-11-requeue-report-of-superseded-diff-task.patch
> 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
> Affects Versions: 2.2.0, 2.2.1, 2.2.2
> Reporter: Siyao Meng
> Priority: Minor
> 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]
