amaliujia commented on code in PR #1481:
URL: https://github.com/apache/ratis/pull/1481#discussion_r3440604728
##########
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:
Do we need to call `close()` or mark `closed = true;` here?
##########
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;
Review Comment:
Add a `if (closed) return` to maintain idempotent close?
##########
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:
I do not understand why we need to do a `failReads` upon a ordinary close.
Do we not expect `close` be call in the happy path and we only call close in
bad cases?
##########
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:
What about else? Is that possible other instances are passed here? If no
maybe add an assertion, otherwise handle other instances?
--
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]
