pvillard31 commented on code in PR #11331:
URL: https://github.com/apache/nifi/pull/11331#discussion_r3400572158
##########
nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/StandardFlowService.java:
##########
@@ -719,32 +727,34 @@ private void offload(final String explanation) throws
InterruptedException {
}
}
- private void handleDisconnectionRequest(final DisconnectMessage request) {
+ private void handleDisconnectionRequest(final DisconnectMessage request,
final long expectedConnectionGeneration) {
logger.info("Received disconnection request message from cluster
coordinator with explanation: {}", request.getExplanation());
- disconnect(request.getExplanation());
+
+ writeLock.lock();
Review Comment:
The new test covers the coordinator cancelling the pending disconnect, but
not this node-side generation guard that ignores a stale disconnect after
reconnect. Could we add a unit or system test for this path?
##########
nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/StandardFlowService.java:
##########
@@ -600,6 +603,11 @@ private NodeIdentifier getNodeId() {
}
}
+ // Visible for testing
+ long getConnectionGeneration() {
Review Comment:
`getConnectionGeneration()` is marked Visible for testing, but no test seems
to reference it. Is it still needed, or can it be removed?
##########
nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/main/java/org/apache/nifi/cluster/coordination/node/NodeClusterCoordinator.java:
##########
@@ -1297,6 +1311,19 @@ private ConnectionResponseMessage
handleConnectionRequest(final ConnectionReques
return createConnectionResponse(requestWithNodeIdentities,
nodeIdentifier);
}
+ private void cancelPendingDisconnection(final NodeIdentifier
nodeIdentifier) {
+ final Future<Void> pendingDisconnect =
pendingDisconnections.remove(nodeIdentifier.getId());
+ if (pendingDisconnect != null) {
+ final boolean cancelled = pendingDisconnect.cancel(true);
Review Comment:
Since `CompletableFuture.cancel(true)` does not interrupt a running thread,
a disconnect worker that is inside `Thread.sleep(retrySeconds * 1000L)` keeps
sleeping until the next loop iteration. Should this log report the disconnect
as cancelled when the worker thread may still be running?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
