peterxcli commented on code in PR #1481:
URL: https://github.com/apache/ratis/pull/1481#discussion_r3441011784
##########
ratis-client/src/main/java/org/apache/ratis/client/impl/DataStreamClientImpl.java:
##########
@@ -236,6 +244,114 @@ private CompletableFuture<DataStreamReply>
sendForward(DataStreamReply writeRepl
}
}
+ public final class DataStreamInputImpl implements DataStreamInput {
+ private final RaftClientRequest header;
+ private final CompletableFuture<DataStreamReply> replyFuture;
+ private final Queue<DataStreamReply> replies = new ArrayDeque<>();
+ private final Queue<CompletableFuture<DataStreamReply>> pendingReads = new
ArrayDeque<>();
+ private Throwable readException;
+ private boolean endOfStream;
+ private boolean closed;
+
+ private DataStreamInputImpl(RaftClientRequest request) {
+ this.header = request;
+ final ByteBuffer buffer =
ClientProtoUtils.toRaftClientRequestProtoByteBuffer(header);
+ final DataStreamRequestHeader h = new
DataStreamRequestHeader(header.getClientId(), Type.STREAM_HEADER,
+ header.getCallId(), 0, buffer.remaining(),
StandardWriteOption.FLUSH, StandardWriteOption.CLOSE);
+ this.replyFuture = dataStreamClientRpc.streamAsync(new
DataStreamRequestByteBuffer(h, buffer),
+ reply -> receive(reply.copy()));
+ replyFuture.whenComplete((reply, exception) -> {
+ if (exception != null) {
+ failReads(exception);
+ } else {
+ markEndOfStream();
+ }
Review Comment:
I think I can refactor this to get better readability.
##########
ratis-client/src/main/java/org/apache/ratis/client/impl/DataStreamClientImpl.java:
##########
@@ -236,6 +244,114 @@ private CompletableFuture<DataStreamReply>
sendForward(DataStreamReply writeRepl
}
}
+ public final class DataStreamInputImpl implements DataStreamInput {
+ private final RaftClientRequest header;
+ private final CompletableFuture<DataStreamReply> replyFuture;
+ private final Queue<DataStreamReply> replies = new ArrayDeque<>();
+ private final Queue<CompletableFuture<DataStreamReply>> pendingReads = new
ArrayDeque<>();
+ private Throwable readException;
+ private boolean endOfStream;
+ private boolean closed;
+
+ private DataStreamInputImpl(RaftClientRequest request) {
+ this.header = request;
+ final ByteBuffer buffer =
ClientProtoUtils.toRaftClientRequestProtoByteBuffer(header);
+ final DataStreamRequestHeader h = new
DataStreamRequestHeader(header.getClientId(), Type.STREAM_HEADER,
+ header.getCallId(), 0, buffer.remaining(),
StandardWriteOption.FLUSH, StandardWriteOption.CLOSE);
+ this.replyFuture = dataStreamClientRpc.streamAsync(new
DataStreamRequestByteBuffer(h, buffer),
+ reply -> receive(reply.copy()));
+ replyFuture.whenComplete((reply, exception) -> {
+ if (exception != null) {
+ failReads(exception);
+ } else {
+ markEndOfStream();
+ }
Review Comment:
no, the caller might have not counsumed the fetched content yet, if we set
close, the `readAsync` would fail,
##########
ratis-client/src/main/java/org/apache/ratis/client/impl/DataStreamClientImpl.java:
##########
@@ -236,6 +244,114 @@ private CompletableFuture<DataStreamReply>
sendForward(DataStreamReply writeRepl
}
}
+ public final class DataStreamInputImpl implements DataStreamInput {
+ private final RaftClientRequest header;
+ private final CompletableFuture<DataStreamReply> replyFuture;
+ private final Queue<DataStreamReply> replies = new ArrayDeque<>();
+ private final Queue<CompletableFuture<DataStreamReply>> pendingReads = new
ArrayDeque<>();
+ private Throwable readException;
+ private boolean endOfStream;
+ private boolean closed;
+
+ private DataStreamInputImpl(RaftClientRequest request) {
+ this.header = request;
+ final ByteBuffer buffer =
ClientProtoUtils.toRaftClientRequestProtoByteBuffer(header);
+ final DataStreamRequestHeader h = new
DataStreamRequestHeader(header.getClientId(), Type.STREAM_HEADER,
+ header.getCallId(), 0, buffer.remaining(),
StandardWriteOption.FLUSH, StandardWriteOption.CLOSE);
+ this.replyFuture = dataStreamClientRpc.streamAsync(new
DataStreamRequestByteBuffer(h, buffer),
+ reply -> receive(reply.copy()));
+ replyFuture.whenComplete((reply, exception) -> {
+ if (exception != null) {
+ failReads(exception);
+ } else {
+ markEndOfStream();
+ }
+ });
+ }
+
+ private void receive(DataStreamReply reply) {
+ for (;;) {
+ final CompletableFuture<DataStreamReply> pending;
+ synchronized (this) {
+ if (closed) {
+ DataStreamReplyByteBuf.release(reply);
+ return;
+ }
+ pending = pendingReads.poll();
+ if (pending == null) {
+ replies.add(reply);
+ return;
+ }
+ }
+ if (pending.complete(reply)) {
+ return;
+ }
+ }
+ }
+
+ private void failReads(Throwable t) {
+ for (;;) {
+ final CompletableFuture<DataStreamReply> pending;
+ synchronized (this) {
+ readException = t;
+ pending = pendingReads.poll();
+ if (pending == null) {
+ return;
+ }
+ }
+ pending.completeExceptionally(t);
+ }
+ }
+
+ private EOFException newEndOfStreamException() {
+ return new EOFException(clientId + ": end of stream, request=" + header);
+ }
+
+ private void markEndOfStream() {
+ for (;;) {
+ final CompletableFuture<DataStreamReply> pending;
+ synchronized (this) {
+ endOfStream = true;
+ pending = pendingReads.poll();
+ if (pending == null) {
+ return;
+ }
+ }
+ pending.completeExceptionally(newEndOfStreamException());
+ }
+ }
+
+ @Override
+ public synchronized CompletableFuture<DataStreamReply> readAsync() {
+ if (closed) {
+ return JavaUtils.completeExceptionally(new AlreadyClosedException(
+ clientId + ": stream already closed, request=" + header));
+ }
+ final DataStreamReply reply = replies.poll();
+ if (reply != null) {
+ return CompletableFuture.completedFuture(reply);
+ }
+ if (readException != null) {
+ return JavaUtils.completeExceptionally(readException);
+ }
+ if (endOfStream) {
+ return JavaUtils.completeExceptionally(newEndOfStreamException());
+ }
+ final CompletableFuture<DataStreamReply> f = new CompletableFuture<>();
+ pendingReads.add(f);
+ return f;
+ }
+
+ @Override
+ public synchronized void close() {
+ closed = true;
+ for (DataStreamReply reply; (reply = replies.poll()) != null;) {
+ DataStreamReplyByteBuf.release(reply);
+ }
+ failReads(new AlreadyClosedException(clientId + ": stream already
closed, request=" + header));
Review Comment:
`close()` could happen while `readAsync()` calls are pending.
##########
ratis-common/src/main/java/org/apache/ratis/datastream/impl/DataStreamReplyByteBuf.java:
##########
@@ -118,4 +118,10 @@ static ByteBuffer copy(ByteBuf buf) {
buf.readBytes(bytes);
return ByteBuffer.wrap(bytes);
}
+
+ public static void release(DataStreamReply reply) {
+ if (reply instanceof DataStreamReplyByteBuf) {
+ ((DataStreamReplyByteBuf) reply).release();
+ }
Review Comment:
there are two class implement the `DataStreamReply`, which is
`DataStreamReplyByteBuf` and `DataStreamReplyByteBuffer`.
And only `DataStreamReplyByteBuf` need to release the Netty byteBuffer
reference count
https://github.com/apache/ratis/blob/3cb6794bcb9fcd35dfd8bd67a885a70cc9de7a56/ratis-common/src/main/java/org/apache/ratis/datastream/impl/DataStreamPacketByteBuf.java#L58-L63
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
