You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ratis.apache.org by sz...@apache.org on 2020/12/24 15:43:58 UTC

[incubator-ratis] branch master updated: RATIS-1263. Refactor RaftClientRequest to use Builder (#374)

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

szetszwo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-ratis.git


The following commit(s) were added to refs/heads/master by this push:
     new cb648e0  RATIS-1263. Refactor RaftClientRequest to use Builder (#374)
cb648e0 is described below

commit cb648e007d08f35f0eebfa785136efd99e5ec044
Author: runzhiwang <51...@users.noreply.github.com>
AuthorDate: Thu Dec 24 23:43:48 2020 +0800

    RATIS-1263. Refactor RaftClientRequest to use Builder (#374)
---
 .../apache/ratis/client/impl/ClientProtoUtils.java | 19 ++---
 .../ratis/client/impl/DataStreamClientImpl.java    | 12 ++-
 .../apache/ratis/client/impl/RaftClientImpl.java   | 11 ++-
 .../apache/ratis/protocol/RaftClientRequest.java   | 85 +++++++++++++++++++---
 .../apache/ratis/server/impl/MiniRaftCluster.java  | 10 ++-
 .../server/impl/TestRatisServerMetricsBase.java    | 10 ++-
 .../apache/ratis/client/TestClientProtoUtils.java  | 11 ++-
 .../org/apache/ratis/retry/TestRetryPolicy.java    |  8 +-
 8 files changed, 134 insertions(+), 32 deletions(-)

diff --git a/ratis-client/src/main/java/org/apache/ratis/client/impl/ClientProtoUtils.java b/ratis-client/src/main/java/org/apache/ratis/client/impl/ClientProtoUtils.java
index 8543523..102e294 100644
--- a/ratis-client/src/main/java/org/apache/ratis/client/impl/ClientProtoUtils.java
+++ b/ratis-client/src/main/java/org/apache/ratis/client/impl/ClientProtoUtils.java
@@ -139,15 +139,16 @@ public interface ClientProtoUtils {
     final RaftClientRequest.Type type = toRaftClientRequestType(p);
     final RaftRpcRequestProto request = p.getRpcRequest();
 
-    return new RaftClientRequest(
-        ClientId.valueOf(request.getRequestorId()),
-        RaftPeerId.valueOf(request.getReplyId()),
-        ProtoUtils.toRaftGroupId(request.getRaftGroupId()),
-        request.getCallId(),
-        toMessage(p.getMessage()),
-        type,
-        request.getSlidingWindowEntry(),
-        getRoutingTable(p));
+    return RaftClientRequest.newBuilder()
+        .setClientId(ClientId.valueOf(request.getRequestorId()))
+        .setServerId(RaftPeerId.valueOf(request.getReplyId()))
+        .setGroupId(ProtoUtils.toRaftGroupId(request.getRaftGroupId()))
+        .setCallId(request.getCallId())
+        .setMessage(toMessage(p.getMessage()))
+        .setType(type)
+        .setSlidingWindowEntry(request.getSlidingWindowEntry())
+        .setRoutingTable(getRoutingTable(p))
+        .build();
   }
 
   static ByteBuffer toRaftClientRequestProtoByteBuffer(RaftClientRequest request) {
diff --git a/ratis-client/src/main/java/org/apache/ratis/client/impl/DataStreamClientImpl.java b/ratis-client/src/main/java/org/apache/ratis/client/impl/DataStreamClientImpl.java
index 35dd56c..98aa575 100644
--- a/ratis-client/src/main/java/org/apache/ratis/client/impl/DataStreamClientImpl.java
+++ b/ratis-client/src/main/java/org/apache/ratis/client/impl/DataStreamClientImpl.java
@@ -190,9 +190,15 @@ public class DataStreamClientImpl implements DataStreamClient {
   public DataStreamOutputRpc stream(ByteBuffer headerMessage, RoutingTable routingTable) {
     final Message message =
         Optional.ofNullable(headerMessage).map(ByteString::copyFrom).map(Message::valueOf).orElse(null);
-    RaftClientRequest request = new RaftClientRequest(clientId, dataStreamServer.getId(), groupId,
-        CallId.getAndIncrement(), message, RaftClientRequest.dataStreamRequestType(), null,
-        routingTable);
+    RaftClientRequest request = RaftClientRequest.newBuilder()
+        .setClientId(clientId)
+        .setServerId(dataStreamServer.getId())
+        .setGroupId(groupId)
+        .setCallId(CallId.getAndIncrement())
+        .setMessage(message)
+        .setType(RaftClientRequest.dataStreamRequestType())
+        .setRoutingTable(routingTable)
+        .build();
     return new DataStreamOutputImpl(request);
   }
 
diff --git a/ratis-client/src/main/java/org/apache/ratis/client/impl/RaftClientImpl.java b/ratis-client/src/main/java/org/apache/ratis/client/impl/RaftClientImpl.java
index d5437c7..b318a28 100644
--- a/ratis-client/src/main/java/org/apache/ratis/client/impl/RaftClientImpl.java
+++ b/ratis-client/src/main/java/org/apache/ratis/client/impl/RaftClientImpl.java
@@ -209,8 +209,15 @@ public final class RaftClientImpl implements RaftClient {
   RaftClientRequest newRaftClientRequest(
       RaftPeerId server, long callId, Message message, RaftClientRequest.Type type,
       SlidingWindowEntry slidingWindowEntry) {
-    return new RaftClientRequest(clientId, server != null? server: leaderId, groupId,
-        callId, message, type, slidingWindowEntry);
+    return RaftClientRequest.newBuilder()
+        .setClientId(clientId)
+        .setServerId(server != null? server: leaderId)
+        .setGroupId(groupId)
+        .setCallId(callId)
+        .setMessage(message)
+        .setType(type)
+        .setSlidingWindowEntry(slidingWindowEntry)
+        .build();
   }
 
   @Override
diff --git a/ratis-common/src/main/java/org/apache/ratis/protocol/RaftClientRequest.java b/ratis-common/src/main/java/org/apache/ratis/protocol/RaftClientRequest.java
index 1b745c9..4aca94f 100644
--- a/ratis-common/src/main/java/org/apache/ratis/protocol/RaftClientRequest.java
+++ b/ratis-common/src/main/java/org/apache/ratis/protocol/RaftClientRequest.java
@@ -224,10 +224,81 @@ public class RaftClientRequest extends RaftClientMessage {
     }
   }
 
+  /**
+   * To build {@link RaftClientRequest}
+   */
+  public static class Builder {
+    private ClientId clientId;
+    private RaftPeerId serverId;
+    private RaftGroupId groupId;
+    private long callId;
+
+    private Message message;
+    private Type type;
+    private SlidingWindowEntry slidingWindowEntry;
+    private RoutingTable routingTable;
+
+    public RaftClientRequest build() {
+      return new RaftClientRequest(
+          clientId, serverId, groupId, callId, message, type, slidingWindowEntry, routingTable);
+    }
+
+    public Builder setClientId(ClientId clientId) {
+      this.clientId = clientId;
+      return this;
+    }
+
+    public Builder setServerId(RaftPeerId serverId) {
+      this.serverId = serverId;
+      return this;
+    }
+
+    public Builder setGroupId(RaftGroupId groupId) {
+      this.groupId = groupId;
+      return this;
+    }
+
+    public Builder setCallId(long callId) {
+      this.callId = callId;
+      return this;
+    }
+
+    public Builder setMessage(Message message) {
+      this.message = message;
+      return this;
+    }
+
+    public Builder setType(Type type) {
+      this.type = type;
+      return this;
+    }
+
+    public Builder setSlidingWindowEntry(SlidingWindowEntry slidingWindowEntry) {
+      this.slidingWindowEntry = slidingWindowEntry;
+      return this;
+    }
+
+    public Builder setRoutingTable(RoutingTable routingTable) {
+      this.routingTable = routingTable;
+      return this;
+    }
+  }
+
+  public static Builder newBuilder() {
+    return new Builder();
+  }
+
   /** Convert the given request to a write request with the given message. */
   public static RaftClientRequest toWriteRequest(RaftClientRequest r, Message message) {
-    return new RaftClientRequest(r.getClientId(), r.getServerId(), r.getRaftGroupId(),
-        r.getCallId(), message, RaftClientRequest.writeRequestType(), r.getSlidingWindowEntry());
+    return RaftClientRequest.newBuilder()
+        .setClientId(r.getClientId())
+        .setServerId(r.getServerId())
+        .setGroupId(r.getRaftGroupId())
+        .setCallId(r.getCallId())
+        .setMessage(message)
+        .setType(RaftClientRequest.writeRequestType())
+        .setSlidingWindowEntry(r.getSlidingWindowEntry())
+        .build();
   }
 
   private final Message message;
@@ -237,18 +308,12 @@ public class RaftClientRequest extends RaftClientMessage {
 
   private final RoutingTable routingTable;
 
-  public RaftClientRequest(ClientId clientId, RaftPeerId serverId, RaftGroupId groupId, long callId, Type type) {
+  protected RaftClientRequest(ClientId clientId, RaftPeerId serverId, RaftGroupId groupId, long callId, Type type) {
     this(clientId, serverId, groupId, callId, null, type, null, null);
   }
 
-  public RaftClientRequest(
-      ClientId clientId, RaftPeerId serverId, RaftGroupId groupId,
-      long callId, Message message, Type type, SlidingWindowEntry slidingWindowEntry) {
-    this(clientId, serverId, groupId, callId, message, type, slidingWindowEntry, null);
-  }
-
   @SuppressWarnings("parameternumber")
-  public RaftClientRequest(
+  private RaftClientRequest(
       ClientId clientId, RaftPeerId serverId, RaftGroupId groupId,
       long callId, Message message, Type type, SlidingWindowEntry slidingWindowEntry,
       RoutingTable routingTable) {
diff --git a/ratis-server/src/test/java/org/apache/ratis/server/impl/MiniRaftCluster.java b/ratis-server/src/test/java/org/apache/ratis/server/impl/MiniRaftCluster.java
index cba7758..9e5ed65 100644
--- a/ratis-server/src/test/java/org/apache/ratis/server/impl/MiniRaftCluster.java
+++ b/ratis-server/src/test/java/org/apache/ratis/server/impl/MiniRaftCluster.java
@@ -703,8 +703,14 @@ public abstract class MiniRaftCluster implements Closeable {
 
   public RaftClientRequest newRaftClientRequest(
       ClientId clientId, RaftPeerId leaderId, long callId, Message message) {
-    return new RaftClientRequest(clientId, leaderId, getGroupId(),
-        callId, message, RaftClientRequest.writeRequestType(), null);
+    return RaftClientRequest.newBuilder()
+        .setClientId(clientId)
+        .setServerId(leaderId)
+        .setGroupId(getGroupId())
+        .setCallId(callId)
+        .setMessage(message)
+        .setType(RaftClientRequest.writeRequestType())
+        .build();
   }
 
   public SetConfigurationRequest newSetConfigurationRequest(
diff --git a/ratis-server/src/test/java/org/apache/ratis/server/impl/TestRatisServerMetricsBase.java b/ratis-server/src/test/java/org/apache/ratis/server/impl/TestRatisServerMetricsBase.java
index 21ea752..838db86 100644
--- a/ratis-server/src/test/java/org/apache/ratis/server/impl/TestRatisServerMetricsBase.java
+++ b/ratis-server/src/test/java/org/apache/ratis/server/impl/TestRatisServerMetricsBase.java
@@ -58,8 +58,14 @@ public abstract class TestRatisServerMetricsBase<CLUSTER extends MiniRaftCluster
     final RaftServer.Division leaderImpl = RaftTestUtil.waitForLeader(cluster);
     ClientId clientId = ClientId.randomId();
     // StaleRead with Long.MAX_VALUE minIndex will fail.
-    RaftClientRequest r = new RaftClientRequest(clientId, leaderImpl.getId(), cluster.getGroupId(),
-        0, Message.EMPTY, RaftClientRequest.staleReadRequestType(Long.MAX_VALUE), null);
+    RaftClientRequest r = RaftClientRequest.newBuilder()
+        .setClientId(clientId)
+        .setServerId(leaderImpl.getId())
+        .setGroupId(cluster.getGroupId())
+        .setCallId(0)
+        .setMessage(Message.EMPTY)
+        .setType(RaftClientRequest.staleReadRequestType(Long.MAX_VALUE))
+        .build();
     final CompletableFuture<RaftClientReply> f = leaderImpl.getRaftServer().submitClientRequestAsync(r);
     Assert.assertTrue(!f.get().isSuccess());
     assertEquals(1L, ((RaftServerMetricsImpl)leaderImpl.getRaftServerMetrics())
diff --git a/ratis-test/src/test/java/org/apache/ratis/client/TestClientProtoUtils.java b/ratis-test/src/test/java/org/apache/ratis/client/TestClientProtoUtils.java
index 3c13413..9d85320 100644
--- a/ratis-test/src/test/java/org/apache/ratis/client/TestClientProtoUtils.java
+++ b/ratis-test/src/test/java/org/apache/ratis/client/TestClientProtoUtils.java
@@ -57,9 +57,14 @@ public class TestClientProtoUtils extends BaseTest {
 
     for(int i = 0; i < n; i++) {
       final ByteString bytes = newByteString(messageSize.getSizeInt(), i);
-      final RaftClientRequest request = new RaftClientRequest(clientId, leaderId, groupId,
-          1, () -> bytes, RaftClientRequest.writeRequestType(), null);
-
+      final RaftClientRequest request = RaftClientRequest.newBuilder()
+          .setClientId(clientId)
+          .setServerId(leaderId)
+          .setGroupId(groupId)
+          .setCallId(1)
+          .setMessage(() -> bytes)
+          .setType(RaftClientRequest.writeRequestType())
+          .build();
       final Timestamp startTime = Timestamp.currentTime();
       final RaftClientRequestProto proto = ClientProtoUtils.toRaftClientRequestProto(request);
       final TimeDuration p = startTime.elapsedTime();
diff --git a/ratis-test/src/test/java/org/apache/ratis/retry/TestRetryPolicy.java b/ratis-test/src/test/java/org/apache/ratis/retry/TestRetryPolicy.java
index d1dcc82..3bd4b6b 100644
--- a/ratis-test/src/test/java/org/apache/ratis/retry/TestRetryPolicy.java
+++ b/ratis-test/src/test/java/org/apache/ratis/retry/TestRetryPolicy.java
@@ -277,7 +277,13 @@ public class TestRetryPolicy extends BaseTest {
 
 
   private static RaftClientRequest newRaftClientRequest(RaftClientRequest.Type type) {
-    return new RaftClientRequest(ClientId.randomId(), RaftPeerId.valueOf("s0"), RaftGroupId.randomId(), 1L, type);
+    return RaftClientRequest.newBuilder()
+        .setClientId(ClientId.randomId())
+        .setServerId(RaftPeerId.valueOf("s0"))
+        .setGroupId(RaftGroupId.randomId())
+        .setCallId(1L)
+        .setType(type)
+        .build();
   }
 
   /**