[
https://issues.apache.org/jira/browse/HDDS-17040?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17040:
------------------------------
Attachment: CR-8-retry-delta-task-that-throws.patch
> Recon delta task that throws is not retried and triggers no reinitialization,
> so its batch is silently skipped
> --------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17040
> URL: https://issues.apache.org/jira/browse/HDDS-17040
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Minor
> Attachments: CR-8-retry-delta-task-that-throws.patch,
> TestBugCR8ThrowingTaskLosesBatch.java
>
>
> h3. Mechanism
> {{ReconTaskControllerImpl.processTasks}} runs each task's {{process}} on the
> task pool. A task that returns a failed result goes through {{thenAccept}}:
> it is added to {{failedTasks}}, retried once, and if the retry fails too
> {{tasksFailed}} is set, which makes the next sync queue a reinitialization. A
> task whose {{process}} throws goes through {{exceptionally}} instead, which
> logs, counts the failure and records status -1 but does not add the task to
> {{failedTasks}}. So there is no retry and no reinitialization, and the next
> batch the task processes successfully sets its {{lastUpdatedSeqNumber}} past
> the lost batch. The javadoc of {{processTasks}} says failed tasks are added
> to the list for retry or reprocessing.
> h3. Trigger
> Any unchecked exception out of a task's {{process}} that does not also close
> the Recon DB. I did not find a production condition that does this, so in
> practice this is a safety net for unexpected runtime exceptions such as a bug
> in a handler. The reproduction injects the exception. If one occurs, it would
> surface where {{FileSizeCountTaskHelper.writeCountsToDB}} rethrows a write
> failure as {{RuntimeException}}, or where
> {{ContainerKeyMapperHelper.process}} lets anything other than an
> {{IOException}} from its key handlers escape. {{NSSummaryTask}} and
> {{OmTableInsightTask}} catch per event and return a failed result, so they
> are not affected. A RocksDB I/O error is not a trigger: it closes the Recon
> DB, the other tasks then fail as well and the reinitialization they cause
> also rebuilds the throwing task (code reading).
> h3. Impact
> The batch is missing from the derived data of the throwing task for good, for
> example from the file size distribution served by the utilization endpoint.
> Nothing detects it: the task cursor is current after the next batch,
> {{tasksFailed}} stays false and the startup check sees no gap. A manual
> rebuild repairs it.
> h3. Reproduction
> PASS, deterministic, unmodified source.
> [^TestBugCR8ThrowingTaskLosesBatch.java] ({{hadoop-ozone/recon}}) uses the
> real controller, {{FileSizeCountTaskOBS}}, the RocksDB backed
> {{ReconFileMetadataManager}} and the task status table, and injects one
> {{IOException}} from the first commit of the file size counts, so {{process}}
> throws once. Two batches are handed to the controller's event processing one
> after the other. Output: one {{process}} call for the first batch, no
> reinitialization requested, cursor at the second batch, and the 1000 byte
> file of the first batch missing from the counts. A passing test means the
> defect is present. The injected exception stands in for an unchecked
> exception that does not close the DB.
> h3. Patch
> [^CR-8-retry-delta-task-that-throws.patch], against
> 0665932dd35da4d05087e02748af0a64eb846ad5. It also applies to master at
> 2264d4d48ac.
> The {{exceptionally}} branch adds the task to {{failedTasks}} without seek
> positions, so a thrown exception is handled like a returned failure: one
> retry of the batch, then a reinitialization if it fails again.
> {{failedTasks}} and {{retryFailedTasks}} become synchronized lists, since
> both branches add to them from pool threads. A task that throws on every
> attempt now leads to a reinitialization of all tasks instead of a silently
> skipped batch, as a task that returns failure already does.
> Covered in the existing {{TestReconTaskControllerImpl}}:
> {{testTaskRecordsFailureOnException}} asserted the single call and now
> asserts the retry and the reinitialization request, and the new
> {{testTaskThrowingOnceIsRetried}} fails without the change ({{expected: 100L
> but was: 0L}}). With the change the Recon tasks suites and
> {{TestOzoneManagerServiceProviderImpl}} (155 tests) pass and checkstyle is
> clean. With the patch the reproduction fails at its first assertion: the task
> is retried and the file is counted.
> Found by code review of the Recon namespace summary and task sync paths, as
> part of the TLA+ verification effort under HDDS-15926, on commit
> 0665932dd35da4d05087e02748af0a64eb846ad5. Checked against HDDS issues and
> apache/ozone pull requests for duplicates before filing. Related but not a
> duplicate: HDDS-16463 (sync the Recon DB before the cursor advances, which
> reworks the success branch of the same method). The attached patch is a
> proposal for review. Generated with Specula (Claude Opus 5.5).
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
