You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2021/07/22 12:28:30 UTC

[GitHub] [ozone] captainzmc opened a new pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

captainzmc opened a new pull request #2452:
URL: https://github.com/apache/ozone/pull/2452


   ## What changes were proposed in this pull request?
   
   Before we add new BlockDataStreamOutput, we need to support stream setup. In this way, we can get the DataStreamOutput instance through xceiverClient.
   
   We can DataStreamOutput instance as we used in our [POC code.](https://github.com/captainzmc/hadoop-ozone/blob/streaming-poc-2/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockOutputStream.java#L175)
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-5480
   
   ## How was this patch tested?
   
   CI test will be added after DataStreamOutput Client is finished
   


-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] bshashikant commented on a change in pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
bshashikant commented on a change in pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#discussion_r674861974



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/XceiverServerRatis.java
##########
@@ -227,6 +231,25 @@ private RaftProperties newRaftProperties() {
 
     // set the configs enable and set the stateMachineData sync timeout
     RaftServerConfigKeys.Log.StateMachineData.setSync(properties, true);
+
+    dataStreamPort = conf.getInt(
+            OzoneConfigKeys.DFS_CONTAINER_RATIS_DATASTREAM_IPC_PORT,
+            OzoneConfigKeys.DFS_CONTAINER_RATIS_DATASTREAM_IPC_PORT_DEFAULT);
+    // set the datastream config
+    NettyConfigKeys.DataStream.setPort(properties, dataStreamPort);
+    RaftConfigKeys.DataStream.setType(properties,
+        SupportedDataStreamType.NETTY);

Review comment:
       can we do these stream specfic settings in a different function?

##########
File path: hadoop-hdds/interface-client/src/main/proto/DatanodeClientProtocol.proto
##########
@@ -391,7 +393,7 @@ enum ChecksumType {
 
 message  WriteChunkRequestProto  {
   required DatanodeBlockID blockID = 1;
-  required ChunkInfo chunkData = 2;
+  optional ChunkInfo chunkData = 2;

Review comment:
       isn't this backword incompatible?

##########
File path: hadoop-hdds/common/src/main/resources/ozone-default.xml
##########
@@ -53,6 +53,24 @@
     <tag>OZONE, CONTAINER, MANAGEMENT</tag>
     <description>The ipc port number of container.</description>
   </property>
+  <property>
+    <name>dfs.container.ratis.datastream.ipc</name>
+    <value>9890</value>
+    <tag>OZONE, CONTAINER, RATIS, DATASTREAM</tag>
+    <description>The datastream ipc port number of container.</description>
+  </property>
+  <property>
+    <name>dfs.container.ratis.datastream.request.threads</name>
+    <value>20</value>
+    <tag>OZONE, CONTAINER, RATIS, DATASTREAM</tag>

Review comment:
       Cab we move these settings to DatanodeRatisServerConfig and clinet config keys separately?

##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
##########
@@ -230,6 +233,40 @@ public BlockManager getBlockManager() {
     return this.blockManager;
   }
 
+  ContainerCommandResponseProto handleStreamInit(
+      ContainerCommandRequestProto request, KeyValueContainer kvContainer,
+      DispatcherContext dispatcherContext) {
+    if (!request.hasWriteChunk()) {
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("Malformed Write Chunk request. trace ID: {}",
+            request.getTraceID());
+      }
+      return malformedRequest(request);
+    }
+
+    String path = null;
+    try {
+      checkContainerOpen(kvContainer);
+
+      WriteChunkRequestProto writeChunk = request.getWriteChunk();
+      BlockID blockID = BlockID.getFromProtobuf(writeChunk.getBlockID());
+
+      if (dispatcherContext == null) {
+        dispatcherContext = new DispatcherContext.Builder().build();
+      }
+
+      path = chunkManager
+          .streamInit(kvContainer, blockID, dispatcherContext);

Review comment:
       I feel, we should set the dispacher contect as null here. Dispacher context being null during write will signify its a stream opeartion.
   
   Also, stream classes should be coupled with ChunkManager. Let's maintain a clear separation. May be we can use StreamHandler. Common functionalities can be moved to chunkUtils.

##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java
##########
@@ -283,6 +284,13 @@ public ContainerCommandResponseProto sendCommand(
     return responseProtoHashMap;
   }
 
+  // TODO: We need separate XceiverClientRatis and XceiverClientGrpc instances
+  //  and remove XceiverClientSpi
+  @Override
+  public DataStreamApi getDataStreamApi() {
+    return null;
+  }

Review comment:
       Its better to throw an exception here.




-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] captainzmc edited a comment on pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
captainzmc edited a comment on pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#issuecomment-885376538


   > Thanks @captainzmc for working on this. I think, we need define a config option to enable/disable the feature both on client and server.
   
   Thanks @bshashikant for the review and suggestions.
   
   On the server side, I think it's easy to distinguish. We can added a config and check in XceiverServerRatis , to confirm whether eanble stream in RaftServer. 
   About the client. Can we write a key based on the size of the key and decide whether to use stream?  For example, we can configure this in putKeyHandler.java.  And if the user uses the API directly, they can call our new interface directly. We won't make any changes to the existing Key/Block OutPutStream. We will add some new interfaces as described in the previous design document values.
   ![image](https://user-images.githubusercontent.com/13825159/126734705-7f37384d-9917-4ff9-a199-15ce7d232668.png)
   


-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] captainzmc edited a comment on pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
captainzmc edited a comment on pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#issuecomment-885376538


   > Thanks @captainzmc for working on this. I think, we need define a config option to enable/disable the feature both on client and server.
   
   Thanks @bshashikant for the review and suggestions.
   
   On the server side, I think it's easy to distinguish. We can added a config and check in XceiverServerRatis , to confirm whether eanble stream in RaftServer. 
   About the client. Can we write a key based on the size of the key and decide whether to use stream?  For example, we can add a configure and check this in putKeyHandler.java.  If the key is smaller than the chunk size, follow the original logic. If greater than, then use streaming.
   And if the user uses the API directly, they can call our new interface directly. We won't make any changes to the existing Key/Block OutPutStream. We will add some new interfaces as described in the previous design document values. We also need to add new interfaces to RpcClient and OzoneBucket to use these interfaces.
   ![image](https://user-images.githubusercontent.com/13825159/126734705-7f37384d-9917-4ff9-a199-15ce7d232668.png)
   


-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] captainzmc commented on a change in pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#discussion_r677222330



##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java
##########
@@ -283,6 +284,14 @@ public ContainerCommandResponseProto sendCommand(
     return responseProtoHashMap;
   }
 
+  // TODO: We need separate XceiverClientRatis and XceiverClientGrpc instances
+  //  and remove XceiverClientSpi
+  @Override
+  public DataStreamApi getDataStreamApi() {
+    throw new UnsupportedOperationException(
+        "Operation Not supported for XceiverClientGrpc");
+  }

Review comment:
       Yes, we can add this when we implement the client




-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] captainzmc closed pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
captainzmc closed pull request #2452:
URL: https://github.com/apache/ozone/pull/2452


   


-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] szetszwo commented on pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
szetszwo commented on pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#issuecomment-886856458


   @captainzmc , Just have went through the entire change once.  The change looks great!  Will review this again tomorrow.
   
   In the mean time, please take a look the build failures.  Thanks a lot.


-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] captainzmc commented on a change in pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#discussion_r675282520



