errose28 commented on code in PR #9836:
URL: https://github.com/apache/ozone/pull/9836#discussion_r2881289412
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStateManager.java:
##########
@@ -755,41 +704,9 @@ public void run() {
scheduleNextHealthCheck();
}
- /**
- * Upgrade finalization needs to move all nodes to a healthy readonly state
- * when finalization finishes to make sure no nodes with metadata layout
- * version older than SCM's are used in pipelines. Pipeline creation is
- * still frozen at this point in the finalization flow.
- *
- * This method is synchronized to coordinate node state updates between
- * the upgrade finalization thread which calls this method, and the
- * node health processing thread that calls {@link #checkNodesHealth}.
- */
- public synchronized void forceNodesToHealthyReadOnly() {
- try {
- List<DatanodeInfo> nodes = nodeStateMap.getDatanodeInfos(null, HEALTHY);
- for (DatanodeInfo node : nodes) {
- nodeStateMap.updateNodeHealthState(node.getID(),
- HEALTHY_READONLY);
- if (state2EventMap.containsKey(HEALTHY_READONLY)) {
- // At this point pipeline creation is already frozen and the node's
- // state has been updated in nodeStateMap. This event should be a
- // no-op aside from logging a message, so it is ok to complete
- // asynchronously.
- eventPublisher.fireEvent(state2EventMap.get(HEALTHY_READONLY),
- node);
- }
- }
-
- } catch (NodeNotFoundException ex) {
- LOG.error("Inconsistent NodeStateMap! {}", nodeStateMap);
- }
- }
-
/**
* This method is synchronized to coordinate node state updates between
- * the upgrade finalization thread which calls
- * {@link #forceNodesToHealthyReadOnly}, and the node health processing
+ * the upgrade finalization thread and the node health processing
Review Comment:
We should be able to safely remove `synchronized` and the comment. This
method is only called by
[run](https://github.com/apache/ozone/blob/61260ebfbd95c6e537c7cd1a0bdc8ff839934a31/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStateManager.java#L653)
which runs on a single thread and it is the only `synchronized` method in this
class, so it is not coordinating with anything.
The comment is incorrect because the finalization thread (which we are
removing anyways) actually calls
[getNodeStatus](https://github.com/apache/ozone/blob/bb546a28374c1b5e06feebb9300b8c734b79bb4e/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/upgrade/SCMUpgradeFinalizer.java#L151).
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
