[ 
https://issues.apache.org/jira/browse/HDDS-17070?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Siyao Meng updated HDDS-17070:
------------------------------
    Attachment: CR-1-sync-suppressed-flag-from-scm.patch

> Recon does not follow container suppression changes made in SCM after it has 
> added the container
> ------------------------------------------------------------------------------------------------
>
>                 Key: HDDS-17070
>                 URL: https://issues.apache.org/jira/browse/HDDS-17070
>             Project: Apache Ozone
>          Issue Type: Bug
>            Reporter: Siyao Meng
>            Priority: Minor
>         Attachments: CR-1-sync-suppressed-flag-from-scm.patch, 
> TestBugCR1SuppressedFlagNotRefreshed.java
>
>
> h3. Mechanism
> Container suppression (HDDS-14103) is set and cleared in SCM with {{ozone 
> admin container report --suppress}} / {{--unsuppress}}. HDDS-14103 states 
> that Recon can use it "to keep its missing container alerts in sync with 
> SCM". Recon's health sweep ({{ReconReplicationManager}}, through 
> {{ReplicationManager.processContainer}}) already skips suppressed containers, 
> using Recon's own copy of {{ContainerInfo}}.
> Recon copies the flag from SCM only when it first adds a container or 
> installs a full SCM DB snapshot. The periodic sync 
> ({{ReconStorageContainerSyncHelper.syncWithSCMContainerInfo}}, every 6 hours 
> by default) works from container IDs and lifecycle states only: 
> {{reconcileExistingContainer}} returns as soon as the states match, and never 
> compares the flag. A full snapshot happens only at startup, when the 
> container count drift exceeds {{ozone.recon.scm.container.threshold}} 
> (default 1,000,000). HDDS-14103 changed no Recon code.
> h3. Trigger
> * An admin suppresses a container in SCM, typically a MISSING one, after 
> Recon has added it. This is the normal order, since Recon learns of a 
> container when it is created.
> * The reverse: an admin unsuppresses a container that Recon added while SCM 
> had it suppressed (a new or rebuilt Recon, or a snapshot install).
> h3. Impact
> * A container suppressed in SCM stays unsuppressed in Recon, so Recon keeps 
> reporting it as MISSING (or under replicated) while {{ozone admin container 
> report}} hides it. This follows from the code above; the patch's tests cover 
> it.
> * A container unsuppressed in SCM stays suppressed in Recon, and Recon's 
> health sweep skips it. If it has lost all its replicas, Recon writes no 
> MISSING row for it. In the attached run, after a full successful periodic 
> sync Recon still had the container suppressed while SCM did not, and the 
> sweep wrote a MISSING row only for the other container, although both had no 
> replica.
> The divergence has no time limit in normal operation.
> h3. Reproduction
> [^TestBugCR1SuppressedFlagNotRefreshed.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}}, the 
> real sync helper and the real {{ReconReplicationManager}} health check chain. 
> Only the SCM RPC peer is mocked, returning what SCM returns after an 
> unsuppress, and the health schema manager is mocked to capture the rows the 
> sweep writes.
> h3. Patch
> [^CR-1-sync-suppressed-flag-from-scm.patch], against 
> 916aa1d9949cbb10f2750a40c8704340893131a2. It also applies to master at 
> d77edaa6c15.
> * The periodic sync gains one step: it pages through SCM's {{listContainer}} 
> with the existing suppressed filter, collects the suppressed IDs, then sets 
> the flag on every Recon container where it differs, in both directions. If 
> the listing fails, nothing is changed and the sync reports failure like the 
> other passes. An SCM that ignores the filter returns unsuppressed containers; 
> since the filter came with the flag, such an SCM has no suppressed 
> containers, so the step stops after the first page. The cost is one 
> {{listContainer}} pass per sync cycle; SCM evaluates the filter by scanning 
> its container map.
> * {{ReplicationManager.processContainer}} returns before it updates the 
> health state of a suppressed container, so a container that was MISSING when 
> it was suppressed kept that state in memory and Recon kept writing its 
> MISSING row until a restart. {{ReconReplicationManager.processAll}} now 
> resets the health state of a suppressed container to HEALTHY before that 
> call, so no health row is written for it. The shared {{processContainer}} and 
> SCM are not changed.
> Covered by three new cases in 
> {{TestReconSCMContainerSyncIntegration.SuppressedFlagSyncTests}} (flags 
> copied in both directions; one listing call against an SCM that ignores the 
> filter; flags unchanged when the listing fails) and by 
> {{TestReconReplicationManager.testProcessAllFollowsSuppressedFlag}} (a 
> MISSING container gets no row once suppressed and gets it back once 
> unsuppressed). Without the change the sync case fails on the first flag and 
> the sweep case still finds the MISSING row after suppression. With it the 
> Recon {{scm}} and {{fsck}} unit tests pass, checkstyle is clean, and the 
> attached test no longer shows the bug.
> A failing health sweep also retries without waiting; that is a separate issue 
> with its own patch.
> 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-14103 (resolved, added container suppression), HDDS-13891 
> (resolved, the current Recon health task). 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