Siyao Meng created HDDS-17220:
---------------------------------
Summary: 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
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]