##########
File path: hadoop-hdds/interface-client/src/main/proto/DatanodeClientProtocol.proto
##########
@@ -391,7 +393,7 @@ enum ChecksumType {
 
 message  WriteChunkRequestProto  {
   required DatanodeBlockID blockID = 1;
-  required ChunkInfo chunkData = 2;
+  optional ChunkInfo chunkData = 2;

Review comment:
       Changed this from required to optional is compatible. The previous interface WriteChunkRequestProto must have chunkData. Init stream will use WriteChunkRequestProto, but do not require chunkData. Just like we implemented in the [POC code](https://github.com/captainzmc/hadoop-ozone/blob/streaming-poc-2/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockOutputStream.java#L162).




-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] captainzmc commented on a change in pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#discussion_r677232778



##########
File path: hadoop-hdds/common/src/main/resources/ozone-default.xml
##########
@@ -53,6 +53,24 @@
     <tag>OZONE, CONTAINER, MANAGEMENT</tag>
     <description>The ipc port number of container.</description>
   </property>
+  <property>
+    <name>dfs.container.ratis.datastream.ipc</name>
+    <value>9890</value>
+    <tag>OZONE, CONTAINER, RATIS, DATASTREAM</tag>
+    <description>The datastream ipc port number of container.</description>
+  </property>
+  <property>
+    <name>dfs.container.ratis.datastream.request.threads</name>
+    <value>20</value>
+    <tag>OZONE, CONTAINER, RATIS, DATASTREAM</tag>

Review comment:
       Had moved datastream.request.threads and datastream.write.threads to DatanodeRatisServerConfig.  The rest of the configurations are similar to [those related to Container](https://github.com/apache/ozone/pull/2452/files#diff-148cad3e4555d212af7e3c58a71fb17b2782587d89659f666227c6d92d7d898cR77), and I keep them in this place.




-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] szetszwo merged pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
szetszwo merged pull request #2452:
URL: https://github.com/apache/ozone/pull/2452


   


-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] mukul1987 commented on pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
mukul1987 commented on pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#issuecomment-884965915


   One more question, whats the use of stream init, and how is it different from the first data packet to be sent from client to the datanode ?


-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] captainzmc commented on pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#issuecomment-885376538


   > Thanks @captainzmc for working on this. I think, we need define a config option to enable/disable the feature both on client and server.
   
   Thanks @bshashikant for the review and suggestions.
   
   On the server side, I think it's easy to distinguish. We added a stream method to ContainerStateMachine that follows the stream Init logic. This method is executed only if invoked actively by the client.  The new interface on the server has no impact on the existing logic.
   About the client. Can we write a key based on the size of the key and decide whether to use stream?  We can configure in putKeyHandler.java.  And if the user uses the API directly, they can call our new interface directly. We won't make any changes to the existing Key/Block OutPutStream. We will add some new interfaces as described in the previous design document values.
   ![image](https://user-images.githubusercontent.com/13825159/126734705-7f37384d-9917-4ff9-a199-15ce7d232668.png)
   


-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] captainzmc edited a comment on pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
captainzmc edited a comment on pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#issuecomment-885376538


   > Thanks @captainzmc for working on this. I think, we need define a config option to enable/disable the feature both on client and server.
   
   Thanks @bshashikant for the review and suggestions.
   
   On the server side, I think it's easy to distinguish. We can added a config and check in XceiverServerRatis , to confirm whether eanble stream in RaftServer. 
   About the client. Can we write a key based on key size  and decide whether to use stream?  For example, we can add a configure and check this in putKeyHandler.java.  If the key is smaller than the chunk size(4MB), follow the original logic. If greater than, then use streaming.
   And if the user uses the API directly, they can call our new interface directly. We won't make any changes to the existing Key/Block OutPutStream. We will add some new interfaces as described in the previous design. We also need to add new interfaces to RpcClient and OzoneBucket to use these interfaces.
   ![image](https://user-images.githubusercontent.com/13825159/126734705-7f37384d-9917-4ff9-a199-15ce7d232668.png)
   


-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] captainzmc commented on pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#issuecomment-885460652


   I will solve the remaining review and CI problems.


