szetszwo commented on a change in pull request #385:
URL: https://github.com/apache/incubator-ratis/pull/385#discussion_r550041368
##########
File path:
ratis-netty/src/main/java/org/apache/ratis/netty/server/DataStreamManagement.java
##########
@@ -170,10 +170,12 @@ public String toString() {
static class StreamMap {
static class Key {
private final ChannelId channelId;
+ private final ClientId clientId;
private final long streamId;
- Key(ChannelId channelId, long streamId) {
+ Key(ChannelId channelId, ClientId clientId, long streamId) {
Review comment:
Could we remove channelId since we have clientId? If yes, we can use
ClientInvocationId and remove the entire StreamMap.Key class.
##########
File path: ratis-proto/src/main/proto/Raft.proto
##########
@@ -327,6 +327,7 @@ message DataStreamPacketHeaderProto {
Type type = 3;
repeated Option options = 4;
uint64 dataLength = 5;
+ bytes clientId = 6;
Review comment:
Since Streaming is not yet released, how about we take this chance to
renumber the proto?
```
bytes clientId = 1;
Type type = 2;
uint64 streamId = 3;
uint64 streamOffset = 4;
uint64 dataLength = 5;
repeated Option options = 6;
```
Then, the proto and the java class will have the same order
```
public DataStreamRequestHeader(ClientId clientId, Type type, long streamId,
long streamOffset, long dataLength,
WriteOption... options) {
```
----------------------------------------------------------------
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]
