GlenGeng commented on a change in pull request #1953:
URL: https://github.com/apache/ozone/pull/1953#discussion_r582600604



##########
File path: hadoop-hdds/interface-server/src/main/proto/ScmServerProtocol.proto
##########
@@ -54,6 +55,7 @@ message SCMBlockLocationRequest {
   optional DeleteScmKeyBlocksRequestProto     deleteScmKeyBlocksRequest = 12;
   optional hadoop.hdds.GetScmInfoRequestProto getScmInfoRequest         = 13;
   optional SortDatanodesRequestProto          sortDatanodesRequest      = 14;
+  optional hadoop.hdds.AddScmRequestProto      addScmRequestProto       = 15;

Review comment:
       indent

##########
File path: 
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMRatisServerImpl.java
##########
@@ -156,16 +200,52 @@ public void stop() throws IOException {
   @Override
   public NotLeaderException triggerNotLeaderException() {
     return new NotLeaderException(
-        division.getMemberId(), null, division.getGroup().getPeers());
+        getDivision().getMemberId(), null, 
getDivision().getGroup().getPeers());
+  }
+
+  @Override
+  public boolean addSCM(AddSCMRequest request) throws IOException {
+    List<RaftPeer> newRaftPeerList =
+        new ArrayList<>(getDivision().getGroup().getPeers());
+    // add the SCM node to be added to the raft peer list
+
+    RaftPeer raftPeer = RaftPeer.newBuilder().setId(request.getScmId())
+        .setAddress(request.getRatisAddr()).build();
+    newRaftPeerList.add(raftPeer);
+
+    LOG.debug("{}: Submitting SetConfiguration request to Ratis server with" +
+            " new SCM peers list: {}", scm.getScmId(),
+        newRaftPeerList);
+
+    RaftServer.Division division = getDivision();
+    SetConfigurationRequest configRequest =
+        new SetConfigurationRequest(clientId, division.getPeer().getId(),
+            division.getGroup().getGroupId(), nextCallId(), newRaftPeerList);
+
+    try {
+      RaftClientReply raftClientReply =
+          division.getRaftServer().setConfiguration(configRequest);
+      if (raftClientReply.isSuccess()) {
+        LOG.info("Successfully added new SCM: {}.", request.getScmId());
+      } else {
+        LOG.error("Failed to add new SCM: {}. Ratis reply: {}" +
+            request.getScmId(), raftClientReply);

Review comment:
       Need throw exception to trigger failover proxy to try next SCM. 
   fpp consider a response with a false status to be a successful response.

##########
File path: 
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAUtils.java
##########
@@ -131,4 +134,24 @@ public static String getScmServiceId(ConfigurationSource 
conf) {
     }
     return localScmServiceId;
   }
+
+  public static OzoneConfiguration removeSelfId(
+      OzoneConfiguration configuration) {
+    final OzoneConfiguration conf = new OzoneConfiguration(configuration);
+    String scmNodes = conf.get(ConfUtils
+        .addKeySuffixes(ScmConfigKeys.OZONE_SCM_NODES_KEY,
+            getScmServiceId(conf)));
+    if (scmNodes != null) {
+      final String selfId = conf.get(ScmConfigKeys.OZONE_SCM_NODE_ID_KEY);
+      String[] parts = scmNodes.split(",");
+      List<String> partList = new ArrayList<>();

Review comment:
       partList -> partsLeft

##########
File path: 
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/InterSCMGrpcClient.java
##########
@@ -55,7 +56,8 @@
 
   public InterSCMGrpcClient(final String host, final ConfigurationSource conf) 
{
     Preconditions.checkNotNull(conf);
-    int port = conf.getObject(SCMHAConfiguration.class).getGrpcBindPort();

Review comment:
       will you remove 
   ```
     @Config(key = "grpc.bind.port",
         type = ConfigType.INT,
         defaultValue = "9899",
         tags = {OZONE, SCM, HA, RATIS},
         description = "Port used by SCM for Grpc Server."
     )
   
     private int grpcBindPort = 9866;
   ```
   from SCMHAConfiguration ?

##########
File path: 
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/XceiverServerRatis.java
##########
@@ -765,12 +765,18 @@ public void addGroup(HddsProtos.PipelineID pipelineId,
         clientId, server.getId(), nextCallId(), group);
 
     RaftClientReply reply;
+    if (LOG.isDebugEnabled()) {
+      LOG.debug("Received create pipeline request for pipeline {}", 
pipelineID);

Review comment:
       `create pipeline request` seem not accurate




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to