hanishakoneru commented on a change in pull request #2886:
URL: https://github.com/apache/ozone/pull/2886#discussion_r767013749
##########
File path:
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/OzoneManager.java
##########
@@ -1596,37 +1602,66 @@ public void bootstrap(OMNodeDetails newOMNode) throws
IOException {
/**
* When OMStateMachine receives a configuration change update, it calls
- * this function to update the peers list, if required.
- */
- public void updatePeerList(List<String> omNodeIds) {
- List<String> ratisServerPeerIdsList = omRatisServer.getPeerIds();
- for (String omNodeId : omNodeIds) {
- // Check if the OM NodeID is already present in the peer list or its
- // the local NodeID.
- if (!peerNodesMap.containsKey(omNodeId) && !isCurrentNode(omNodeId)) {
+ * this function to update the peers list, if required. The configuration
+ * change could be to add or to remove an OM from the ring.
+ */
+ public void updatePeerList(List<String> newPeers) {
+ List<String> currentPeers = omRatisServer.getPeerIds();
+
+ // NodeIds present in new node list and not in current peer list are the
+ // bootstapped OMs and should be added to the peer list
+ List<String> bootstrappedOMs = new ArrayList<>();
+ bootstrappedOMs.addAll(newPeers);
+ bootstrappedOMs.removeAll(currentPeers);
+
+ // NodeIds present in current peer list but not in new node list are the
+ // decommissioned OMs and should be removed from the peer list
+ List<String> decommissionedOMs = new ArrayList<>();
+ decommissionedOMs.addAll(currentPeers);
+ decommissionedOMs.removeAll(newPeers);
+
+ // Add bootstrapped OMs to peer list
+ for (String omNodeId : bootstrappedOMs) {
+ // Check if its the local nodeId (bootstrapping OM)
+ if (isCurrentNode(omNodeId)) {
+ // For a Bootstrapping OM, none of the peers are added to it's
+ // RatisServer's peer list and it needs to be updated here after
+ // receiving the conf change notification from Ratis.
+ for (String peerNodeId : newPeers) {
+ if (peerNodeId.equals(omNodeId)) {
+ omRatisServer.addRaftPeer(omNodeDetails);
+ } else {
+ omRatisServer.addRaftPeer(peerNodesMap.get(peerNodeId));
+ }
+ }
+ } else {
+ // For other nodes, add bootstrapping OM to OM peer list (which
+ // internally adds to Ratis peer list too)
try {
addOMNodeToPeers(omNodeId);
} catch (IOException e) {
LOG.error("Fatal Error: Shutting down the system as otherwise it " +
"could lead to OM state divergence.", e);
exitManager.forceExit(1, e, LOG);
}
+ }
+ }
+
+ // Remove decommissioned OMs from peer list
+ for (String omNodeId : decommissionedOMs) {
+ if (isCurrentNode(omNodeId)) {
+ // Decommissioning Node should not receive the configuration change
+ // request. Shut it down.
+ String errorMsg = "Shutting down as OM has been decommissioned.";
+ LOG.error("Fatal Error: {}", errorMsg);
+ exitManager.forceExit(1, errorMsg, LOG);
} else {
- // Check if the OMNodeID is present in the RatisServer's peer list
- if (!ratisServerPeerIdsList.contains(omNodeId)) {
- // This can happen on a bootstrapping OM. The peer information
- // would be present in OzoneManager but OMRatisServer peer list
- // would not have the peers list. OMRatisServer peer list of
- // bootstrapping node should be updated after it gets the RaftConf
- // through Ratis.
- if (isCurrentNode(omNodeId)) {
- // OM Ratis server has the current node also in the peer list as
- // this is the Raft Group peers list. Hence, add the current node
- // also to Ratis peers list if not present.
- omRatisServer.addRaftPeer(omNodeDetails);
- } else {
- omRatisServer.addRaftPeer(peerNodesMap.get(omNodeId));
- }
+ // Remove decommissioned node from peer list (which internally
+ // removed from Ratis peer list too)
+ try {
+ removeOMNodeFromPeers(omNodeId);
+ } catch (IOException e) {
+ e.printStackTrace();
Review comment:
Yes, updated.
##########
File path:
hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/protocolPB/OMAdminProtocolClientSideImpl.java
##########
@@ -123,12 +175,47 @@ public OMConfiguration getOMConfiguration() throws
IOException {
}
return omMedatataBuilder.build();
} catch (ServiceException e) {
- LOG.error("Failed to retrieve configuration of OM {}",
- remoteOmNodeDetails.getOMPrintInfo(), e);
+ LOG.error("Failed to retrieve configuration of OM {}", omPrintInfo, e);
}
return null;
}
+ @Override
+ public void decommission(OMNodeDetails removeOMNode) throws IOException {
+ DecommissionOMRequest decommOMRequest = DecommissionOMRequest.newBuilder()
+ .setNodeId(removeOMNode.getNodeId())
+ .setNodeAddress(removeOMNode.getHostAddress())
+ .build();
+
+ DecommissionOMResponse response;
+ try {
+ response = rpcProxy.decommission(NULL_RPC_CONTROLLER, decommOMRequest);
+ } catch (ServiceException e) {
+ OMNotLeaderException notLeaderException =
+ OMFailoverProxyProvider.getNotLeaderException(e);
+ if (notLeaderException != null) {
+ throwException(notLeaderException.getMessage());
+ }
+
+ OMLeaderNotReadyException leaderNotReadyException =
+ OMFailoverProxyProvider.getLeaderNotReadyException(e);
+ if (leaderNotReadyException != null) {
+ throwException(leaderNotReadyException.getMessage());
+ }
+ throw ProtobufHelper.getRemoteException(e);
+ }
+
+ if (!response.getSuccess()) {
+ throwException("Decommission Request to " + omPrintInfo +
Review comment:
Done.
--
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]
