[
https://issues.apache.org/jira/browse/HDDS-17220?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Siyao Meng updated HDDS-17220:
------------------------------
Attachment: TestBugCR2QueuedRecommissionDropsAdminRequest.java
> Decommission or maintenance requested while a recommission is still queued is
> acknowledged and then lost, and the datanode returns to IN_SERVICE
> ------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: HDDS-17220
> URL: https://issues.apache.org/jira/browse/HDDS-17220
> Project: Apache Ozone
> Issue Type: Bug
> Reporter: Siyao Meng
> Priority: Major
> Attachments:
> CR-2-withdraw-queued-recommission-on-new-admin-request.patch,
> TestBugCR2QueuedRecommissionDropsAdminRequest.java
>
>
> h3. Mechanism
> {{NodeDecommissionManager.recommission}} does not change the op state of the
> node. It calls {{DatanodeAdminMonitorImpl.stopMonitoring}}, which queues the
> node in {{cancelledNodes}}, and the next monitor run
> ({{processCancelledNodes}}, every
> {{ozone.scm.datanode.admin.monitor.interval}}, default 30s) puts the node
> back to IN_SERVICE and stops tracking it.
> Until that run the node is still DECOMMISSIONING, DECOMMISSIONED,
> ENTERING_MAINTENANCE or IN_MAINTENANCE. {{startDecommission}} and
> {{startMaintenance}} look only at the op state, so a new request for the same
> kind of state takes the "Nothing to do" branch: no error is returned and
> {{monitor.startMonitoring}}, which removes a queued cancel, is not called.
> The queued recommission then wins.
> h3. Trigger
> # {{ozone admin datanode decommission <dn>}} (or {{maintenance}}), and the
> monitor runs.
> # {{ozone admin datanode recommission <dn>}}.
> # Within the same monitor interval, {{ozone admin datanode decommission
> <dn>}} again (with or without {{--force}}), or {{maintenance}}.
> # The next monitor run puts the node IN_SERVICE and stops tracking it.
> The same happens for a DECOMMISSIONED node and when the recommission is
> queued before the monitor has picked up the node. A recommission followed by
> a request of the other kind (for example maintenance, recommission,
> decommission) is not lost silently: it is rejected with "Cannot decommission
> node ... in state ENTERING_MAINTENANCE", and the node returns to IN_SERVICE.
> h3. Impact
> * The request returns an empty error list, so the CLI reports success, but
> the node ends IN_SERVICE and untracked. No later monitor run, heartbeat or
> leader change brings it back, because it is now IN_SERVICE.
> * An operator who then stops the node believing it is in maintenance gets a
> normal dead node, and SCM re-replicates its containers (argued from code, not
> run). No data is at risk.
> h3. Reproduction
> PASS on the unmodified source, deterministic.
> [^TestBugCR2QueuedRecommissionDropsAdminRequest.java]
> ({{hadoop-hdds/server-scm}}) uses the setup of
> {{TestNodeDecommissionManager}}: a real {{SCMNodeManager}} with registered
> datanodes and a real {{NodeDecommissionManager}} (with a mocked
> {{ReplicationManager}}, so it also runs on master), called through the
> {{decommissionNodes}}, {{startMaintenanceNodes}} and {{recommissionNodes}}
> methods that {{SCMClientProtocolServer}} calls. The scheduled monitor is set
> to 1h and the monitor is run by hand in place of the 30s interval. Output on
> the pin:
> {noformat}
> monitor run after decommission: state=DECOMMISSIONING tracked=true
> recommission (queued): state=DECOMMISSIONING tracked=true
> decommission again, errors=0: state=DECOMMISSIONING tracked=true
> monitor run after the new request: state=IN_SERVICE tracked=false
> monitor run after maintenance: state=ENTERING_MAINTENANCE tracked=true
> recommission (queued): state=ENTERING_MAINTENANCE tracked=true
> maintenance again, errors=0: state=ENTERING_MAINTENANCE tracked=true
> monitor run after the new request: state=IN_SERVICE tracked=false
> {noformat}
> A control with a monitor run between the recommission and the new request
> ends ENTERING_MAINTENANCE and tracked. A passing test means the defect is
> present. With the patch the two defect tests fail, because the node stays
> DECOMMISSIONING or ENTERING_MAINTENANCE and tracked, and the control passes.
> Not run on a cluster with real datanodes.
> h3. Patch
> [^CR-2-withdraw-queued-recommission-on-new-admin-request.patch], against
> b0aa6475b7856d2c7f467dd3db12b9a1dd116f63. It also applies to master at
> 20d95ed37fe.
> {{DatanodeAdminMonitor}} gets {{withdrawStopMonitoring}}, which removes every
> recommission queued for the node under the monitor lock and reports whether
> there was one. More than one can be queued, because the host list of a
> command is not deduplicated and because two commands can arrive in one
> monitor interval. {{startDecommission}} and {{startMaintenance}} call it
> before they read the node status. If a recommission was withdrawn and the
> node is already in the requested kind of admin state, the node keeps its
> state, which is not written, and {{startMonitoring}} is called again unless
> the node is DECOMMISSIONED (that node is not tracked, and keeping it
> DECOMMISSIONED is enough). Keeping the state also keeps a stopped
> IN_MAINTENANCE node in maintenance. A request of the other kind is still
> rejected, and the recommission is queued again, so its outcome does not
> change. Requests without a queued recommission are unchanged.
> Op state and the monitor queues live in the leader SCM's memory and are not
> in the Ratis log, so a failover while the recommission is queued loses the
> recommission, before and after this patch.
> Covered by {{testAdminRequestReplacesQueuedRecommission}} in the existing
> {{TestNodeDecommissionManager}}: decommissioning, entering maintenance and
> dead IN_MAINTENANCE nodes keep their state and stay tracked, a node whose
> recommission was queued before the monitor picked it up is tracked again, a
> DECOMMISSIONED node stays DECOMMISSIONED and untracked whether its
> recommission was queued once or twice, and a decommission of a node in
> maintenance is rejected and the node returns to IN_SERVICE. Without the
> change it fails with {{expected: <DECOMMISSIONING> but was: <IN_SERVICE>}}.
> With it {{TestNodeDecommissionManager}}, {{TestDatanodeAdminMonitor}},
> {{TestNodeDecommissionMetrics}} and {{TestSCMNodeManager}} (113 tests) pass,
> {{TestDecommissionAndMaintenance}} (9 tests, mini cluster) passes with this
> patch alone, and checkstyle is clean.
> Found by code review of the SCM datanode decommission and maintenance admin
> paths, as part of the TLA+ verification effort 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 and Claude
> Opus 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]