-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] bshashikant commented on pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
bshashikant commented on pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#issuecomment-884976963


   Thanks @captainzmc for working on this. I think, we need define a config option to enable/disable the feature both on client and server.


-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] captainzmc edited a comment on pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
captainzmc edited a comment on pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#issuecomment-885376538


   > Thanks @captainzmc for working on this. I think, we need define a config option to enable/disable the feature both on client and server.
   
   Thanks @bshashikant for the review and suggestions.
   
   On the server side, I think it's easy to distinguish. We can added a config and check in XceiverServerRatis , to confirm whether eanble stream in RaftServer. 
   About the client. Can we write a key based on the size of the key and decide whether to use stream?  For example, we can add a configure and check this in putKeyHandler.java.  If the key is smaller than the chunk size, follow the original logic. If greater than, then use streaming.
   And if the user uses the API directly, they can call our new interface directly. We won't make any changes to the existing Key/Block OutPutStream. We will add some new interfaces as described in the previous design document values.
   ![image](https://user-images.githubusercontent.com/13825159/126734705-7f37384d-9917-4ff9-a199-15ce7d232668.png)
   


-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] captainzmc commented on pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#issuecomment-887154507


   > @captainzmc , Just have went through the entire change once. The change looks great! Will review this again tomorrow.
   > 
   > In the mean time, please take a look the build failures. Thanks a lot.
   
   Yes, now I am solving the problem of CI failure, and I will update this PR again after solving 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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] captainzmc edited a comment on pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
