Siyao Meng created HDDS-17061:
---------------------------------

             Summary: Recon drops the replicas of a DELETED container when it 
moves it back to SCM's live state
                 Key: HDDS-17061
                 URL: https://issues.apache.org/jira/browse/HDDS-17061
             Project: Apache Ozone
          Issue Type: Bug
            Reporter: Siyao Meng
         Attachments: 
MC-10-keep-replicas-when-recon-reopens-deleted-container.patch, 
TestBugMC10RebuildDropsReplicas.java

h3. Mechanism
When Recon has a container DELETED but SCM lists it as CLOSED or QUASI_CLOSED, 
{{ReconStorageContainerSyncHelper.rebuildContainerFromScm}} calls 
{{deleteContainer}}, which removes the whole entry including its replica set, 
and then {{addNewContainer}} with SCM's {{ContainerInfo}}. Two symptoms follow:
# The re-added container has no replicas. The next health sweep 
({{ReconReplicationManager.processAll}}) records it MISSING although datanodes 
still hold it, at the latest until each datanode's next full container report 
({{hdds.container.report.interval}}, default 60m) adds its replica back.
# If {{addNewContainer}} throws, the failure is only logged and the container 
is no longer in Recon's map. {{storeHealthStatesToDatabase}} rewrites 
UNHEALTHY_CONTAINERS rows only for containers in the map, so a row the 
container had before stays. {{ContainerEndpoint.toUnhealthyMetadata}} then gets 
{{ContainerNotFoundException}} for it and {{/api/v1/containers/unhealthy}} and 
{{/api/v1/containers/unhealthy/MISSING}} fail with HTTP 500 for every page that 
includes the row, until a later sync adds the container back.

SCM's own report handler moves a container back from DELETED in place with 
{{ContainerManager.transitionDeletingOrDeletedToTargetState}}, keeping the 
record and its replicas.

h3. Trigger
The rebuild runs only when Recon has the container DELETED while SCM lists it 
as live, and Recon has not moved it back itself. Recon's report handler moves 
it back in place when it gets the same datanode report that made SCM do so, so 
the state needs Recon to miss or misjudge that report. The route that was run: 
Recon's DELETED pass reads a page of SCM's DELETED IDs, a datanode reports a 
non-empty CLOSING replica that makes SCM move the container to QUASI_CLOSED 
(HDDS-14684) before the page is processed, Recon retires the container from the 
stale page while keeping the replica, and the datanode then closes its replica 
(bcsId not above the container's), which does not move a DELETED container 
back. The next sync cycle runs the rebuild. Another route, argued and not run: 
Recon is down while the datanode reports the replica that makes SCM move the 
container back.

Symptom 2 also needs {{addNewContainer}} to fail and a MISSING row from before 
the container was retired, which survives only if no health sweep ran between 
the two syncs. At defaults the sweep runs every 5 minutes 
({{ozone.recon.task.missingcontainer.interval}}) and the sync every 6 hours, so 
this needs a second sync within minutes of the one that retired the container: 
a sync interval configured below the sweep interval, or a Recon restart, where 
the first sync runs 1 minute after startup (argued, not run). For a CLOSED or 
QUASI_CLOSED container {{addNewContainer}} can fail only on a container table 
write error or if Recon's pipeline sync removes the container's pipeline 
between the pipeline check and the pipeline update inside 
{{ContainerStateManagerImpl.addContainer}}. Neither was driven; the 
reproduction injects the exception.

h3. Impact
* Symptom 1: a false MISSING container in Recon's unhealthy containers API and 
UI for up to one full report interval. No data is lost; the replica history in 
Recon's DB is kept. The same code is in 2.2.0 and 2.2.1.
* Symptom 2: the unhealthy containers API returns HTTP 500 until the next 
successful sync ({{ozone.recon.scm.container.sync.task.interval.delay}}, 
default 6h).

h3. Reproduction
Both PASS, deterministic, unmodified source, in {{hadoop-ozone/recon}}. A 
passing test means the defect is present.
* [^TestBugMC10RebuildDropsReplicas.java]: real {{ReconContainerManager}}, 
{{ReconContainerReportHandler}} fed with full reports, {{SCMNodeManager}}, sync 
helper and health sweep (its database writes captured by a mock); only the SCM 
client is a fake backed by a state table, and the SCM state change and datanode 
report are placed right after the DELETED page RPC returns. For SCM 
QUASI_CLOSED and for SCM CLOSED, the rebuild leaves 0 replicas and the sweep 
writes MISSING; the next full report restores the replica and the row goes away.
* [^TestBugMC11StaleRowAfterFailedRebuild.java]: real container manager, sync 
helper, health sweep, UNHEALTHY_CONTAINERS table (Derby) and 
{{ContainerEndpoint}}; the SCM client is a mock and one {{IOException}} is 
injected into {{addNewContainer}}. The container leaves the map, its MISSING 
row survives three sweeps, and {{getUnhealthyContainers}} throws a 
{{WebApplicationException}} with status 500 caused by 
{{ContainerNotFoundException}}. The next sync re-adds the container and the 
endpoint returns 200. A control test with a sweep between the two syncs shows 
no failure.

h3. Patch
[^MC-10-keep-replicas-when-recon-reopens-deleted-container.patch], against 
916aa1d9949cbb10f2750a40c8704340893131a2. It also applies to master at 
62d982ff2d9.

{{rebuildContainerFromScm}} still checks that SCM lists the container in that 
state, then moves Recon's record back in place with 
{{transitionDeletingOrDeletedToTargetState}}, as SCM's report handler does, 
instead of deleting and re-adding it. The replicas are kept, and if the update 
fails the container stays in Recon's map with its replicas, so the next sweep 
rewrites its rows and the next sync tries again. This fixes both symptoms. 
Recon keeps its own copy of the container metadata (sequence ID, key count, 
used bytes) instead of taking SCM's; the report handler keeps those up to date 
from replica reports.

Covered by {{containerDeletedInReconButClosedInScmKeepsReplicas}} and 
{{failedCorrectionFromDeletedKeepsContainerInRecon}} in 
{{TestReconSCMContainerSyncIntegration}}. Without the change they fail (0 
replicas; container gone from the map); with it the Recon {{scm}} package tests 
pass and checkstyle is clean. With the patch the reproductions fail at their 
defect assertions: the replica is kept, and {{addNewContainer}} is no longer 
called so the container stays in the map. The control test of the second 
reproduction passes with and without the patch; it only checks that the 
endpoint answers 200 when a sweep ran between the two syncs.

Not changed: the stale DELETED page itself. Recon still retires such a 
container and shows it DELETED until the next sync moves it back. A tolerant 
{{ContainerEndpoint.toUnhealthyMetadata}} (skip and log a row whose container 
is not in the map) would be separate hardening.

Found by TLA+ model checking and code review of the datanode container report 
and Recon container tracking paths under HDDS-15926, on commit 
916aa1d9949cbb10f2750a40c8704340893131a2. 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
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to