[
https://issues.apache.org/jira/browse/HDDS-16895?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-16895:
------------------------------
Attachment: TestBugMC1RefreshSubtractsReportedContainers.java
> SCM can leave safe mode below the container threshold when a container report
> is processed during the container rule refresh
> ----------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-16895
> URL: https://issues.apache.org/jira/browse/HDDS-16895
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Minor
> Attachments:
> MC-1-count-only-containers-removed-by-deleted-scan.patch,
> TestBugMC1RefreshSubtractsReportedContainers.java
>
>
> h3. Mechanism
> {{AbstractContainerSafeModeRule.reinitializeRule}} removes DELETED containers
> from the rule's container map and then lowers {{totalContainers}} by the
> difference between the map size before and after the scan. The refresh holds
> the rule's lock, but report processing does not:
> {{SafeModeExitRule.onMessage}} calls {{process()}} between two {{validate()}}
> calls, and {{handleReportedContainer}} removes each reported container from
> the same map and increments {{containersWithMinReplicas}}. Every container
> that a report removes while the refresh is between its two size reads is
> therefore counted as reported and also subtracted from the total. The ratio
> can exceed 1.0, the rule validates, and once the other rules pass SCM leaves
> safe mode. The exit is not reversed.
> {{ECContainerSafeModeRule.handleReportedContainer}} has a second, smaller gap
> of the same kind: it increments the count without checking that its own
> {{remove()}} removed the container, so an EC container removed by the refresh
> between {{containsKey}} and {{remove}} is still counted (code reading, not
> run).
> h3. Trigger
> # SCM starts in safe mode. The {{safemode-refresh-thread}} calls
> {{refresh(false)}} on every rule every
> {{hdds.scm.safemode.rule.refresh.interval}} (default 5s) while the rule is
> not met.
> # A refresh starts while a datanode's registration container report is being
> processed.
> No fault and no operator action is needed. {{refresh()}} and {{validate()}}
> are synchronized on the rule, and {{onMessage}} calls {{validate()}} before
> {{process()}}, so a report that arrives during a refresh waits for it. Only
> the one report already inside {{process()}} when the refresh starts can
> overlap, and the chance of a hit per registration report is roughly that
> report's processing time divided by the refresh interval. The window itself
> is the DELETED lookup (a copy of the DELETED containers under the container
> state read lock) plus the removal loop: about 50ms per 100,000 DELETED
> records in a unit timing with a mocked {{ContainerManager}}. It decides how
> much of the report in progress fits inside the window.
> h3. Impact
> * SCM leaves safe mode with fewer containers reported than
> {{hdds.scm.safemode.threshold.pct}} (default 0.99). Each refresh can subtract
> at most the containers that the one report in progress when it starts removes
> during the refresh, because the next report waits for the rule lock in
> {{validate()}}. Successive refreshes add up. At the default 5s interval a hit
> is rare, and the expected shortfall is estimated at a few percent of the
> containers below the threshold (estimated from the code, not measured). A
> single hit on a large early report in a small cluster can cost more, up to
> that report's share of the containers. The other rules, mainly the 90% one
> replica pipeline rule, still need most datanodes registered before SCM can
> exit.
> * On exit, block allocation and {{allocateContainer}} stop returning
> {{SAFE_MODE_EXCEPTION}}, and {{getContainerWithPipeline}} stops refusing open
> containers without enough replicas. ReplicationManager, block deletion and
> the pipeline scrubber start after {{hdds.scm.wait.time.after.safemode.exit}}
> (default 5m). The background pipeline creator already runs in safe mode once
> the pre check passes ({{hdds.scm.safemode.pipeline.creation}}, default true),
> so the exit does not change it. Containers whose replicas have still not been
> reported by then are handled as missing or under replicated (argued from the
> code, not run). No data loss is expected.
> * The subtraction was added by HDDS-15238 and is in 2.2.0 and 2.2.1. There
> the refresh runs from {{SCMStateMachine.applyTransaction}} while in safe mode
> instead of from the periodic thread, so the same race should be reachable
> (argued, not run on 2.2.x).
> h3. Reproduction
> [^TestBugMC1RefreshSubtractsReportedContainers.java]
> ({{hadoop-hdds/server-scm}}) uses the real {{SCMSafeModeManager}}, its
> refresh thread (interval lowered to 100ms) and the EventQueue handler of
> {{RatisContainerSafeModeRule}}, with a mocked {{ContainerManager}} holding 3
> closed containers. One datanode reports 2 of them. Two latches only order the
> threads: the handler waits just before {{process()}} until the refresh is
> inside the DELETED lookup, and the refresh waits there until the report is
> processed. On the unmodified commit it fails every time with {{SCM left safe
> mode with 2 of 3 containers reported}}, and the log shows {{Refreshed RATIS
> Containers threshold count to 1.}} followed by {{SCM exiting safe mode.}}
> With the patch it passes.
> Without any ordering, a stress run (20,000 closed containers, 200,000 DELETED
> containers in the mock, one report of 19,700 containers, i.e. 98.5%, fired at
> a random offset to a 100ms refresh) left safe mode in 6 of 20 trials on the
> unmodified commit and in 0 of 20 with the patch. In the failing trials the
> total fell to as little as 5,132 of 20,000. That size comes from the stress
> settings: a 100ms interval and a DELETED lookup of about 100ms (200,000
> records) against one report of 19,700 containers, so a large part of that
> single report fell inside refresh windows. It is not a typical shortfall. The
> stress harness is not attached. Not run at the default 5s interval or on a
> real cluster.
> h3. Patch
> [^MC-1-count-only-containers-removed-by-deleted-scan.patch], against
> b0aa6475b7856d2c7f467dd3db12b9a1dd116f63. It also applies to master at
> 6818707698c.
> {{reinitializeRule}} now subtracts only the containers that its own
> {{containers.remove()}} removed (non null return) instead of the change in
> map size. Each container is then removed exactly once, either by a report
> (counted, kept in the total) or by the DELETED scan (subtracted, not
> counted). {{ECContainerSafeModeRule.handleReportedContainer}} increments only
> when its {{remove()}} returned a value, as the Ratis rule already does.
> Report processing stays outside the rule lock. Making {{process()}}
> synchronized would also close the race, but it would hold the lock for a
> whole registration report and block {{validate()}}, the safe mode status call
> and the refresh meanwhile.
> Covered by {{testRefreshKeepsContainersReportedDuringRefresh}} in the
> existing {{AbstractContainerSafeModeRuleTest}}, which runs for the Ratis and
> the EC rule. A report is processed inside the mocked DELETED lookup, and the
> test expects only the DELETED container to leave the total. Without the
> change it fails for both rules ({{expected: <2> but was: <1>}}). With it, the
> {{org.apache.hadoop.hdds.scm.safemode}} unit tests (76) and
> {{TestSCMSafeModeWithPipelineRules}} on a mini cluster (2) pass, and
> checkstyle is clean.
> Found by TLA+ model checking and code review of the SCM safe mode rules under
> HDDS-15926, on commit b0aa6475b7856d2c7f467dd3db12b9a1dd116f63. 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]