captainzmc edited a comment on pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#issuecomment-885376538


   > Thanks @captainzmc for working on this. I think, we need define a config option to enable/disable the feature both on client and server.
   
   Thanks @bshashikant for the review and suggestions.
   
   On the server side, I think it's easy to distinguish. We added a stream method to ContainerStateMachine that follows the stream Init logic. This method is executed only if invoked actively by the client.  The new interface on the server has no impact on the existing logic.
   About the client. Can we write a key based on the size of the key and decide whether to use stream?  For example, we can configure this in putKeyHandler.java.  And if the user uses the API directly, they can call our new interface directly. We won't make any changes to the existing Key/Block OutPutStream. We will add some new interfaces as described in the previous design document values.
   ![image](https://user-images.githubusercontent.com/13825159/126734705-7f37384d-9917-4ff9-a199-15ce7d232668.png)
   


-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] captainzmc commented on pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#issuecomment-888011462


   Hi @szetszwo @bshashikant @mukul1987 CI had been fixed. Could you help take another look? 


-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] captainzmc commented on a change in pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#discussion_r674841027



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java
##########
@@ -199,7 +199,8 @@ private ContainerCommandResponseProto dispatchRequest(
     boolean isWriteStage =
         (cmdType == Type.WriteChunk && dispatcherContext != null
             && dispatcherContext.getStage()
-            == DispatcherContext.WriteChunkStage.WRITE_DATA);
+            == DispatcherContext.WriteChunkStage.WRITE_DATA)
+            || (cmdType == Type.StreamInit);

Review comment:
       Not yet now. But next step, when our client initializes the DataStreamOutput, we will set CmdType to type.streamInit. As we implement in our [POC code](https://github.com/captainzmc/hadoop-ozone/blob/streaming-poc-2/hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockOutputStream.java#L169).




-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] ckj996 commented on pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
ckj996 commented on pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#issuecomment-885365975


   > One more question, whats the use of stream init, and how is it different from the first data packet to be sent from client to the datanode ?
   
   Stream init do 2 things: build the stream pipeline, and get a file path from the container for subsequent writes.
   
   Ratis: Build the stream pipeline according to the routing table. If no routing table is given, it will fallback to
   
   ```
   Client -----> Primary -----> Secondary 1
                    |
                    +---------> Secondary 2
   ```
   
   HDDS Container: prepare a file for write, but does not write any data. In our POC code, we let the ContainerStateMachine calculate the path of that file as if there is a WriteChunk request.
   
   ---
   
   The first data packet, of course, write data into that file.


-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] captainzmc closed pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
captainzmc closed pull request #2452:
URL: https://github.com/apache/ozone/pull/2452


   


-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] captainzmc commented on a change in pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#discussion_r675283554



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
##########
@@ -230,6 +233,40 @@ public BlockManager getBlockManager() {
     return this.blockManager;
   }
 
+  ContainerCommandResponseProto handleStreamInit(
+      ContainerCommandRequestProto request, KeyValueContainer kvContainer,
+      DispatcherContext dispatcherContext) {
+    if (!request.hasWriteChunk()) {
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("Malformed Write Chunk request. trace ID: {}",
+            request.getTraceID());
+      }
+      return malformedRequest(request);
+    }
+
+    String path = null;
+    try {
+      checkContainerOpen(kvContainer);
+
+      WriteChunkRequestProto writeChunk = request.getWriteChunk();
+      BlockID blockID = BlockID.getFromProtobuf(writeChunk.getBlockID());
+
+      if (dispatcherContext == null) {
+        dispatcherContext = new DispatcherContext.Builder().build();
+      }
+
+      path = chunkManager
+          .streamInit(kvContainer, blockID, dispatcherContext);

Review comment:
       I would try to change the current implementation in this way.




-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] captainzmc commented on a change in pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#discussion_r674829182



##########
File path: hadoop-hdds/common/src/main/resources/ozone-default.xml
##########
@@ -53,6 +53,24 @@
     <tag>OZONE, CONTAINER, MANAGEMENT</tag>
     <description>The ipc port number of container.</description>
   </property>
+  <property>
+    <name>dfs.container.ratis.datastream.ipc</name>
+    <value>9890</value>

Review comment:
       I will fix the problem and we should not use port 9890. This will conflict with the port in docker-compose




