rpuch commented on code in PR #1653:
URL: https://github.com/apache/ignite-3/pull/1653#discussion_r1101213331
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/raft/ValidationManager.java:
##########
@@ -146,46 +120,33 @@ ValidationResult validateNode(
}
boolean isNodeValidated(ClusterNode node) {
- return storage.isNodeValidated(node.id()) ||
logicalTopology.isNodeInLogicalTopology(node);
- }
-
- /**
- * Checks and removes the node from the list of validated nodes thus
completing the validation procedure.
- *
- * @param node Node that wishes to join the logical topology.
- */
- void completeValidation(ClusterNode node) {
- Future<?> cleanupFuture = cleanupFutures.remove(node.id());
-
- if (cleanupFuture != null) {
- cleanupFuture.cancel(false);
- }
-
- storage.removeValidatedNode(node.id());
+ return storage.isNodeValidated(node) ||
logicalTopology.isNodeInLogicalTopology(node);
}
- private void putValidatedNode(ClusterNode node) {
- storage.putValidatedNode(node.id());
+ void putValidatedNode(ClusterNode node) {
+ storage.putValidatedNode(node);
- scheduleValidatedNodeRemoval(node.id());
+ logicalTopology.onNodeValidated(node);
}
- private void scheduleValidatedNodeRemoval(String nodeId) {
- Future<?> future = executor.schedule(() -> {
- LOG.info("Removing node from the list of validated nodes since no
JoinReady requests have been received [node={}]", nodeId);
-
- cleanupFutures.remove(nodeId);
+ void removeValidatedNodes(Collection<ClusterNode> nodes) {
+ Set<ClusterNode> validatedNodes = storage.getValidatedNodes();
- storage.removeValidatedNode(nodeId);
- }, configuration.incompleteJoinTimeout().value(),
TimeUnit.MILLISECONDS);
+ nodes.forEach(node -> {
+ if (validatedNodes.contains(node)) {
Review Comment:
This reasoning implies that same node ID can only correspond to one address.
This seems reasonable, but, if we switch to using just an ID, we don't need to
imply anything. The code will be easier to reason about and, if, for some
unknown reason (like switching to a different thing instead of SWIM?) that we
cannot imagine now, the 'one address for same ID' is now longer true, we will
not be affected (as we would not imply anything).
It seems that the change is not too heavy to implement it and sleep
patiently :)
--
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]