ivandika3 commented on code in PR #9385:
URL: https://github.com/apache/ozone/pull/9385#discussion_r2616738214
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/SCMConnectionManager.java:
##########
@@ -234,6 +235,7 @@ public void removeSCMServer(InetSocketAddress address)
throws IOException {
}
EndpointStateMachine endPoint = scmMachines.get(address);
Review Comment:
Thanks for the review. Although scmMachines are protected by read and write
lock, I think it's better to remove the endpoint from the `scmMachines` first.
If there are no locks, there might be issues where
`SCMConnectionManager#getValues` might return `EndpointStateMachine` that was
shutdown which might trigger DN shutdown (see `RunningDatanodeState#execute`).
One thing I find odd is the additional copy in
`SCMConnectionManager#getValues` and that the copy are used
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/SCMConnectionManager.java:
##########
@@ -234,6 +235,7 @@ public void removeSCMServer(InetSocketAddress address)
throws IOException {
}
EndpointStateMachine endPoint = scmMachines.get(address);
Review Comment:
Thanks for the review. Although scmMachines are protected by read and write
lock, I think it's better to remove the endpoint from the `scmMachines` first.
If there are no locks, there might be issues where
`SCMConnectionManager#getValues` might return `EndpointStateMachine` that was
shutdown which might trigger DN shutdown (see `RunningDatanodeState#execute`).
--
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]
