devmadhuu commented on code in PR #6360:
URL: https://github.com/apache/ozone/pull/6360#discussion_r1540467919
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/NodeStateManager.java:
##########
@@ -88,6 +88,10 @@
*/
public class NodeStateManager implements Runnable, Closeable {
+ public void removeNode(DatanodeDetails datanodeDetails) throws
NodeNotFoundException {
+ nodeStateMap.removeNode(datanodeDetails);
+ }
+
Review Comment:
> I think we should avoid adding anything that could affect SCM. I can see
someone doing an SCM change and incorrectly calling this method when it will
probably do something incorrect in that context. I know the Recon/SCM
inheritance model makes this tricky, but can we either keep these changes out
of SCM, or make sure that they work equally as well in an SCM context (if we
were to add a remove node command to SCM in the future, for example).
We want Recon to stop tracking and clear data from memory data structures in
Recon w.r.t that removed node, However above removeNode API should be
supplemented with few more cleanups of maps and should be present in SCM code
because I feel we have same issue in SCM as well , hence if we want to expose
this remove API in SCM context later, it will be easy and flexible. To
completely keep remove API in recon, lot of maps and code will not be available
in Recon code only and we want to avoid code duplication. There is no easy way
out. We can go ahead having this remove node API in inherited SCM code with
due diligence because there are so many APIs already being shared between Recon
and SCM code.
--
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]
