Bryan Bende created NIFI-16536:
----------------------------------
Summary: Decommission leaves node stranded outside the cluster
when it is the only connected node
Key: NIFI-16536
URL: https://issues.apache.org/jira/browse/NIFI-16536
Project: Apache NiFi
Issue Type: Bug
Reporter: Bryan Bende
h3. Summary
When a node is decommissioned (for example via \{{nifi.sh decommission}}, which
sends DELETE \{{/health/cluster}} to the management server) while it is the
only node in the CONNECTED state, \{{ClusterDecommissionTask}} fails with
\{{IllegalNodeDisconnectionException}} after it has already stopped
heartbeating and marked the FlowController as not clustered. Nothing rolls
those changes back, so the node is left outside the cluster: it never sends a
connection request again, it never offloads, and it does not rejoin after other
nodes reconnect. Only a restart of the node recovers it.
This is easy to hit in orchestrated environments (Kubernetes scale-down,
rolling restarts): the decommission of one node coincides with another node
restarting, so the node being decommissioned is briefly the only connected node.
Steps to reproduce
# Start a two-node cluster (node A and node B).
# Stop or restart node A so that node B is the only CONNECTED node.
# On node B, run \{{bin/nifi.sh decommission --shutdown=false}}.
# Start node A again and let it connect.
*Expected:* the decommission is refused or rolled back, and node B remains (or
returns to being) a connected cluster member.
*Actual:* node B logs the exception below, logs "Cluster State changed from
Clustered to Not Clustered", stops heartbeating, and stays out of the cluster
indefinitely. After node A connects, node B only keeps logging that the Cluster
Coordinator is located at node A; it never reconnects, offloads, or is removed.
Its \{{/health/cluster}} status remains non-CONNECTED, so it also stays not
ready for any readiness probe built on that endpoint.
{code}
ERROR org.apache.nifi.runtime.StandardUncaughtExceptionHandler Uncaught
Exception in Thread [DecommissionTask]
org.apache.nifi.cluster.manager.exception.IllegalNodeDisconnectionException:
Cannot disconnect node <node-b>:8443 because it is the only node currently
connected
at
org.apache.nifi.cluster.coordination.node.NodeClusterCoordinator.requestNodeDisconnect(NodeClusterCoordinator.java:577)
at
org.apache.nifi.cluster.lifecycle.ClusterDecommissionTask.disconnectNode(ClusterDecommissionTask.java:102)
at
org.apache.nifi.cluster.lifecycle.ClusterDecommissionTask.decommission(ClusterDecommissionTask.java:75)
at
org.apache.nifi.runtime.HealthClusterHttpHandler.lambda$startDecommission$0(HealthClusterHttpHandler.java:92)
{code}
h3. Cause
* \{{ClusterDecommissionTask.decommission()}} calls
\{{flowController.stopHeartbeating()}} and
\{{flowController.setClustered(false, null)}} before \{{disconnectNode()}}.
* \{{NodeClusterCoordinator.requestNodeDisconnect()}} throws
\{{IllegalNodeDisconnectionException}} *synchronously* when the target is the
only CONNECTED node. The retry loop in \{{disconnectNode()}} only catches
\{{ExecutionException}} from \{{future.get()}}, so this exception escapes the
loop and ends the task.
* \{{HealthClusterHttpHandler.startDecommission()}} runs the task on a virtual
thread that only handles \{{InterruptedException}}, so the failure is only
logged by the uncaught exception handler. The heartbeat and clustered state are
never restored.
* \{{offloadNode()}} has the same pattern: \{{requestNodeOffload()}} can throw
\{{IllegalNodeOffloadException}} synchronously, and that exception would also
escape the loop.
h3. Proposed changes
# *Fail fast:* before stopping heartbeats, check whether the local node is the
only CONNECTED node
(\{{clusterCoordinator.getNodeIdentifiers(NodeConnectionState.CONNECTED)}}). If
it is, log a clear warning and abort the decommission without changing any
local state.
# *Roll back on failure:* catch \{{RuntimeException}} from
\{{requestNodeDisconnect()}} and \{{requestNodeOffload()}}. For non-retryable
failures (\{{IllegalNodeDisconnectionException}},
\{{IllegalNodeOffloadException}}), restore the clustered state and heartbeating
(or request reconnection for the local node) so the node stays a healthy
cluster member instead of being stranded. The check in (1) narrows the window
but cannot close it, so (2) is still needed.
# *Single decommission at a time:* each DELETE on \{{/health/cluster}} starts a
new virtual thread. Because \{{decommission()}} is synchronized, repeated
requests queue up and run again after the first one finishes. A request while a
decommission is already running should return 202 (or 409) without starting
another task.
# *Report the outcome:* GET \{{/health/cluster}} only reports the connection
state, so a caller that starts a decommission cannot tell that it failed and
will poll indefinitely. Adding the decommission outcome to the response, for
example a second line \{{Decommission Status: IN_PROGRESS | FAILED: <reason> |
COMPLETED}}, lets tools that drive decommission through \{{nifi.sh
decommission}} and \{{nifi.sh cluster-status}} react to a failure. The existing
\{{Cluster Status:}} line stays unchanged for compatibility.
Items 1 and 2 fix the bug. Items 3 and 4 are related hardening and could be
split into a separate improvement.
h3. Testing
There are currently no unit tests for \{{ClusterDecommissionTask}} or
\{{HealthClusterHttpHandler}}. A new \{{ClusterDecommissionTaskTest}} with a
mocked \{{ClusterCoordinator}} and \{{FlowController}} could cover these cases:
* the only-connected-node case aborts without calling \{{stopHeartbeating()}};
* a synchronous \{{IllegalNodeDisconnectionException}} restores the clustered
state and heartbeating;
* the normal path still disconnects, offloads, and removes the node.
{code}
{code}
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
