Siyao Meng created HDDS-17041:
---------------------------------
Summary: Retry of a failed NSSummaryTask applies namespace summary
deltas twice or skips them, so Recon file counts and sizes stay wrong
Key: HDDS-17041
URL: https://issues.apache.org/jira/browse/HDDS-17041
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments:
MC-1-resume-nssummary-subtasks-from-committed-position.patch,
TestBugMC1NSSummaryRetryReplay.java
h3. Mechanism
{{ReconTaskControllerImpl.processOMUpdateBatch}} retries a failed task once on
the same event batch, passing the seek positions the failed attempt returned
({{TaskResult.getSubTaskSeekPositions}}). {{NSSummaryTask.process}} runs three
sub-tasks ({{NSSummaryTaskWithFSO.processWithFSO}},
{{NSSummaryTaskWithLegacy.processWithLegacy}},
{{NSSummaryTaskWithOBS.processWithOBS}}) and records one seek position per
sub-task. Each sub-task skips that many events and adds the remaining ones to
the stored totals (read, add, write back), so a retry is only correct if the
seek position equals the number of leading events whose deltas are already
committed to the Recon DB. Three exits of the sub-tasks break that:
* *A sub-task that succeeded reports the position it started from.* After its
final flush it returns {{seekPos}}, which is 0 unless a threshold flush
happened. If a sibling sub-task fails, the retry runs the successful sub-task
again from that position and adds its committed deltas a second time.
* *The position after a threshold flush is one too far.* When the in memory map
reaches {{ozone.recon.nssummary.flush.db.max.threshold}} entries, the sub-task
commits it and sets {{seekPos = eventCounter + 1}}. {{eventCounter}} already
counts the event that was just applied, so the retry also skips the next event,
which was never applied.
* *An unchecked exception discards the flushed position.* The loops catch only
{{IOException}}. Any other exception ends the sub-task, and
{{NSSummaryTask.awaitSubTask}} records the seek position the attempt started
from. The retry then applies again everything that the threshold flushes of the
failed attempt had committed.
In each case the retry succeeds, the controller moves the task's
{{lastUpdatedSeqNumber}} to the end of the batch and does not set
{{tasksFailed}}, so no rebuild is queued. Later batches add to the wrong totals.
h3. Trigger
A sub-task of {{NSSummaryTask}} has to fail once and succeed on the immediate
retry. A failure that repeats makes the retry fail as well, which sets
{{tasksFailed}}, and the full rebuild that follows replaces the wrong totals.
An I/O or corruption error of the Recon RocksDB does not qualify, because
{{RocksDatabase}} then closes the database and the retry fails too.
* Double counting of a sibling: any batch in which one sub-task fails and
another one succeeds. The batch size and the flush threshold do not matter.
* Skipped event, and double counting after an unchecked exception: in addition,
the failing sub-task must have done a threshold flush earlier in the same
batch, that is the batch touches at least
{{ozone.recon.nssummary.flush.db.max.threshold}} distinct summary entries in
that sub-task (150000 in ozone-default.xml). In practice this needs an FSO
bucket, or a Legacy bucket with file system paths, with a wide directory tree.
OBS would need that many buckets in one batch.
Argued from the code, not run: the sub-tasks look up buckets (OBS, Legacy) and
Legacy parent directories in the Recon OM DB.
{{OzoneManagerServiceProviderImpl.syncDataFromOM}} applies each delta to that
DB before it queues the events, and keeps applying later deltas while the
controller processes the queue, so the DB is ahead of the batch being processed
and can change between the first attempt and the retry. If a bucket that the
sub-task has not cached yet, or a Legacy parent directory, is deleted and
recreated under the same name, and the first attempt runs after the delete is
applied but the recreate is applied before the retry, the first attempt fails
on the missing entry and the retry succeeds. The window is narrow, since the
two attempts run back to back. Otherwise the failure repeats and the rebuild
repairs the totals. (On such a retry the key is counted under the recreated
bucket or directory, a separate inaccuracy that the patch does not change.) The
reproductions inject the failure.
The first two exits exist since HDDS-12062 (ozone-2.0.0 to ozone-2.2.1 by
reading the tagged source, not run on a release). The third exists since
HDDS-15335 (ozone-2.2.0, ozone-2.2.1). Before HDDS-15335 an unchecked exception
was not retried at all.
h3. Impact
The namespace summary that Recon serves for {{/api/v1/namespace/summary}} and
{{/api/v1/namespace/du}} keeps wrong file counts and sizes for the affected
buckets and directories until a full rebuild. Nothing compares the incremental
totals with a rebuild. No OM data is affected.
h3. Reproduction
Run on unmodified source. [^TestBugMC1NSSummaryRetryReplay.java]
({{hadoop-ozone/recon}}) builds the summary of one bucket per layout with a
full rebuild, then sends one batch through the real
{{ReconTaskControllerImpl.processReconEvent}}, which runs the real
{{NSSummaryTask}} on the real Recon RocksDB and then its retry. The only change
is a Mockito spy of the summary manager that throws once when the OBS bucket's
summary is read. Each test also checks that the batch is recorded as applied
and that {{hasTasksFailed}} is false. A passing test means the defect is
present.
* Legacy put plus OBS put, OBS fails once: the Legacy bucket ends with 3 files,
a rebuild gives 2.
* Flush threshold 1, two OBS puts, the second fails once with {{IOException}}:
the bucket ends with 2 files, a rebuild gives 3.
* The same with {{IllegalStateException}}: the bucket ends with 4 files, a
rebuild gives 3.
With the patch all three tests fail because the totals equal the rebuild.
h3. Patch
[^MC-1-resume-nssummary-subtasks-from-committed-position.patch], against
0665932dd35da4d05087e02748af0a64eb846ad5. It also applies to master at
2264d4d48ac.
The patch makes the seek position of each sub-task mean "events of this batch
already committed", counted from the start of the batch. In the three
sub-tasks: a successful run returns the end of the batch ({{itrPos +
eventCounter}}), so a retry skips it; a threshold flush records {{itrPos +
eventCounter}} instead of {{eventCounter + 1}}; and the per event {{catch}}
also takes {{RuntimeException}}, so the sub-task returns its last flushed
position the same way it already does for {{IOException}}. In FSO and Legacy
the threshold flush moves inside that {{try}}, as it already is in OBS, so an
unchecked exception from the flush is covered too. A failed flush commits
nothing (the batch is atomic), so the last committed position is still right.
Not covered: an unchecked exception from the final flush after the loop, an
{{Error}} and an interrupt, which still fall back to the position the attempt
started from. A failure that repeats still fails the retry and leads to the
rebuild as before. The controller and {{NSSummaryTask}} are unchanged. Retrying
only the failed sub-tasks would need extra state in {{NSSummaryTask}}, the seek
position already carries what is needed.
Covered by four new cases in the existing {{TestNSSummaryTask}}:
{{testRetryDoesNotReapplySucceededSubTask}},
{{testRetryAfterThresholdFlushResumesAfterIOException}},
{{testRetryAfterThresholdFlushResumesAfterRuntimeException}} (OBS) and
{{testRetryResumesAfterUncheckedExceptionInThresholdFlush}} (Legacy, the flush
throws). They fail without the change ({{expected: 2 but was: 3}}, {{expected:
3 but was: 2}}, {{expected: 3 but was: 4}}, {{expected: 3 but was: 4}}). Three
existing assertions on the returned seek positions are updated to the new
meaning ({{testIndependentSubTaskSeekPositions}} in {{TestNSSummaryTask}}, two
in {{TestNSSummaryTaskWithFSO}}). With the change the Recon namespace summary,
task controller, delta sync and namespace summary endpoint suites pass (287
test runs) and checkstyle is clean. The patch does not repair totals that are
already wrong.
Found by TLA+ model checking and code review of the Recon namespace summary and
task sync paths under HDDS-15926, on commit
0665932dd35da4d05087e02748af0a64eb846ad5. Checked against HDDS issues and
apache/ozone pull requests for duplicates before filing. Related but not
duplicates: HDDS-12062 (resolved, introduced the seek position retry),
HDDS-5146 (a failure between the Recon OM DB update and the task run loses a
batch) and HDDS-16463 (sync the Recon DB before the cursor advances). 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]
