szetszwo commented on a change in pull request #164:
URL: https://github.com/apache/incubator-ratis/pull/164#discussion_r465264228
##########
File path:
ratis-datastream/src/main/java/org/apache/ratis/datastream/objects/DataStreamReply.java
##########
@@ -0,0 +1,7 @@
+package org.apache.ratis.datastream.objects;
+
+public class DataStreamReply {
Review comment:
Change both DataStreamRequest and DataStreamReply to interfaces so that
we will have methods like getStreamId() but no fields.
##########
File path:
ratis-datastream/src/main/java/org/apache/ratis/datastream/server/DataStreamServer.java
##########
@@ -0,0 +1,30 @@
+package org.apache.ratis.datastream.server;
+
+import org.apache.ratis.datastream.objects.DataStreamReply;
+import org.apache.ratis.datastream.objects.DataStreamRequest;
+
+import java.util.concurrent.CompletableFuture;
+
+/**
+ * A server interface handling incoming streams
+ * Relays those streams to other servers after persisting
+ * It will have an associated Netty client, server for streaming and listening.
+ */
+public interface DataStreamServer {
+
+ /**
+ * Invoked from the server to persist data and add to relay queue.
+ */
+ boolean persistAndAdd(DataStreamRequest request);
+
+ /**
+ * Poll the queue and trigger streaming for messages in relay queue.
+ */
+ CompletableFuture<DataStreamReply> streamAsync(DataStreamRequest request);
+
+ /**
+ * receive a reply from the client and set the necessary future.
+ * Invoked by the Netty Client associated with the object.
+ */
+ void setFuture(DataStreamReply reply);
Review comment:
rename to setReply
##########
File path: ratis-datastream/pom.xml
##########
@@ -0,0 +1,35 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0";
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0
http://maven.apache.org/xsd/maven-4.0.0.xsd";>
+ <parent>
+ <artifactId>ratis</artifactId>
+ <groupId>org.apache.ratis</groupId>
+ <version>1.1.0-SNAPSHOT</version>
+ </parent>
+ <modelVersion>4.0.0</modelVersion>
+
+ <artifactId>ratis-datastream</artifactId>
Review comment:
We must separated the client and the server modules since client is
light-weighted but the server is not. So we should either add two modules or
just using the existing Ratis-client and Ratis-server modules. I prefer the
latter.
##########
File path:
ratis-datastream/src/main/java/org/apache/ratis/datastream/server/DataStreamServer.java
##########
@@ -0,0 +1,30 @@
+package org.apache.ratis.datastream.server;
+
+import org.apache.ratis.datastream.objects.DataStreamReply;
+import org.apache.ratis.datastream.objects.DataStreamRequest;
+
+import java.util.concurrent.CompletableFuture;
+
+/**
+ * A server interface handling incoming streams
+ * Relays those streams to other servers after persisting
+ * It will have an associated Netty client, server for streaming and listening.
+ */
+public interface DataStreamServer {
+
+ /**
+ * Invoked from the server to persist data and add to relay queue.
+ */
+ boolean persistAndAdd(DataStreamRequest request);
Review comment:
rename to receiveRequest (the implementation could decide what to do)
Also change this and all other methods to return CompletableFuture.
##########
File path:
ratis-datastream/src/main/java/org/apache/ratis/datastream/objects/DataStreamRequest.java
##########
@@ -0,0 +1,10 @@
+package org.apache.ratis.datastream.objects;
+
+import org.apache.ratis.thirdparty.io.netty.buffer.CompositeByteBuf;
+
+public class DataStreamRequest {
+ long streamId;
+ long messageId;
Review comment:
Use offset and length.
##########
File path:
ratis-datastream/src/main/java/org/apache/ratis/datastream/OutboundDataStream.java
##########
@@ -0,0 +1,24 @@
+package org.apache.ratis.datastream;
+
+import org.apache.ratis.datastream.objects.DataStreamReply;
+import org.apache.ratis.protocol.Message;
+import org.apache.ratis.protocol.RaftClientReply;
+import org.apache.ratis.thirdparty.io.netty.buffer.ByteBuf;
Review comment:
We must not use any netty in the API since the API should support
multiple implementations.
##########
File path:
ratis-datastream/src/main/java/org/apache/ratis/datastream/objects/DataStreamReply.java
##########
@@ -0,0 +1,7 @@
+package org.apache.ratis.datastream.objects;
+
+public class DataStreamReply {
+ long streamId;
+ long messageId;
+ String response;
Review comment:
response should be binary so that it can encode exceptions.
----------------------------------------------------------------
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]
