You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ozone.apache.org by sa...@apache.org on 2020/12/17 07:15:30 UTC

[ozone] branch HDDS-2823 updated: HDDS-4575: Refactor SCMHAManager and SCMRatisServer with RaftServer.Division (#1683)

This is an automated email from the ASF dual-hosted git repository.

sammichen pushed a commit to branch HDDS-2823
in repository https://gitbox.apache.org/repos/asf/ozone.git


The following commit(s) were added to refs/heads/HDDS-2823 by this push:
     new 7f5e522  HDDS-4575: Refactor SCMHAManager and SCMRatisServer with RaftServer.Division (#1683)
7f5e522 is described below

commit 7f5e52291272fc390d02a66f7d2566828f013c54
Author: GlenGeng <gl...@tencent.com>
AuthorDate: Thu Dec 17 15:15:17 2020 +0800

    HDDS-4575: Refactor SCMHAManager and SCMRatisServer with RaftServer.Division (#1683)
---
 .../hadoop/hdds/scm/ha/MockSCMHAManager.java       | 42 ++++---------
 .../apache/hadoop/hdds/scm/ha/SCMHAManager.java    | 19 ------
 .../hadoop/hdds/scm/ha/SCMHAManagerImpl.java       | 68 +---------------------
 .../apache/hadoop/hdds/scm/ha/SCMRatisServer.java  | 15 +++--
 .../hadoop/hdds/scm/ha/SCMRatisServerImpl.java     | 68 ++++++++++++----------
 .../hdds/scm/pipeline/PipelineManagerV2Impl.java   |  4 +-
 .../hdds/scm/server/SCMClientProtocolServer.java   |  3 +-
 .../hdds/scm/server/StorageContainerManager.java   | 11 ----
 .../dist/src/main/smoketest/admincli/scmha.robot   |  2 +-
 9 files changed, 66 insertions(+), 166 deletions(-)

diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/MockSCMHAManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/MockSCMHAManager.java
index dd9c5a8..08fd2b2 100644
--- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/MockSCMHAManager.java
+++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/MockSCMHAManager.java
@@ -33,7 +33,6 @@ import org.apache.ratis.protocol.Message;
 import org.apache.ratis.protocol.RaftClientReply;
 import org.apache.ratis.protocol.RaftGroupId;
 import org.apache.ratis.protocol.RaftGroupMemberId;
-import org.apache.ratis.protocol.RaftPeer;
 import org.apache.ratis.protocol.RaftPeerId;
 import org.apache.ratis.protocol.exceptions.NotLeaderException;
 import org.apache.ratis.server.RaftServer;
@@ -77,11 +76,6 @@ public final class MockSCMHAManager implements SCMHAManager {
     this.isLeader = isLeader;
   }
 
-  @Override
-  public RaftPeer getSuggestedLeader() {
-    throw new UnsupportedOperationException();
-  }
-
   /**
    * {@inheritDoc}
    */
@@ -98,24 +92,6 @@ public final class MockSCMHAManager implements SCMHAManager {
     ratisServer.stop();
   }
 
-  @Override
-  public List<String> getRatisRoles() {
-    return Arrays.asList(
-        "180.3.14.5:9865",
-        "180.3.14.21:9865",
-        "180.3.14.145:9865");
-  }
-
-  /**
-   * {@inheritDoc}
-   */
-  @Override
-  public NotLeaderException triggerNotLeaderException() {
-    return new NotLeaderException(RaftGroupMemberId.valueOf(
-        RaftPeerId.valueOf("peer"), RaftGroupId.randomId()),
-        null, new ArrayList<>());
-  }
-
   private class MockRatisServer implements SCMRatisServer {
 
     private Map<RequestType, Object> handlers =
@@ -205,23 +181,27 @@ public final class MockSCMHAManager implements SCMHAManager {
     }
 
     @Override
-    public RaftServer getServer() {
-      return null;
+    public void stop() {
     }
 
     @Override
-    public RaftGroupId getRaftGroupId() {
+    public RaftServer.Division getDivision() {
       return null;
     }
 
     @Override
-    public List<RaftPeer> getRaftPeers() {
-      return new ArrayList<>();
+    public List<String> getRatisRoles() {
+      return Arrays.asList(
+          "180.3.14.5:9865",
+          "180.3.14.21:9865",
+          "180.3.14.145:9865");
     }
 
     @Override
-    public void stop() {
+    public NotLeaderException triggerNotLeaderException() {
+      return new NotLeaderException(RaftGroupMemberId.valueOf(
+          RaftPeerId.valueOf("peer"), RaftGroupId.randomId()),
+          null, new ArrayList<>());
     }
   }
-
 }
\ No newline at end of file
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAManager.java
index 0fd5e82..59410b1 100644
--- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAManager.java
+++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAManager.java
@@ -17,10 +17,6 @@
 
 package org.apache.hadoop.hdds.scm.ha;
 
-import java.util.List;
-import org.apache.ratis.protocol.RaftPeer;
-import org.apache.ratis.protocol.exceptions.NotLeaderException;
-
 import java.io.IOException;
 import java.util.Optional;
 
@@ -49,22 +45,7 @@ public interface SCMHAManager {
   SCMRatisServer getRatisServer();
 
   /**
-   * Returns suggested leader from RaftServer.
-   */
-  RaftPeer getSuggestedLeader();
-
-  /**
    * Stops the HA service.
    */
   void shutdown() throws IOException;
-
-  /**
-   * Returns roles of ratis peers.
-   */
-  List<String> getRatisRoles();
-
-  /**
-   * Returns NotLeaderException with useful info.
-   */
-  NotLeaderException triggerNotLeaderException();
 }
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAManagerImpl.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAManagerImpl.java
index 966db43..ae91fc2 100644
--- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAManagerImpl.java
+++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMHAManagerImpl.java
@@ -18,14 +18,8 @@
 package org.apache.hadoop.hdds.scm.ha;
 
 import com.google.common.base.Preconditions;
-import java.util.List;
-import java.util.stream.Collectors;
 import org.apache.hadoop.hdds.conf.ConfigurationSource;
 import org.apache.ratis.proto.RaftProtos;
-import org.apache.ratis.protocol.RaftGroupMemberId;
-import org.apache.ratis.protocol.RaftPeer;
-import org.apache.ratis.protocol.RaftPeerId;
-import org.apache.ratis.protocol.exceptions.NotLeaderException;
 import org.apache.ratis.server.RaftServer;
 import org.apache.ratis.server.impl.RaftServerImpl;
 import org.apache.ratis.server.impl.RaftServerProxy;
@@ -77,13 +71,14 @@ public class SCMHAManagerImpl implements SCMHAManager {
       // When SCM HA is not enabled, the current SCM is always the leader.
       return Optional.of((long)0);
     }
-    RaftServer server = ratisServer.getServer();
+    RaftServer server = ratisServer.getDivision().getRaftServer();
     Preconditions.checkState(server instanceof RaftServerProxy);
     try {
       // SCM only has one raft group.
       RaftServerImpl serverImpl = ((RaftServerProxy) server)
-          .getImpl(ratisServer.getRaftGroupId());
+          .getImpl(ratisServer.getDivision().getGroup().getGroupId());
       if (serverImpl != null) {
+        // TODO: getRoleInfoProto() will be exposed from Division later.
         RaftProtos.RoleInfoProto roleInfoProto = serverImpl.getRoleInfoProto();
         return roleInfoProto.hasLeaderInfo()
             ? Optional.of(roleInfoProto.getLeaderInfo().getTerm())
@@ -104,42 +99,6 @@ public class SCMHAManagerImpl implements SCMHAManager {
     return ratisServer;
   }
 
-  private RaftPeerId getPeerIdFromRoleInfo(RaftServerImpl serverImpl) {
-    if (serverImpl.isLeader()) {
-      return RaftPeerId.getRaftPeerId(
-          serverImpl.getRoleInfoProto().getLeaderInfo().toString());
-    } else if (serverImpl.isFollower()) {
-      return RaftPeerId.valueOf(
-          serverImpl.getRoleInfoProto().getFollowerInfo()
-              .getLeaderInfo().getId().getId());
-    } else {
-      return null;
-    }
-  }
-
-  @Override
-  public RaftPeer getSuggestedLeader() {
-    RaftServer server = ratisServer.getServer();
-    Preconditions.checkState(server instanceof RaftServerProxy);
-    RaftServerImpl serverImpl = null;
-    try {
-      // SCM only has one raft group.
-      serverImpl = ((RaftServerProxy) server)
-          .getImpl(ratisServer.getRaftGroupId());
-      if (serverImpl != null) {
-        RaftPeerId peerId =  getPeerIdFromRoleInfo(serverImpl);
-        if (peerId != null) {
-          return RaftPeer.newBuilder().setId(peerId).build();
-        }
-        return null;
-      }
-    } catch (IOException ioe) {
-      LOG.error("Fail to get RaftServer impl and therefore it's not clear " +
-          "whether it's leader. ", ioe);
-    }
-    return null;
-  }
-
   /**
    * {@inheritDoc}
    */
@@ -147,25 +106,4 @@ public class SCMHAManagerImpl implements SCMHAManager {
   public void shutdown() throws IOException {
     ratisServer.stop();
   }
-
-  @Override
-  public List<String> getRatisRoles() {
-    return getRatisServer()
-            .getRaftPeers()
-            .stream()
-            .map(peer -> peer.getAddress() == null ? "" : peer.getAddress())
-            .collect(Collectors.toList());
-  }
-
-  /**
-   * {@inheritDoc}
-   */
-  @Override
-  public NotLeaderException triggerNotLeaderException() {
-    return new NotLeaderException(RaftGroupMemberId.valueOf(
-        ratisServer.getServer().getId(),
-        ratisServer.getRaftGroupId()),
-        getSuggestedLeader(),
-        ratisServer.getRaftPeers());
-  }
 }
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMRatisServer.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMRatisServer.java
index 2f99776..d8a78be 100644
--- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMRatisServer.java
+++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMRatisServer.java
@@ -18,8 +18,7 @@
 package org.apache.hadoop.hdds.scm.ha;
 
 import org.apache.hadoop.hdds.protocol.proto.SCMRatisProtocol.RequestType;
-import org.apache.ratis.protocol.RaftGroupId;
-import org.apache.ratis.protocol.RaftPeer;
+import org.apache.ratis.protocol.exceptions.NotLeaderException;
 import org.apache.ratis.server.RaftServer;
 
 import java.io.IOException;
@@ -40,9 +39,15 @@ public interface SCMRatisServer {
 
   void stop() throws IOException;
 
-  RaftServer getServer();
+  RaftServer.Division getDivision();
 
-  RaftGroupId getRaftGroupId();
+  /**
+   * Returns roles of ratis peers.
+   */
+  List<String> getRatisRoles();
 
-  List<RaftPeer> getRaftPeers();
+  /**
+   * Returns NotLeaderException with useful info.
+   */
+  NotLeaderException triggerNotLeaderException();
 }
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMRatisServerImpl.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMRatisServerImpl.java
index ab766c9..3a81d2b 100644
--- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMRatisServerImpl.java
+++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/ha/SCMRatisServerImpl.java
@@ -23,7 +23,6 @@ import java.net.InetSocketAddress;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.List;
 import java.util.UUID;
 import java.util.concurrent.ExecutionException;
@@ -42,6 +41,7 @@ import org.apache.ratis.protocol.RaftGroup;
 import org.apache.ratis.protocol.RaftGroupId;
 import org.apache.ratis.protocol.RaftPeer;
 import org.apache.ratis.protocol.RaftPeerId;
+import org.apache.ratis.protocol.exceptions.NotLeaderException;
 import org.apache.ratis.server.RaftServer;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -53,12 +53,8 @@ public class SCMRatisServerImpl implements SCMRatisServer {
   private static final Logger LOG =
       LoggerFactory.getLogger(SCMRatisServerImpl.class);
 
+  private final RaftServer.Division division;
   private final InetSocketAddress address;
-  private final RaftServer server;
-  private final RaftGroupId raftGroupId;
-  private final RaftGroup raftGroup;
-  private final RaftPeerId raftPeerId;
-  private final SCMStateMachine scmStateMachine;
   private final ClientId clientId = ClientId.randomId();
   private final AtomicLong callId = new AtomicLong();
 
@@ -69,41 +65,49 @@ public class SCMRatisServerImpl implements SCMRatisServer {
       throws IOException {
     this.address = haConf.getRatisBindAddress();
 
-    SCMHAGroupBuilder scmHAGroupBuilder = new SCMHAGroupBuilder(haConf, conf);
-    this.raftPeerId = scmHAGroupBuilder.getPeerId();
-    this.raftGroupId = scmHAGroupBuilder.getRaftGroupId();
-    this.raftGroup = scmHAGroupBuilder.getRaftGroup();
+    SCMHAGroupBuilder haGrpBuilder = new SCMHAGroupBuilder(haConf, conf);
 
     final RaftProperties serverProperties = RatisUtil
         .newRaftProperties(haConf, conf);
-    this.scmStateMachine = new SCMStateMachine();
-    this.server = RaftServer.newBuilder()
-        .setServerId(raftPeerId)
-        .setGroup(raftGroup)
+
+    RaftServer server = RaftServer.newBuilder()
+        .setServerId(haGrpBuilder.getPeerId())
+        .setGroup(haGrpBuilder.getRaftGroup())
         .setProperties(serverProperties)
-        .setStateMachine(scmStateMachine)
+        .setStateMachine(new SCMStateMachine())
         .build();
+
+    this.division = server.getDivision(haGrpBuilder.getRaftGroupId());
   }
 
   @Override
   public void start() throws IOException {
-    server.start();
+    division.getRaftServer().start();
   }
 
   @Override
   public void registerStateMachineHandler(final RequestType handlerType,
                                           final Object handler) {
-    scmStateMachine.registerHandler(handlerType, handler);
+    ((SCMStateMachine) division.getStateMachine())
+        .registerHandler(handlerType, handler);
   }
 
   @Override
   public SCMRatisResponse submitRequest(SCMRatisRequest request)
       throws IOException, ExecutionException, InterruptedException {
-    final RaftClientRequest raftClientRequest = new RaftClientRequest(
-        clientId, server.getId(), raftGroupId, nextCallId(), request.encode(),
-        RaftClientRequest.writeRequestType(), null);
+    final RaftClientRequest raftClientRequest =
+        new RaftClientRequest(
+            clientId,
+            division.getId(),
+            division.getGroup().getGroupId(),
+            nextCallId(),
+            request.encode(),
+            RaftClientRequest.writeRequestType(),
+            null);
     final RaftClientReply raftClientReply =
-        server.submitClientRequestAsync(raftClientRequest).get();
+        division.getRaftServer()
+            .submitClientRequestAsync(raftClientRequest)
+            .get();
     return SCMRatisResponse.decode(raftClientReply);
   }
 
@@ -113,26 +117,30 @@ public class SCMRatisServerImpl implements SCMRatisServer {
 
   @Override
   public void stop() throws IOException {
-    server.close();
+    division.getRaftServer().close();
   }
 
   @Override
-  public RaftServer getServer() {
-    return server;
+  public RaftServer.Division getDivision() {
+    return division;
   }
 
   @Override
-  public RaftGroupId getRaftGroupId() {
-    return raftGroupId;
+  public List<String> getRatisRoles() {
+    return division.getGroup().getPeers().stream()
+        .map(peer -> peer.getAddress() == null ? "" : peer.getAddress())
+        .collect(Collectors.toList());
   }
 
+  /**
+   * {@inheritDoc}
+   */
   @Override
-  public List<RaftPeer> getRaftPeers() {
-    return Collections.singletonList(RaftPeer.newBuilder()
-        .setId(raftPeerId).build());
+  public NotLeaderException triggerNotLeaderException() {
+    return new NotLeaderException(
+        division.getMemberId(), null, division.getGroup().getPeers());
   }
 
-
   /**
    * If the SCM group starts from {@link ScmConfigKeys#OZONE_SCM_NAMES},
    * its raft peers should locate on different nodes, and use the same port
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineManagerV2Impl.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineManagerV2Impl.java
index 3f2f6e2..8b7d849 100644
--- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineManagerV2Impl.java
+++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/pipeline/PipelineManagerV2Impl.java
@@ -589,9 +589,7 @@ public final class PipelineManagerV2Impl implements PipelineManager {
         startPipelineCreator();
       }
     } catch (NotLeaderException ex) {
-      LOG.warn("Not the current leader SCM and cannot process pipeline" +
-              " creation. Suggested leader is: ",
-          scmhaManager.getSuggestedLeader().getAddress());
+      LOG.warn("Not leader SCM, cannot process pipeline creation.");
     }
 
   }
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java
index a6b7574..d303705 100644
--- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java
+++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java
@@ -533,7 +533,8 @@ public class SCMClientProtocolServer implements
           new ScmInfo.Builder()
               .setClusterId(scm.getScmStorageConfig().getClusterID())
               .setScmId(scm.getScmStorageConfig().getScmId())
-              .setRatisPeerRoles(scm.getScmHAManager().getRatisRoles());
+              .setRatisPeerRoles(
+                  scm.getScmHAManager().getRatisServer().getRatisRoles());
       return builder.build();
     } catch (Exception ex) {
       auditSuccess = false;
diff --git a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
index 7e4e3b6..22c9649 100644
--- a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
+++ b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java
@@ -1055,17 +1055,6 @@ public final class StorageContainerManager extends ServiceRuntimeInfoImpl
     return scmHAManager.isLeader().isPresent();
   }
 
-  /**
-   * Get suggested leader from Raft.
-   * @return - suggested leader address.
-   */
-  public String getSuggestedLeader() {
-    if (scmHAManager.getSuggestedLeader() == null) {
-      return null;
-    }
-    return scmHAManager.getSuggestedLeader().getAddress();
-  }
-
   public void checkAdminAccess(String remoteUser) throws IOException {
     if (remoteUser != null && !scmAdminUsernames.contains(remoteUser)) {
       throw new IOException(
diff --git a/hadoop-ozone/dist/src/main/smoketest/admincli/scmha.robot b/hadoop-ozone/dist/src/main/smoketest/admincli/scmha.robot
index 31a990f..4d7c232 100644
--- a/hadoop-ozone/dist/src/main/smoketest/admincli/scmha.robot
+++ b/hadoop-ozone/dist/src/main/smoketest/admincli/scmha.robot
@@ -25,4 +25,4 @@ Test Timeout        5 minutes
 *** Test Cases ***
 Run scm roles
     ${output} =         Execute          ozone admin scm roles
-                        Should contain   ${output}  []
+                        Should contain   ${output}  [scm:9865]


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@ozone.apache.org
For additional commands, e-mail: commits-help@ozone.apache.org