[
https://issues.apache.org/jira/browse/HDDS-17735?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17735:
------------------------------
Attachment: MC-6-keep-job-resubmitted-during-cleanup-scan.patch
> Snapshot diff cleanup can delete a job resubmitted during its scan, failing
> the submit or losing the new job
> ------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17735
> URL: https://issues.apache.org/jira/browse/HDDS-17735
> Project: Apache Ozone
> Issue Type: Bug
> Affects Versions: 2.2.0, 2.2.1, 2.2.2
> Reporter: Siyao Meng
> Priority: Minor
> Attachments: MC-6-keep-job-resubmitted-during-cleanup-scan.patch,
> TestBugMC6CleanupDeletesResubmittedJob.java
>
>
> h3. Mechanism
> {{SnapshotDiffCleanupService.moveOldSnapDiffJobsToPurgeTable}} iterates the
> snapshot diff job table, and for every expired or CANCELLED, FAILED or
> REJECTED job it queues a delete of that job key in a write batch, which it
> writes after the scan. It does not read the row again before the write and
> takes no lock that {{SnapshotDiffManager}} takes. A resubmit of the same
> snapshot pair in {{SnapshotDiffManager.createSnapDiffJob}} stores a new job
> (new job ID, QUEUED) under the same job key. If that happens after the
> cleanup read the old row and before it wrote the batch, the batch deletes the
> new job.
> This is the same symptom as HDDS-16443, through a different cause. HDDS-16443
> describes a new job created with the previous terminal status, so that a
> fresh scan purges it. That stated cause is already closed on master by
> HDDS-15397, which creates the new job as QUEUED; it does not close this one,
> where the scan is stale.
> h3. Trigger
> # A snapshot diff job ends CANCELLED, FAILED or REJECTED.
> # A cleanup run scans the job table and reads that job.
> # Before the cleanup writes its batch, a client submits the same pair again.
> No fault is needed. The window is the duration of the cleanup's scan, once
> per cleanup interval (default 60 minutes).
> h3. Impact
> * If the batch lands between the creation of the new job and its switch to
> IN_PROGRESS in {{submitSnapshotDiff}}, {{updateJobStatus}} and then
> {{updateJobStatusToFailed}} hit the missing row and the submit RPC fails with
> a NullPointerException.
> * If it lands after the submit returned, the client was told the job was
> submitted but the job is gone: {{cancelSnapshotDiff}} answers that the job
> does not exist, and the next submit starts a new job, so the diff work is
> done again.
> In a test harness with the cleanup running back to back on its own thread,
> 482 of 20,000 submit and cancel iterations failed with the
> NullPointerException and 1 job was lost after a successful submit; in
> production the cleanup scans once per interval, so the rate is far lower.
> In the lost job case the job's task keeps running and writes report rows
> under the lost job ID: its whole report if a new submit created a job under
> the same key before its next {{areDiffJobAndSnapshotsActive}} check,
> otherwise the rows written up to that check. Nothing queues the lost job ID
> for purge (the cleanup queued the old job ID it scanned, and the next submit
> finds no previous job), so these rows stay in the OM's local snapshot diff
> DB. This is read from the code, not run. The fix proposed in the related
> issue on report rows of a cancelled task, which queues a task's job ID for
> cleanup when its job row is gone or replaced at the end of the task, would
> also purge these rows. No user data is affected.
> This is less severe than HDDS-16443 suggests: the window is one stale scan
> per cleanup interval, and a client retry recovers.
> h3. Reproduction
> PASS, deterministic, unmodified source.
> [^TestBugMC6CleanupDeletesResubmittedJob.java]
> ({{hadoop-ozone/ozone-manager}}) runs the real {{SnapshotDiffManager}} and
> {{SnapshotDiffCleanupService}} on a real RocksDB with the rest of the OM
> mocked; the report generation task does nothing, so a submitted job stays
> IN_PROGRESS like a long running diff. The only timing aid is a hook on the
> cleanup's codec: when the cleanup decodes the job it scanned, the hook starts
> the resubmit on another thread and waits until it has finished or is blocked.
> Output on d3b75c75bf9ad475551202477c3a21a3f4b968b7:
> {noformat}
> resubmit response: Previous snapshot diff attempt found. Status: CANCELLED.
> job after one cleanup run: null
> cancel of the resubmitted job: Snapshot diff job doesn't exist for given
> parameters.
> {noformat}
> A passing test means the defect is present.
> h3. Patch
> [^MC-6-keep-job-resubmitted-during-cleanup-scan.patch], against
> d3b75c75bf9ad475551202477c3a21a3f4b968b7. It also applies to master at
> bbc8505e00d.
> {{SnapshotDiffCleanupService.run}} calls {{moveOldSnapDiffJobsToPurgeTable}}
> inside {{synchronized (db)}}, and {{SnapshotDiffManager.createSnapDiffJob}}
> writes the new job inside the same monitor. The cleanup's scan and its delete
> are then atomic with respect to the resubmit's write, so the cleanup either
> sees the new job (not purgeable) or removes the old one before the new one is
> written. The monitor is the shared {{ManagedRocksDB}} instance, the same one
> the open PR for HDDS-16442 uses in {{run}}, so the two changes compose (they
> touch the same lines of {{run}} and merge into one block). The cost: job
> creation, and the progress and status updates of running diff tasks that wait
> for the manager monitor meanwhile, wait for one scan of the job table per
> cleanup run.
> Covered by {{testCleanupKeepsJobResubmittedDuringItsScan}} in the existing
> {{TestSnapshotDiffManager}}, which uses the same codec hook and asserts that
> the resubmitted job survives with a new job ID while the cancelled job is
> still queued for purge. Without the change it fails because the job is gone.
> With it {{TestSnapshotDiffManager}}, {{TestSnapshotDiffCleanupService}} and
> {{TestSnapshotDiffManagerMXBean}} (88 tests) pass, checkstyle is clean, the
> stress run above completes with no failed submit and no lost job, and the
> reproduction fails because the job is present. The released 2.2 line also has
> the HDDS-16443 cause, which this patch does not address there.
> 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]