-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] captainzmc edited a comment on pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
captainzmc edited a comment on pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#issuecomment-885376538


   > Thanks @captainzmc for working on this. I think, we need define a config option to enable/disable the feature both on client and server.
   
   Thanks @bshashikant for the review and suggestions.
   
   On the server side, I think it's easy to distinguish. We can added a config and check in XceiverServerRatis , to confirm whether eanble stream in RaftServer. 
   About the client. Can we write a key based on the size of the key and decide whether to use stream?  For example, we can add a configure and check this in putKeyHandler.java.  
   And if the user uses the API directly, they can call our new interface directly. We won't make any changes to the existing Key/Block OutPutStream. We will add some new interfaces as described in the previous design document values.
   ![image](https://user-images.githubusercontent.com/13825159/126734705-7f37384d-9917-4ff9-a199-15ce7d232668.png)
   


-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] bshashikant commented on pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
bshashikant commented on pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#issuecomment-885008538


   > One more question, whats the use of stream init, and how is it different from the first data packet to be sent from client to the datanode ?
   
   The strream init will be used to instantiate the streaming client instance and the will try to write the 1st packet. 
   


-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] szetszwo commented on a change in pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
szetszwo commented on a change in pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#discussion_r677143241



##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java
##########
@@ -79,6 +85,18 @@
       "dfs.container.ratis.server.port";
   public static final int DFS_CONTAINER_RATIS_SERVER_PORT_DEFAULT = 9856;
 
+  /**
+   * Ratis Port where containers listen to datastream requests.
+   */
+  public static final String DFS_CONTAINER_RATIS_DATASTREAM_ENABLE
+      = "dfs.container.ratis.datastream.enable";
+  public static final boolean DFS_CONTAINER_RATIS_DATASTREAM_ENABLE_DEFAULT
+      = true;
+  public static final String DFS_CONTAINER_RATIS_DATASTREAM_IPC_PORT
+      = "dfs.container.ratis.datastream.ipc";

Review comment:
       Let's rename this to "dfs.container.ratis.datastream.port".

