nandakumar131 commented on code in PR #5455:
URL: https://github.com/apache/ozone/pull/5455#discussion_r1389018881
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAManagerStub.java:
##########
@@ -214,6 +214,11 @@ public boolean triggerSnapshot() throws IOException {
throw new IOException("submitSnapshotRequest is called.");
}
+ @Override
+ public void updateRaftPeerPriority(String peerId) throws IOException {
+ return;
Review Comment:
There is no need for `return` statement here.
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/safemode/SCMSafeModeManager.java:
##########
@@ -249,8 +254,39 @@ public void exitSafeMode(EventPublisher eventQueue) {
// TODO: Remove handler registration as there is no need to listen to
// register events anymore.
-
emitSafeModeStatus();
+ updateRaftPeerPriority();
+ }
+
+ /**
+ * If SCM HA is configured, update priority of the RaftPeer for the current
+ * SCM as it is out of safeMode. This ensures that if all the Raft peers have
+ * up-to-date indexes,the peer with the higher priority is elected as the
+ * leader.
+ */
+ private void updateRaftPeerPriority() {
+ if (scmHaManager != null && scmHaManager.getRatisServer() != null) {
+ RaftServer.Division division =
+ scmHaManager.getRatisServer().getDivision();
+ if (division != null) {
+ RaftPeer self = division.getPeer();
+ String selfPeerId = self.getId().toString();
+ boolean success = true;
+ try {
+ scmHaManager.getRatisServer().updateRaftPeerPriority(selfPeerId);
+ } catch (Exception exception) {
+ success = false;
+ LOG.info("Could not update priority for peer {} {}", selfPeerId,
+ exception);
+ }
+ if (!success) {
+ LOG.error("Could not update priority for peer {}", selfPeerId);
+ } else {
+ LOG.info("Successfully updated Raft peer priority for peer : {}",
+ selfPeerId);
+ }
Review Comment:
```suggestion
try {
scmHaManager.getRatisServer().updateRaftPeerPriority(selfPeerId);
LOG.info("Successfully updated Raft peer priority for peer : {}",
selfPeerId);
} catch (Exception exception) {
LOG.error("Could not update priority for peer {}", selfPeerId,
exception);
}
```
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMRatisServerImpl.java:
##########
@@ -338,6 +340,49 @@ public boolean addSCM(AddSCMRequest request) throws
IOException {
}
}
+ public void updateRaftPeerPriority(String peerId) throws IOException {
+ List<RaftPeer> raftPeers =
+ new ArrayList<>(getDivision().getGroup().getPeers());
+ List<RaftPeer> oldPeers = new ArrayList<>(raftPeers);
+ RaftPeer peerToUpdate = raftPeers.stream()
+ .filter(peer -> peer.getId().toString().equals(peerId)).findFirst()
+ .get();
+ RaftPeer newPeer =
RaftPeer.newBuilder(peerToUpdate).setPriority(1).build();
+ raftPeers.add(newPeer);
+ raftPeers.remove(peerToUpdate);
+ final SetConfigurationRequest configRequest =
+ new SetConfigurationRequest(clientId, division.getInfo().getLeaderId(),
+ division.getGroup().getGroupId(), nextCallId(), raftPeers);
+ RaftClient raftClient = null;
+ try {
+ RaftClient.Builder clientBuilder = RaftClient.newBuilder().setRaftGroup(
+ RaftGroup.valueOf(division.getGroup().getGroupId(), oldPeers))
+ .setProperties(new RaftProperties());
+ boolean success = false;
+ int retries = 0;
+ raftClient = clientBuilder.build();
+ while (!success && retries < UPDATE_PEER_PRIORITY_RETRY_COUNT) {
Review Comment:
We don't need a retry logic for updating the priority. If this call fails,
it means that this SCM is not able to communicate with other SCMs. So there is
no point in updating the priority of this SCM.
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java:
##########
@@ -249,6 +249,10 @@ public int getNodeCount(NodeStatus nodeStatus) {
return nodeStateManager.getNodeCount(nodeStatus);
}
+ public SCMStorageConfig getScmStorageConfig() {
+ return scmStorageConfig;
+ }
+
Review Comment:
This method is not used anywhere. Do we need this?
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMRatisServerImpl.java:
##########
@@ -338,6 +340,49 @@ public boolean addSCM(AddSCMRequest request) throws
IOException {
}
}
+ public void updateRaftPeerPriority(String peerId) throws IOException {
+ List<RaftPeer> raftPeers =
+ new ArrayList<>(getDivision().getGroup().getPeers());
+ List<RaftPeer> oldPeers = new ArrayList<>(raftPeers);
+ RaftPeer peerToUpdate = raftPeers.stream()
+ .filter(peer -> peer.getId().toString().equals(peerId)).findFirst()
+ .get();
Review Comment:
`findFirst` returns an `Optional` object, which can be `null`. Calling `get`
can result in `NoSuchElementException`.
--
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]
