[
https://issues.apache.org/jira/browse/HDDS-17069?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17069:
------------------------------
Attachment: TestBugCR2OpenContainerCountDrift.java
> 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
> Priority: Minor
> 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]