##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/protocol/DatanodeDetails.java
##########
@@ -774,7 +774,8 @@ public static Port newPort(Port.Name name, Integer value) {
      * Ports that are supported in DataNode.
      */
     public enum Name {
-      STANDALONE, RATIS, REST, REPLICATION, RATIS_ADMIN, RATIS_SERVER;
+      STANDALONE, RATIS, REST, REPLICATION, RATIS_ADMIN, RATIS_SERVER,
+      DATASTREAM;

Review comment:
       Let's call it RATIS_DATASTREAM.

##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java
##########
@@ -283,6 +284,14 @@ public ContainerCommandResponseProto sendCommand(
     return responseProtoHashMap;
   }
 
+  // TODO: We need separate XceiverClientRatis and XceiverClientGrpc instances
+  //  and remove XceiverClientSpi
+  @Override
+  public DataStreamApi getDataStreamApi() {
+    throw new UnsupportedOperationException(
+        "Operation Not supported for XceiverClientGrpc");
+  }

Review comment:
       getDataStreamApi is yet not used.  Let's add it later when it is being called.
   
   BTW, we may only add the method to XceiverClientRatis and cast the object for calling it as below.
   ```
   XceiverClientSpi client = ...
   DataStreamApi streamApi = ((XceiverClientRatis)client).getDataStreamApi();
   ```

##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java
##########
@@ -57,6 +57,12 @@
   public static final boolean DFS_CONTAINER_IPC_RANDOM_PORT_DEFAULT =
       false;
 
+  public static final String DFS_CONTAINER_RATIS_DATASTREAM_IPC_RANDOM_PORT =
+      "dfs.container.ratis.datastream.ipc.random.port";

Review comment:
       Let's remove "ipc", i.e. dfs.container.ratis.datastream.random.port".  It is not really an ipc.

##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/RatisHelper.java
##########
@@ -170,8 +172,12 @@ public static RaftGroup newRaftGroup(Pipeline pipeline) {
   public static RaftClient newRaftClient(RpcType rpcType, Pipeline pipeline,
       RetryPolicy retryPolicy, GrpcTlsConfig tlsConfig,
       ConfigurationSource ozoneConfiguration) throws IOException {
+    // TODO: For now, always set leader as primary. In the future we should
+    //  use the local or closest node in the pipline.
+    final DatanodeDetails leader = pipeline.getLeaderNode();
     return newRaftClient(rpcType,
-        toRaftPeerId(pipeline.getLeaderNode()),
+        toRaftPeerId(leader),
+        toRaftPeer(leader),

Review comment:
       Let's use the first node:
   ```
           toRaftPeer(pipeline.getFirstNode()),
   ```
   

##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/XceiverServerRatis.java
##########
@@ -177,6 +184,7 @@ private XceiverServerRatis(DatanodeDetails dd,
         HddsConfigKeys.HDDS_DATANODE_RATIS_SERVER_REQUEST_TIMEOUT,
         HddsConfigKeys.HDDS_DATANODE_RATIS_SERVER_REQUEST_TIMEOUT_DEFAULT,
         TimeUnit.MILLISECONDS);
+

Review comment:
       Let's revert this whitespace change.




-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] captainzmc commented on a change in pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#discussion_r677223582



##########
File path: hadoop-hdds/interface-client/src/main/proto/DatanodeClientProtocol.proto
##########
@@ -100,6 +100,8 @@ enum Type {
   GetSmallFile = 16;
   CloseContainer = 17;
   GetCommittedBlockLength = 18;
+
+  StreamInit = 19;

Review comment:
       I tried to change this to 30, but I got an NPE exception when start datanode. Change this to 19 in order and it start normal.




-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] captainzmc commented on a change in pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
captainzmc commented on a change in pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#discussion_r677232778



##########
File path: hadoop-hdds/common/src/main/resources/ozone-default.xml
##########
@@ -53,6 +53,24 @@
     <tag>OZONE, CONTAINER, MANAGEMENT</tag>
     <description>The ipc port number of container.</description>
   </property>
+  <property>
+    <name>dfs.container.ratis.datastream.ipc</name>
+    <value>9890</value>
+    <tag>OZONE, CONTAINER, RATIS, DATASTREAM</tag>
+    <description>The datastream ipc port number of container.</description>
+  </property>
+  <property>
+    <name>dfs.container.ratis.datastream.request.threads</name>
+    <value>20</value>
+    <tag>OZONE, CONTAINER, RATIS, DATASTREAM</tag>

Review comment:
       Had moved datastream.request.threads and datastream.write.threads to DatanodeRatisServerConfig.  The rest of the configurations are similar to [those related to Container](https://github.com/apache/ozone/pull/2452/files#diff-148cad3e4555d212af7e3c58a71fb17b2782587d89659f666227c6d92d7d898cR77), and I keep them in this place for better management.




-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] szetszwo commented on a change in pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
szetszwo commented on a change in pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#discussion_r677474151



##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/RatisHelper.java
##########
@@ -170,8 +173,12 @@ public static RaftGroup newRaftGroup(Pipeline pipeline) {
   public static RaftClient newRaftClient(RpcType rpcType, Pipeline pipeline,
       RetryPolicy retryPolicy, GrpcTlsConfig tlsConfig,
       ConfigurationSource ozoneConfiguration) throws IOException {
+    // TODO: For now, always set first node as primary. In the future we should
+    //  use the local or closest node in the pipline.
+    final DatanodeDetails leader = pipeline.getLeaderNode();

Review comment:
       The TODO can be removed since the first datanode is supposed to be the closest datanode.  SCM is supposed to sort the datanode in the pipeline according the distance to the client.  Not sure if SCM is currently sorting the datanodes.  We should check the code.
   
   BTW, the leader local variable is no longer needed.  We should revert the change.




-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] captainzmc edited a comment on pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
captainzmc edited a comment on pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#issuecomment-885376538


   > Thanks @captainzmc for working on this. I think, we need define a config option to enable/disable the feature both on client and server.
   
   Thanks @bshashikant for the review and suggestions.
   
   On the server side, I think it's easy to distinguish. We can added a config and check in XceiverServerRatis , to confirm whether eanble stream in RaftServer. 
   About the client. Can we write a key based on key size  and decide whether to use stream?  For example, we can add a configure and check this in putKeyHandler.java.  If the key is smaller than the chunk size, follow the original logic. If greater than, then use streaming.
   And if the user uses the API directly, they can call our new interface directly. We won't make any changes to the existing Key/Block OutPutStream. We will add some new interfaces as described in the previous design. We also need to add new interfaces to RpcClient and OzoneBucket to use these interfaces.
   ![image](https://user-images.githubusercontent.com/13825159/126734705-7f37384d-9917-4ff9-a199-15ce7d232668.png)
   


-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] captainzmc edited a comment on pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
captainzmc edited a comment on pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#issuecomment-885376538


   > Thanks @captainzmc for working on this. I think, we need define a config option to enable/disable the feature both on client and server.
   
   Thanks @bshashikant for the review and suggestions.
   
   On the server side, I think it's easy to distinguish. We added a config and check in XceiverServerRatis , to confirm whether eanble stream in RaftServer. 
   About the client. Can we write a key based on the size of the key and decide whether to use stream?  For example, we can configure this in putKeyHandler.java.  And if the user uses the API directly, they can call our new interface directly. We won't make any changes to the existing Key/Block OutPutStream. We will add some new interfaces as described in the previous design document values.
   ![image](https://user-images.githubusercontent.com/13825159/126734705-7f37384d-9917-4ff9-a199-15ce7d232668.png)
   


-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] captainzmc edited a comment on pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
captainzmc edited a comment on pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#issuecomment-885376538


   > Thanks @captainzmc for working on this. I think, we need define a config option to enable/disable the feature both on client and server.
   
   Thanks @bshashikant for the review and suggestions.
   
   On the server side, I think it's easy to distinguish. We can added a config and check in XceiverServerRatis , to confirm whether eanble stream in RaftServer. 
   About the client. Can we write a key based on the size of the key and decide whether to use stream?  For example, we can add a configure and check this in putKeyHandler.java.  If the key is smaller than the chunk size, follow the original logic. If greater than, then use streaming.
   And if the user uses the API directly, they can call our new interface directly. We won't make any changes to the existing Key/Block OutPutStream. We will add some new interfaces as described in the previous design. We also need to add new interfaces to RpcClient and OzoneBucket to use these interfaces.
   ![image](https://user-images.githubusercontent.com/13825159/126734705-7f37384d-9917-4ff9-a199-15ce7d232668.png)
   


-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org


[GitHub] [ozone] mukul1987 commented on a change in pull request #2452: HDDS-5480. [Ozone-Streaming] Client and server should support stream setup.

Posted by GitBox <gi...@apache.org>.
mukul1987 commented on a change in pull request #2452:
URL: https://github.com/apache/ozone/pull/2452#discussion_r674753841



##########
File path: hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/RatisHelper.java
##########
@@ -170,8 +172,12 @@ public static RaftGroup newRaftGroup(Pipeline pipeline) {
   public static RaftClient newRaftClient(RpcType rpcType, Pipeline pipeline,
       RetryPolicy retryPolicy, GrpcTlsConfig tlsConfig,
       ConfigurationSource ozoneConfiguration) throws IOException {
+    // TODO: For now, always set leader as primary. In the future we should

Review comment:
       Lets create new jira for this

##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientGrpc.java
##########
@@ -283,6 +284,13 @@ public ContainerCommandResponseProto sendCommand(
     return responseProtoHashMap;
   }
 
+  // TODO: We need separate XceiverClientRatis and XceiverClientGrpc instances

Review comment:
       Lets create a jira for this

##########
File path: hadoop-hdds/interface-client/src/main/proto/DatanodeClientProtocol.proto
##########
@@ -100,6 +100,8 @@ enum Type {
   GetSmallFile = 16;
   CloseContainer = 17;
   GetCommittedBlockLength = 18;
+
+  StreamInit = 19;

Review comment:
       lets start this at 30.

##########
File path: hadoop-hdds/common/src/main/resources/ozone-default.xml
##########
@@ -53,6 +53,24 @@
     <tag>OZONE, CONTAINER, MANAGEMENT</tag>
     <description>The ipc port number of container.</description>
   </property>
+  <property>
+    <name>dfs.container.ratis.datastream.ipc</name>
+    <value>9890</value>

Review comment:
       These values are different from the values in the java conf file.

##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/HddsDispatcher.java
##########
@@ -199,7 +199,8 @@ private ContainerCommandResponseProto dispatchRequest(
     boolean isWriteStage =
         (cmdType == Type.WriteChunk && dispatcherContext != null
             && dispatcherContext.getStage()
-            == DispatcherContext.WriteChunkStage.WRITE_DATA);
+            == DispatcherContext.WriteChunkStage.WRITE_DATA)
+            || (cmdType == Type.StreamInit);

Review comment:
       are there any callers sending stream init at this time ?




-- 
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@ozone.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org