[
https://issues.apache.org/jira/browse/HDDS-17525?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17525:
------------------------------
Affects Version/s: 2.2.1
2.0.0
1.4.0
2.1.2
2.1.0
1.4.1
2.1.1
1.3.0
2.2.0
> Container balancer that stops on its own keeps persisted shouldRun=true if
> the save fails, so a later leader change starts a new run
> ------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17525
> URL: https://issues.apache.org/jira/browse/HDDS-17525
> Project: Apache Ozone
> Issue Type: Bug
> Affects Versions: 1.3.0, 1.4.0, 2.0.0, 1.4.1, 2.1.0, 2.2.0, 2.1.1, 2.1.2,
> 2.2.1
> Reporter: Siyao Meng
> Priority: Minor
> Attachments: MC-2-retry-failed-save-on-balancer-self-stop.patch,
> TestBugMC2TerminalSaveFailureResume.java
>
>
> h3. Mechanism
> When the balancing task stops on its own (INITIALIZATION_FAILED,
> CAN_NOT_BALANCE_ANY_MORE, COMPLETED_ALL_ITERATIONS),
> {{ContainerBalancerTask.tryStopWithSaveConfiguration}} saves
> {{shouldRun=false}} through the Ratis replicated
> {{StatefulServiceStateManager}}. If that save throws, the catch only records
> the stop reason and logs a WARN. It does not retry, and nothing writes
> {{false}} later without an admin command. The task still ends STOPPED, so
> {{ContainerBalancer.getBalancerStatusInfo}} reports {{shouldRun=false}} while
> the persisted value is {{true}}.
> The next {{ContainerBalancer.notifyStatusChanged}} on a leader out of safe
> mode (failover back to this SCM, failover to another SCM, or a restart) reads
> the persisted {{true}} and calls {{start()}}, which starts a new balancing
> run for a run that had already ended.
> For comparison, a failed save in the admin {{stopBalancer}} path is returned
> to the caller and the balancer keeps running, so the admin knows the stop did
> not take effect.
> h3. Trigger
> # An admin starts the balancer ({{shouldRun=true}} is saved).
> # The task stops on its own, for example because the cluster is already
> balanced.
> # The {{shouldRun=false}} write is rejected by Ratis before it is appended
> while the SCM stays leader, for example {{ResourceUnavailableException}}
> (pending write limit, Ratis default 4096 requests or 64MB, not overridden by
> SCM) or {{LeaderSteppingDownException}} during a leadership transfer that
> then fails. {{ScmInvoker}} turns the reply into an {{SCMException}}.
> # Later, after the cluster has drifted, any leader change or restart starts a
> new run.
> Step 3 is narrow under default settings: SCM Ratis writes are synchronous, so
> the pending limit is only reached after commits stall for minutes while the
> leader keeps its majority, and the terminal save has to land then. A save
> that times out while the SCM stays leader commits later and ends durable as
> {{false}}, which is harmless. An exception while applying the entry
> terminates the SCM. These points are from reading the code, not run.
> h3. Impact
> * A balancing run nobody asked for starts after the next leader change or
> restart, with the last admin config, and moves containers.
> * Until then the status command says the balancer will not run, while the
> persisted state says it will. Only the WARN {{Save configuration failed}}
> shows it. Running {{stopBalancer}} fixes the state.
> * For COMPLETED_ALL_ITERATIONS the new run starts at the persisted next
> iteration index. It stops after the start delay without moving data only if
> the index save of the last iteration succeeded, which runs just before the
> terminal save.
> h3. Reproduction
> PASS, deterministic, unmodified source, injected save failure.
> [^TestBugMC2TerminalSaveFailureResume.java] ({{hadoop-hdds/server-scm}}) uses
> a mocked SCM as {{TestContainerBalancer}} does, with {{ContainerBalancer}}
> and {{ContainerBalancerTask}} unmodified on a real {{SCMContext}}. The
> cluster is balanced, so the first iteration fails to initialize, and the
> first {{shouldRun=false}} save throws the {{SCMException}} that wraps a
> {{ResourceUnavailableException}}. After the task stops, status reports
> {{shouldRun=false}} and the persisted value is {{true}} on a leader. The
> cluster is then made unbalanced and a step-down and step-up are driven
> through {{SCMContext.updateLeaderAndTerm}} and {{notifyStatusChanged}}: a new
> task starts and selects one over-utilized and one under-utilized datanode. A
> passing test means the defect is present. A real Ratis rejection was not
> exercised.
> h3. Patch
> [^MC-2-retry-failed-save-on-balancer-self-stop.patch], against
> 9118d5a85a37db8c64404d56703b7bacb040e063. It also applies to master at
> f59f8d3d123.
> {{tryStopWithSaveConfiguration}} retries a failed {{shouldRun=false}} save
> while the task is still running and the SCM is still leader ready and out of
> safe mode, waiting 1 second before the first retry and doubling the wait up
> to 60 seconds. The first failure is logged with its stack trace, later ones
> as one line. It gives up, leaving {{shouldRun=true}} so the next leader
> resumes as on any step down today, when the SCM steps down or enters safe
> mode or the task is stopped by someone else. The task lock is not held while
> waiting, so a step down or an admin stop waits at most for one attempt as
> today and then ends the wait. While it retries, the task stays RUNNING and
> status shows {{shouldRun=true}}, which matches the persisted value, the same
> way a failed admin stop leaves the balancer running. During that time an
> admin {{startBalancer}} is refused as already running, and an admin
> {{stopBalancer}} ends the retry (if its own save succeeds).
> Covered by {{testFailedSaveOnSelfStopIsRetried}} and
> {{testStepDownWhileRetryingFailedSaveOnSelfStop}} in the existing
> {{TestContainerBalancer}}. Without the change the first fails because the
> persisted value is still {{true}} and the second times out because nothing
> retries. With it the balancer unit suites in {{hadoop-hdds/server-scm}} pass
> (437 tests) and checkstyle is clean. With the patch the reproduction fails
> because the persisted value is {{false}} after the stop.
> Not covered: when the save fails because the SCM is stepping down, the next
> leader still resumes the ended run, as in the step-down case today. That run
> starts within minutes on almost the same cluster state, so it usually stops
> again with the same reason.
> Found by TLA+ model checking and code review of the SCM container balancer
> start, stop and failover resume paths under HDDS-15926, on commit
> 9118d5a85a37db8c64404d56703b7bacb040e063. 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
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
