Siyao Meng created HDDS-17069:
---------------------------------
Summary: Recon's per datanode open container count drifts from the
containers that are actually open
Key: HDDS-17069
URL: https://issues.apache.org/jira/browse/HDDS-17069
Project: Apache Ozone
Issue Type: Bug
Reporter: Siyao Meng
Attachments: CR-2-count-open-containers-per-pipeline-from-state.patch,
TestBugCR2OpenContainerCountDrift.java
h3. Mechanism
{{NodeEndpoint.getDatanodes}} fills {{openContainers}} of each datanode in
{{/api/v1/datanodes}} by adding up
{{ReconContainerManager.getPipelineToOpenContainer()}} over the datanode's
pipelines. That map is a counter: {{addNewContainer}} increments it and
{{transitionOpenToClosing}} decrements it, and nothing rebuilds or checks it.
Three paths make it wrong:
* An UNHEALTHY (or INVALID, DELETED) replica report of an OPEN container.
Recon's own step, {{checkContainerStateAndUpdate}}, skips
{{transitionOpenToClosing}} for unhealthy replicas (a check that dates from
HDDS-4029 and was moved there by HDDS-5126), and SCM's shared
{{AbstractContainerReportHandler}} then moves the container to CLOSING through
{{updateContainerState}}, which does not touch the counter. The count stays one
too high.
* Two datanodes reporting the same new container.
{{ContainerStateManagerImpl.addContainer}} silently skips a container that
already exists, but {{addNewContainer}} increments anyway. Reports from
different datanodes are handled in parallel, and the window includes an SCM
call.
* Restart. The map starts empty and is not rebuilt from the OPEN containers
Recon loads, so they count as zero, and the first two paths keep adding to
whatever is left.
h3. Trigger
A datanode reporting an unhealthy replica of an open container, reports of a
new container from two of the pipeline's datanodes that race (both pass the
existence check before either adds the container), or a Recon restart.
h3. Impact
Wrong per datanode open container count in Recon's datanode API and UI, growing
over the life of a pipeline and reset to zero by each restart. In the attached
run: two reports of one new container gave a count of 2 with one OPEN
container; after an UNHEALTHY then a CLOSED report the container was CLOSED and
the count stayed 1; after a restart the OPEN container was loaded and the
pipeline had no count. Display only: nothing else reads the counter.
h3. Reproduction
[^TestBugCR2OpenContainerCountDrift.java] goes in {{hadoop-ozone/recon}}
(target path and Maven command in the class comment) and passes on unmodified
source when the bug is present. It uses a real {{ReconContainerManager}} and
{{ReconPipelineManager}} on RocksDB and delivers reports through the real
{{ReconIncrementalContainerReportHandler}}. Only the SCM RPC peer is mocked.
The concurrent case handles the second datanode's report inside the first one's
SCM call, so it is deterministic. The restart case builds a new container
manager over the same Recon SCM DB.
h3. Patch
[^CR-2-count-open-containers-per-pipeline-from-state.patch], against
916aa1d9949cbb10f2750a40c8704340893131a2. It also applies to master at
d77edaa6c15.
The counter is removed. {{getPipelineToOpenContainer()}} now counts Recon's
OPEN containers per pipeline from {{getContainers(OPEN)}}, which uses the
container state index, and {{NodeEndpoint.getDatanodes}} calls it once per
request instead of once per pipeline per datanode. The count can no longer
drift, and nothing needs rebuilding at startup.
Covered by the new
{{TestReconContainerManager.testOpenContainerCountWhenContainerAddedAgain}},
{{TestReconContainerManager.testOpenContainerCountAfterRestart}} and
{{TestReconIncrementalContainerReportHandler.testOpenContainerCountAfterUnhealthyReplicaReport}};
all fail without the change (count 2 instead of 1, no count after the restart,
and 1 left for a CLOSED container).
{{testTransitionOpenToClosingDoesNotDecrementCountOnFailure}} set the removed
counter directly; its check that {{transitionOpenToClosing}} of an unknown
container throws {{ContainerNotFoundException}} is kept as
{{testTransitionOpenToClosingOfUnknownContainer}}. With the change the Recon
{{scm}}, {{fsck}} and {{api}} unit tests pass, checkstyle is clean, and the
attached tests no longer show the bug.
Found by code review of the datanode container report and Recon container
tracking paths, as part of the TLA+ verification effort under HDDS-15926, on
commit 916aa1d9949cbb10f2750a40c8704340893131a2. Checked against HDDS issues
and apache/ozone pull requests for duplicates before filing. Related but not
duplicates: HDDS-5314 (resolved, introduced the counter), HDDS-5126 (resolved,
moved the unhealthy replica check into its current method), HDDS-15308
(resolved, reworked Recon's report driven transitions), HDDS-14560 (resolved,
per disk open containers, does not use this counter). 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]
