szetszwo commented on a change in pull request #364:
URL: https://github.com/apache/incubator-ratis/pull/364#discussion_r546506442
##########
File path: ratis-proto/src/main/proto/Raft.proto
##########
@@ -280,6 +284,15 @@ message WatchRequestTypeProto {
ReplicationLevel replication = 2;
}
+message RouteProto {
+ RaftPeerIdProto from = 1;
+ repeated RaftPeerIdProto to = 2;
+}
+
+message RoutingTableProto {
+ repeated RouteProto routingTable = 1;
Review comment:
Rename this to `routes`.
##########
File path: ratis-proto/src/main/proto/Raft.proto
##########
@@ -280,6 +284,15 @@ message WatchRequestTypeProto {
ReplicationLevel replication = 2;
}
+message RouteProto {
+ RaftPeerIdProto from = 1;
+ repeated RaftPeerIdProto to = 2;
Review comment:
To be consistent with RoutingTable.java, let's rename the fields to
peerId and successors.
##########
File path:
ratis-netty/src/main/java/org/apache/ratis/netty/server/DataStreamManagement.java
##########
@@ -145,6 +148,26 @@ LocalStream getLocal() {
public String toString() {
return JavaUtils.getClassSimpleName(getClass()) + ":" + request;
}
+
+ private Set<RaftPeer> getSuccessors(RaftPeerId peerId) throws IOException {
+ final RoutingTable routingTable = request.getRoutingTable();
+ final RaftGroupId groupId = request.getRaftGroupId();
+ final RaftConfiguration conf = server.getDivision(groupId).getRaftConf();
+ Set<RaftPeer> successors =
routingTable.getSuccessors(peerId).stream().map(conf::getPeer)
+ .collect(Collectors.toSet());
+ if (successors.size() > 0) {
Review comment:
If successors.size() == 0, either there is no routingTable, or there is
no successors in the route. We need to handle this two cases separately.
When there is no routing table, we should change request.getRoutingTable()
to return null.
##########
File path:
ratis-client/src/main/java/org/apache/ratis/client/impl/ClientProtoUtils.java
##########
@@ -118,14 +118,23 @@
static RaftClientRequest toRaftClientRequest(RaftClientRequestProto p) {
final RaftClientRequest.Type type = toRaftClientRequestType(p);
final RaftRpcRequestProto request = p.getRpcRequest();
+ RoutingTable.Builder routingTablebuilder = new RoutingTable.Builder();
Review comment:
Check `p.hasRoutingTable()`. Pass null if `p.hasRoutingTable() ==
false`.
##########
File path:
ratis-common/src/main/java/org/apache/ratis/protocol/RaftClientRequest.java
##########
@@ -235,17 +235,28 @@ public static RaftClientRequest
toWriteRequest(RaftClientRequest r, Message mess
private final SlidingWindowEntry slidingWindowEntry;
+ private final RoutingTable routingTable;
+
public RaftClientRequest(ClientId clientId, RaftPeerId serverId, RaftGroupId
groupId, long callId, Type type) {
- this(clientId, serverId, groupId, callId, null, type, null);
+ 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(
+ ClientId clientId, RaftPeerId serverId, RaftGroupId groupId,
+ long callId, Message message, Type type, SlidingWindowEntry
slidingWindowEntry,
+ RoutingTable routingTable) {
super(clientId, serverId, groupId, callId);
this.message = message;
this.type = type;
this.slidingWindowEntry = slidingWindowEntry != null? slidingWindowEntry:
SlidingWindowEntry.getDefaultInstance();
+ this.routingTable = routingTable != null ? routingTable : new
RoutingTable.Builder().build();
Review comment:
Just set this.routingTable to null if there is no routingTable.
##########
File path:
ratis-common/src/main/java/org/apache/ratis/protocol/RoutingTable.java
##########
@@ -0,0 +1,81 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ratis.protocol;
+
+import org.apache.ratis.proto.RaftProtos.RoutingTableProto;
+import org.apache.ratis.util.ProtoUtils;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicReference;
+
+public interface RoutingTable {
+ Set<RaftPeerId> getSuccessors(RaftPeerId peerId);
+
+ RoutingTableProto toProto();
+
+ class Builder {
Review comment:
I forgot to add a `newBuilder()` method and a private constructor.
```
private Builder() {}
```
Please add them.
----------------------------------------------------------------
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]
