You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ratis.apache.org by GitBox <gi...@apache.org> on 2021/12/21 06:30:08 UTC
[GitHub] [ratis] guohao-rosicky opened a new pull request #568: RATIS-1475. Use the stream level SlidingWindow.client in OrderedStrea…
guohao-rosicky opened a new pull request #568:
URL: https://github.com/apache/ratis/pull/568
## What changes were proposed in this pull request?
Use the stream level SlidingWindow.client in OrderedStreamAsync
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/RATIS-1475
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo merged pull request #568: RATIS-1475. Use the stream level SlidingWindow.client in OrderedStrea…
Posted by GitBox <gi...@apache.org>.
szetszwo merged pull request #568:
URL: https://github.com/apache/ratis/pull/568
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] guohao-rosicky commented on a change in pull request #568: RATIS-1475. Use the stream level SlidingWindow.client in OrderedStrea…
Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on a change in pull request #568:
URL: https://github.com/apache/ratis/pull/568#discussion_r773084914
##########
File path: ratis-client/src/main/java/org/apache/ratis/client/impl/OrderedStreamAsync.java
##########
@@ -103,18 +106,27 @@ public String toString() {
}
private final DataStreamClientRpc dataStreamClientRpc;
- private final SlidingWindow.Client<DataStreamWindowRequest, DataStreamReply> slidingWindow;
+
private final Semaphore requestSemaphore;
private final TimeDuration requestTimeout;
private final TimeoutScheduler scheduler = TimeoutScheduler.getInstance();
+ private final ClientId clientId;
+
+ private final ConcurrentMap<ClientInvocationId, SlidingWindow.Client<DataStreamWindowRequest, DataStreamReply>>
+ slidingWindows = new ConcurrentHashMap<>();
Review comment:
Has been modified @szetszwo
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on a change in pull request #568: RATIS-1475. Use the stream level SlidingWindow.client in OrderedStrea…
Posted by GitBox <gi...@apache.org>.
szetszwo commented on a change in pull request #568:
URL: https://github.com/apache/ratis/pull/568#discussion_r773128391
##########
File path: ratis-client/src/main/java/org/apache/ratis/client/impl/DataStreamClientImpl.java
##########
@@ -105,12 +107,13 @@ private DataStreamOutputImpl(RaftClientRequest request) {
this.header = request;
final ByteBuffer buffer = ClientProtoUtils.toRaftClientRequestProtoByteBuffer(header);
this.headerFuture = send(Type.STREAM_HEADER, buffer, buffer.remaining());
+ this.slidingWindow = new SlidingWindow.Client<>(ClientInvocationId.valueOf(clientId, header.getCallId()));
Review comment:
Oops, we need to initialize the slidingWindow before sending the header, i.e.
```
this.header = request;
this.slidingWindow = new SlidingWindow.Client<>(ClientInvocationId.valueOf(clientId, header.getCallId()));
final ByteBuffer buffer = ClientProtoUtils.toRaftClientRequestProtoByteBuffer(header);
this.headerFuture = send(Type.STREAM_HEADER, buffer, buffer.remaining());
```
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] szetszwo commented on a change in pull request #568: RATIS-1475. Use the stream level SlidingWindow.client in OrderedStrea…
Posted by GitBox <gi...@apache.org>.
szetszwo commented on a change in pull request #568:
URL: https://github.com/apache/ratis/pull/568#discussion_r772952433
##########
File path: ratis-client/src/main/java/org/apache/ratis/client/impl/OrderedStreamAsync.java
##########
@@ -103,18 +106,27 @@ public String toString() {
}
private final DataStreamClientRpc dataStreamClientRpc;
- private final SlidingWindow.Client<DataStreamWindowRequest, DataStreamReply> slidingWindow;
+
private final Semaphore requestSemaphore;
private final TimeDuration requestTimeout;
private final TimeoutScheduler scheduler = TimeoutScheduler.getInstance();
+ private final ClientId clientId;
+
+ private final ConcurrentMap<ClientInvocationId, SlidingWindow.Client<DataStreamWindowRequest, DataStreamReply>>
+ slidingWindows = new ConcurrentHashMap<>();
Review comment:
We may move slidingWindow to DataStreamOutputImpl. Then, we don't have to create this map.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] guohao-rosicky commented on pull request #568: RATIS-1475. Use the stream level SlidingWindow.client in OrderedStrea…
Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on pull request #568:
URL: https://github.com/apache/ratis/pull/568#issuecomment-998512478
On the Stream client, it is only necessary to ensure that requests within the stream ID are ordered, which increases throughput
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] captainzmc commented on pull request #568: RATIS-1475. Use the stream level SlidingWindow.client in OrderedStrea…
Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #568:
URL: https://github.com/apache/ratis/pull/568#issuecomment-998771091
Thanks @guohao-rosicky for the contribution. This should help with performance.
In the CI test, OrderedStreamAsync has a NullPointerException. Can you fix it?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] guohao-rosicky commented on pull request #568: RATIS-1475. Use the stream level SlidingWindow.client in OrderedStrea…
Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on pull request #568:
URL: https://github.com/apache/ratis/pull/568#issuecomment-998514229
What do you think of this change @szetszwo
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [ratis] guohao-rosicky commented on a change in pull request #568: RATIS-1475. Use the stream level SlidingWindow.client in OrderedStrea…
Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on a change in pull request #568:
URL: https://github.com/apache/ratis/pull/568#discussion_r773558014
##########
File path: ratis-client/src/main/java/org/apache/ratis/client/impl/DataStreamClientImpl.java
##########
@@ -105,12 +107,13 @@ private DataStreamOutputImpl(RaftClientRequest request) {
this.header = request;
final ByteBuffer buffer = ClientProtoUtils.toRaftClientRequestProtoByteBuffer(header);
this.headerFuture = send(Type.STREAM_HEADER, buffer, buffer.remaining());
+ this.slidingWindow = new SlidingWindow.Client<>(ClientInvocationId.valueOf(clientId, header.getCallId()));
Review comment:
Has been modified @szetszwo @captainzmc
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ratis.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org