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 2022/03/17 13:45:10 UTC

[GitHub] [ozone] guohao-rosicky opened a new pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

guohao-rosicky opened a new pull request #3195:
URL: https://github.com/apache/ozone/pull/3195


   ## What changes were proposed in this pull request?
   
   Optimized the putBlock method of BlockDataStreamOutput
   
   
   
   1. PutBlock does not use RaftRpc, but through the stream close() method
   ```
   // putBlock metadata
   
   ByteBuffer buf =
       ContainerCommandRequestMessage.toMessage(putBlockRequestProto, null)
           .getContent().asReadOnlyByteBuffer();
   
   out.writeAsync(buf, StandardWriteOption.CLOSE); 
   
   ```
   
   
   2.Mark the stream data boundary by sending an empty stream packet
   ```
   // Mark the stream data boundary
   
   out.writeAsync(ByteBuffer.allocateDirect(0).asReadOnlyBuffer()); 
   ```
   
   3. Flush is also in the form of async RPC, because putBlock needs to fetch bcsId (Raft log index)
   
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-6137
   
   ## How was this patch tested?
   
   Use the existing UT
   


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   @guohao-rosicky , since you have done some initial code refactoring (the common code here and #2860 , could you push it in this pull request so that we can merge it?
   
   Then, I will file another JIRA for the putBlockRequest serialization and the buffering in KeyValueStreamDataChannel.


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   > @captainzmc , @guohao-rosicky, Fixed the bug; see https://issues.apache.org/jira/secure/attachment/13041255/3195_TestStreamDataChannel.patch
   
   Thanks @szetszwo  for the update. Using new patch I found  IndexOutOfBoundsException still appear.  
   I found that the ERROR only appeared when writing with high concurrency. I was successful with 30 concurrent writes, but I get the above exception with 600 concurrent writes.
   ![image](https://user-images.githubusercontent.com/13825159/158751660-57d9e0a0-8aa6-44bc-bd50-1f6a242b2ca9.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] szetszwo commented on a change in pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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



##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockDataStreamOutput.java
##########
@@ -438,12 +439,39 @@ private void executePutBlock(boolean close,
       byteBufferList = null;
     }
     waitFuturesComplete();
+    BlockData blockData = containerBlockData.build();
     if (close) {
-      dataStreamCloseReply = out.closeAsync();
+      out.writeAsync(ByteBuffer.allocateDirect(0).asReadOnlyBuffer())

Review comment:
       @guohao-rosicky, Writing an empty buffer may not work as expected.  In the streaming model, data is continuous but not block-by-block.  Both sender and receiver must consider that the data is a stream of bytes.
   
   For example, the sender may write 3 times with lengths 3, 4, 5.  Since there could be buffers/encoder/decoder in between, the receiver may receive a single buffer with length 12.  
   
   Let me think about how to make it work.




-- 
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 removed a comment on pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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






-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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



##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockDataStreamOutput.java
##########
@@ -438,12 +439,39 @@ private void executePutBlock(boolean close,
       byteBufferList = null;
     }
     waitFuturesComplete();
+    BlockData blockData = containerBlockData.build();
     if (close) {
-      dataStreamCloseReply = out.closeAsync();
+      out.writeAsync(ByteBuffer.allocateDirect(0).asReadOnlyBuffer())

Review comment:
       @guohao-rosicky , this is a standard problem in streaming.  The standard solution is to write the length with the data.  Let's don't change the API for 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 commented on pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   > @captainzmc , @guohao-rosicky, Fixed the bug; see https://issues.apache.org/jira/secure/attachment/13041255/3195_TestStreamDataChannel.patch
   
   Thanks @szetszwo  for the update. Using new patch I found  IndexOutOfBoundsException still appear.
   ![image](https://user-images.githubusercontent.com/13825159/158751660-57d9e0a0-8aa6-44bc-bd50-1f6a242b2ca9.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] szetszwo commented on pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   @captainzmc , thanks a lot for testing it!
   
   I added one more test for the serialization and buffers.  It worked fine.  See https://issues.apache.org/jira/secure/attachment/13041275/3195_testBuffers.patch
   
   > I found that the ERROR only appeared when writing with high concurrency. I was successful with 30 concurrent writes, but I get the exception with 600 concurrent writes.
   
   It seems there are some synchronization problems.  Let me check.


-- 
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 removed a comment on pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


    @szetszwo Here is my datanode log. https://issues.apache.org/jira/secure/attachment/13041276/ozone-root-datanode-9-29-173-57.log
   he test, I wrote 30 files concurrently. Each file is 128MB and the chunk size is 1MB
   


-- 
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] guohao-rosicky commented on pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on pull request #3195:
URL: https://github.com/apache/ozone/pull/3195#issuecomment-1073460573


   > @guohao-rosicky , thanks a lot for testing it. Does it fix the bug?
   
   I used the new Ratis Patch to test again


-- 
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] guohao-rosicky edited a comment on pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

Posted by GitBox <gi...@apache.org>.
guohao-rosicky edited a comment on pull request #3195:
URL: https://github.com/apache/ozone/pull/3195#issuecomment-1077493971


   > Of course, Ozone may choose to add a header for each call but it will add more overhead and need more code changes.
   
   @szetszwo We can continue this discussion in #3229
   


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   @szetszwo yes here is my [test code](https://github.com/captainzmc/ozonerpc2/blob/master/src/main/java/org/apache/hadoop/ozone/examples/OzoneRpc.java). Is written according to the ratis Filestore test. 
   
   Actually there a problem with my previous description. I noticed that the  when I use 30 threads  writing, we’ll  get same error. 


-- 
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] guohao-rosicky commented on a change in pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on a change in pull request #3195:
URL: https://github.com/apache/ozone/pull/3195#discussion_r827723384



##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockDataStreamOutput.java
##########
@@ -438,12 +439,39 @@ private void executePutBlock(boolean close,
       byteBufferList = null;
     }
     waitFuturesComplete();
+    BlockData blockData = containerBlockData.build();
     if (close) {
-      dataStreamCloseReply = out.closeAsync();
+      out.writeAsync(ByteBuffer.allocateDirect(0).asReadOnlyBuffer())

Review comment:
       Thanks for your comment @szetszwo .   I wrote a piece of pseudo-code, Ideas about the modification of `org.apache.ratis.netty.server.DataStreamManagement#writeTo`.   
   
   ![image](https://user-images.githubusercontent.com/5122636/158544128-f60c7e94-efa3-412a-85ac-7304e284c241.png)
   
   What do you think of this idea? There is no need to send this empty buffer rpc.




-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   @captainzmc , @guohao-rosicky, Fixed the bug; see https://issues.apache.org/jira/secure/attachment/13041255/3195_TestStreamDataChannel.patch


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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






-- 
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] guohao-rosicky edited a comment on pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

Posted by GitBox <gi...@apache.org>.
guohao-rosicky edited a comment on pull request #3195:
URL: https://github.com/apache/ozone/pull/3195#issuecomment-1078626315


   > @guohao-rosicky , since you have done some initial code refactoring (the common code here and #2860 , could you push it in this pull request so that we can merge it?
   > 
   > Then, I will file another JIRA for the putBlockRequest serialization and the buffering in KeyValueStreamDataChannel.
   
   @szetszwo   I extracted common Code, please take a look,Thanks.


-- 
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] guohao-rosicky commented on pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on pull request #3195:
URL: https://github.com/apache/ozone/pull/3195#issuecomment-1078626315


   > @guohao-rosicky , since you have done some initial code refactoring (the common code here and #2860 , could you push it in this pull request so that we can merge it?
   > 
   > Then, I will file another JIRA for the putBlockRequest serialization and the buffering in KeyValueStreamDataChannel.
   
   @szetszwo   Please take a look,Thanks.


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/KeyValueStreamDataChannel.java
##########
@@ -18,17 +18,100 @@
 
 package org.apache.hadoop.ozone.container.keyvalue.impl;
 
+import com.google.common.base.Preconditions;
 import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos;
+import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProto;
+import org.apache.hadoop.hdds.ratis.ContainerCommandRequestMessage;
+import org.apache.hadoop.hdds.scm.ByteStringConversion;
 import org.apache.hadoop.hdds.scm.container.common.helpers.StorageContainerException;
 import org.apache.hadoop.ozone.container.common.helpers.ContainerMetrics;
 import org.apache.hadoop.ozone.container.common.impl.ContainerData;
+import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
+import org.apache.ratis.thirdparty.io.netty.buffer.ByteBuf;
+import org.apache.ratis.thirdparty.io.netty.buffer.Unpooled;
 
 import java.io.File;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.Deque;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.function.Function;
+
+import org.apache.hadoop.hdds.scm.storage.BlockDataStreamOutput;
 
 /**
  * This class is used to get the DataChannel for streaming.
  */
-class KeyValueStreamDataChannel extends StreamDataChannelBase {
+public class KeyValueStreamDataChannel extends StreamDataChannelBase {
+  private static final ByteBuffer[] EMPTY = {};
+  private static final Function<ByteBuffer, ByteString> CONVERTER
+      // TODO: pass OzoneConfigKeys.OZONE_UNSAFEBYTEOPERATIONS_ENABLED
+      = ByteStringConversion.createByteBufferConversion(false);
+
+  /**
+   * Keep the last {@link Buffers#max} bytes in the buffer
+   * in order to create putBlockRequest
+   * at {@link #closeBuffers(Buffers, WriteMethod)}}.
+   */
+  static class Buffers {
+    private final Deque<ByteBuffer> deque = new LinkedList<>();
+    private final int max;
+    private int length;
+
+    Buffers(int max) {
+      this.max = max;
+    }
+
+    private boolean isExtra(int n) {
+      return length - n >= max;

Review comment:
       > ... what is the purpose of being greater than 1M?
   
   We write putBlockRequestProto at the end of the stream but the putBlockRequestProto should not be written to the file.
   The last 1M bytes is kept in the buffer so that we can extract the putBlockRequestProto when the channel is closed.
   




-- 
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] guohao-rosicky commented on pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on pull request #3195:
URL: https://github.com/apache/ozone/pull/3195#issuecomment-1077461064


   @szetszwo  I agree with you, I can split part of #2860 code.
   How can I participate in this work together?


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   @captainzmc , @guohao-rosicky , after some debugging, I believe the problem is that netty is reusing the buffers.  Since we keep the buffer after write and Ratis will then release the buffer, the content inside the buffer may change when the buffer is reused.


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   > @captainzmc , let try buffer copying; see https://issues.apache.org/jira/secure/attachment/13041304/3195_buffer_copying.patch
   
   Thanks @szetszwo ,I had test this patch, IndexOutOfBoundsException is no longer present in the test.  However, I found that after adding buffer copy, the memory of the DataNode becomes very large and datanode keeps reporting IllegalStateException. There are many timeouts in the client, and eventually the client will hung. [Here is log of datanode.] I used 600 concurrent threads to write the file. (https://issues.apache.org/jira/secure/attachment/13041313/ozone-root-datanode-9-29-173-57.log4)
   <img width="1167" alt="image" src="https://user-images.githubusercontent.com/13825159/158996837-c0487dae-9cb2-4436-a798-b20b57192661.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] szetszwo commented on pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   @guohao-rosicky , the idea is to write the proto length at the end as shown below; see also https://issues.apache.org/jira/secure/attachment/13041230/3195_review.patch
   ```
       //   readerIndex   protoIndex   lengthIndex    readerIndex+readableBytes
       //         V            V             V                              V
       // format: |--- data ---|--- proto ---|--- proto length (4 bytes) ---|
   ```


-- 
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] guohao-rosicky commented on pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on pull request #3195:
URL: https://github.com/apache/ozone/pull/3195#issuecomment-1069100755


   > @guohao-rosicky , the idea is to write the proto length at the end as shown below; see also https://issues.apache.org/jira/secure/attachment/13041230/3195_review.patch
   > 
   > ```
   >     //   readerIndex   protoIndex   lengthIndex    readerIndex+readableBytes
   >     //         V            V             V                              V
   >     // format: |--- data ---|--- proto ---|--- proto length (4 bytes) ---|
   > ```
   
   Thanks @szetszwo, Patch has been applied 


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   Thanks @guohao-rosicky @szetszwo update this.  I used this PR test and found that the file failed to write. In DataNode IndexOutOfBoundsException will appear.
   ![image](https://user-images.githubusercontent.com/13825159/158737103-6059c28b-ba72-43bb-b3c7-93ccde49fea2.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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


    @szetszwo Here is my datanode log. https://issues.apache.org/jira/secure/attachment/13041276/ozone-root-datanode-9-29-173-57.log
   he test, I wrote 30 files concurrently. Each file is 128MB and the chunk size is 1MB
   


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/KeyValueStreamDataChannel.java
##########
@@ -18,17 +18,90 @@
 
 package org.apache.hadoop.ozone.container.keyvalue.impl;
 
+import com.google.common.base.Preconditions;
 import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos;
+import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProto;
+import org.apache.hadoop.hdds.ratis.ContainerCommandRequestMessage;
+import org.apache.hadoop.hdds.scm.ByteStringConversion;
 import org.apache.hadoop.hdds.scm.container.common.helpers.StorageContainerException;
 import org.apache.hadoop.ozone.container.common.helpers.ContainerMetrics;
 import org.apache.hadoop.ozone.container.common.impl.ContainerData;
+import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
+import org.apache.ratis.thirdparty.io.netty.buffer.ByteBuf;
+import org.apache.ratis.thirdparty.io.netty.buffer.Unpooled;
 
 import java.io.File;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.Deque;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.function.Function;
+
+import org.apache.hadoop.hdds.scm.storage.BlockDataStreamOutput;
 
 /**
  * This class is used to get the DataChannel for streaming.
  */
-class KeyValueStreamDataChannel extends StreamDataChannelBase {
+public class KeyValueStreamDataChannel extends StreamDataChannelBase {
+  private static final ByteBuffer[] EMPTY = {};
+  private static final Function<ByteBuffer, ByteString> CONVERTER
+      // TODO: pass OzoneConfigKeys.OZONE_UNSAFEBYTEOPERATIONS_ENABLED
+      = ByteStringConversion.createByteBufferConversion(false);
+
+  /**
+   * Keep the last {@link BlockDataStreamOutput#PUT_BLOCK_REQUEST_LENGTH_MAX}
+   * bytes in the buffer in order to create putBlockRequest at {@link #close()}.
+   */
+  private static class Buffers {
+    private final Deque<ByteBuffer> deque = new LinkedList<>();
+    private int length;
+
+    private boolean isExtra(int n) {
+      return length - n >= BlockDataStreamOutput.PUT_BLOCK_REQUEST_LENGTH_MAX;

Review comment:
       Hi @szetszwo , what is the purpose of being greater than 1M?
   
   In the test, writing file size is 128MB, the actual stream's offset in the log was 135272596.
   135272596 - (128*1024*1024) = 6292, which is the same as the size of the trespass recorded in logs.




-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   @captainzmc , could you post the test code?


-- 
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] guohao-rosicky commented on pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on pull request #3195:
URL: https://github.com/apache/ozone/pull/3195#issuecomment-1072251468


   > @captainzmc , let try buffer copying; see https://issues.apache.org/jira/secure/attachment/13041304/3195_buffer_copying.patch
   
   Thanks @szetszwo,  I have a try
   


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/KeyValueStreamDataChannel.java
##########
@@ -18,17 +18,90 @@
 
 package org.apache.hadoop.ozone.container.keyvalue.impl;
 
+import com.google.common.base.Preconditions;
 import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos;
+import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProto;
+import org.apache.hadoop.hdds.ratis.ContainerCommandRequestMessage;
+import org.apache.hadoop.hdds.scm.ByteStringConversion;
 import org.apache.hadoop.hdds.scm.container.common.helpers.StorageContainerException;
 import org.apache.hadoop.ozone.container.common.helpers.ContainerMetrics;
 import org.apache.hadoop.ozone.container.common.impl.ContainerData;
+import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
+import org.apache.ratis.thirdparty.io.netty.buffer.ByteBuf;
+import org.apache.ratis.thirdparty.io.netty.buffer.Unpooled;
 
 import java.io.File;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.Deque;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.function.Function;
+
+import org.apache.hadoop.hdds.scm.storage.BlockDataStreamOutput;
 
 /**
  * This class is used to get the DataChannel for streaming.
  */
-class KeyValueStreamDataChannel extends StreamDataChannelBase {
+public class KeyValueStreamDataChannel extends StreamDataChannelBase {
+  private static final ByteBuffer[] EMPTY = {};
+  private static final Function<ByteBuffer, ByteString> CONVERTER
+      // TODO: pass OzoneConfigKeys.OZONE_UNSAFEBYTEOPERATIONS_ENABLED
+      = ByteStringConversion.createByteBufferConversion(false);
+
+  /**
+   * Keep the last {@link BlockDataStreamOutput#PUT_BLOCK_REQUEST_LENGTH_MAX}
+   * bytes in the buffer in order to create putBlockRequest at {@link #close()}.
+   */
+  private static class Buffers {
+    private final Deque<ByteBuffer> deque = new LinkedList<>();
+    private int length;
+
+    private boolean isExtra(int n) {
+      return length - n >= BlockDataStreamOutput.PUT_BLOCK_REQUEST_LENGTH_MAX;

Review comment:
       Hi @szetszwo , what is the purpose of being greater than 1M?

##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/KeyValueStreamDataChannel.java
##########
@@ -18,17 +18,90 @@
 
 package org.apache.hadoop.ozone.container.keyvalue.impl;
 
+import com.google.common.base.Preconditions;
 import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos;
+import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProto;
+import org.apache.hadoop.hdds.ratis.ContainerCommandRequestMessage;
+import org.apache.hadoop.hdds.scm.ByteStringConversion;
 import org.apache.hadoop.hdds.scm.container.common.helpers.StorageContainerException;
 import org.apache.hadoop.ozone.container.common.helpers.ContainerMetrics;
 import org.apache.hadoop.ozone.container.common.impl.ContainerData;
+import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
+import org.apache.ratis.thirdparty.io.netty.buffer.ByteBuf;
+import org.apache.ratis.thirdparty.io.netty.buffer.Unpooled;
 
 import java.io.File;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.Deque;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.function.Function;
+
+import org.apache.hadoop.hdds.scm.storage.BlockDataStreamOutput;
 
 /**
  * This class is used to get the DataChannel for streaming.
  */
-class KeyValueStreamDataChannel extends StreamDataChannelBase {
+public class KeyValueStreamDataChannel extends StreamDataChannelBase {
+  private static final ByteBuffer[] EMPTY = {};
+  private static final Function<ByteBuffer, ByteString> CONVERTER
+      // TODO: pass OzoneConfigKeys.OZONE_UNSAFEBYTEOPERATIONS_ENABLED
+      = ByteStringConversion.createByteBufferConversion(false);
+
+  /**
+   * Keep the last {@link BlockDataStreamOutput#PUT_BLOCK_REQUEST_LENGTH_MAX}
+   * bytes in the buffer in order to create putBlockRequest at {@link #close()}.
+   */
+  private static class Buffers {
+    private final Deque<ByteBuffer> deque = new LinkedList<>();
+    private int length;
+
+    private boolean isExtra(int n) {
+      return length - n >= BlockDataStreamOutput.PUT_BLOCK_REQUEST_LENGTH_MAX;

Review comment:
       Hi @szetszwo , what is the purpose of being greater than 1M?
   
   In the test, writing file size is 128MB, the actual stream's offset in the log was 135272596.
   135272596 - (128*1024*1024) = 6292, which is the same as the size of the trespass recorded in logs.

##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/KeyValueStreamDataChannel.java
##########
@@ -18,17 +18,90 @@
 
 package org.apache.hadoop.ozone.container.keyvalue.impl;
 
+import com.google.common.base.Preconditions;
 import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos;
+import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProto;
+import org.apache.hadoop.hdds.ratis.ContainerCommandRequestMessage;
+import org.apache.hadoop.hdds.scm.ByteStringConversion;
 import org.apache.hadoop.hdds.scm.container.common.helpers.StorageContainerException;
 import org.apache.hadoop.ozone.container.common.helpers.ContainerMetrics;
 import org.apache.hadoop.ozone.container.common.impl.ContainerData;
+import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
+import org.apache.ratis.thirdparty.io.netty.buffer.ByteBuf;
+import org.apache.ratis.thirdparty.io.netty.buffer.Unpooled;
 
 import java.io.File;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.Deque;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.function.Function;
+
+import org.apache.hadoop.hdds.scm.storage.BlockDataStreamOutput;
 
 /**
  * This class is used to get the DataChannel for streaming.
  */
-class KeyValueStreamDataChannel extends StreamDataChannelBase {
+public class KeyValueStreamDataChannel extends StreamDataChannelBase {
+  private static final ByteBuffer[] EMPTY = {};
+  private static final Function<ByteBuffer, ByteString> CONVERTER
+      // TODO: pass OzoneConfigKeys.OZONE_UNSAFEBYTEOPERATIONS_ENABLED
+      = ByteStringConversion.createByteBufferConversion(false);
+
+  /**
+   * Keep the last {@link BlockDataStreamOutput#PUT_BLOCK_REQUEST_LENGTH_MAX}
+   * bytes in the buffer in order to create putBlockRequest at {@link #close()}.
+   */
+  private static class Buffers {
+    private final Deque<ByteBuffer> deque = new LinkedList<>();
+    private int length;
+
+    private boolean isExtra(int n) {
+      return length - n >= BlockDataStreamOutput.PUT_BLOCK_REQUEST_LENGTH_MAX;

Review comment:
       Hi @szetszwo , what is the purpose of being greater than 1M?
   
   In the test, writing file size is 128MB, the actual stream's offset in the log was 135272596.
   135272596 - (128 \* 1024 \* 1024) = 6292, which is the same as the size of the trespass recorded in logs.

##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/KeyValueStreamDataChannel.java
##########
@@ -18,17 +18,90 @@
 
 package org.apache.hadoop.ozone.container.keyvalue.impl;
 
+import com.google.common.base.Preconditions;
 import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos;
+import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProto;
+import org.apache.hadoop.hdds.ratis.ContainerCommandRequestMessage;
+import org.apache.hadoop.hdds.scm.ByteStringConversion;
 import org.apache.hadoop.hdds.scm.container.common.helpers.StorageContainerException;
 import org.apache.hadoop.ozone.container.common.helpers.ContainerMetrics;
 import org.apache.hadoop.ozone.container.common.impl.ContainerData;
+import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
+import org.apache.ratis.thirdparty.io.netty.buffer.ByteBuf;
+import org.apache.ratis.thirdparty.io.netty.buffer.Unpooled;
 
 import java.io.File;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.Deque;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.function.Function;
+
+import org.apache.hadoop.hdds.scm.storage.BlockDataStreamOutput;
 
 /**
  * This class is used to get the DataChannel for streaming.
  */
-class KeyValueStreamDataChannel extends StreamDataChannelBase {
+public class KeyValueStreamDataChannel extends StreamDataChannelBase {
+  private static final ByteBuffer[] EMPTY = {};
+  private static final Function<ByteBuffer, ByteString> CONVERTER
+      // TODO: pass OzoneConfigKeys.OZONE_UNSAFEBYTEOPERATIONS_ENABLED
+      = ByteStringConversion.createByteBufferConversion(false);
+
+  /**
+   * Keep the last {@link BlockDataStreamOutput#PUT_BLOCK_REQUEST_LENGTH_MAX}
+   * bytes in the buffer in order to create putBlockRequest at {@link #close()}.
+   */
+  private static class Buffers {
+    private final Deque<ByteBuffer> deque = new LinkedList<>();
+    private int length;
+
+    private boolean isExtra(int n) {
+      return length - n >= BlockDataStreamOutput.PUT_BLOCK_REQUEST_LENGTH_MAX;

Review comment:
       Hi @szetszwo , what is the purpose of being greater than 1M?
   
   In the test, writing file size is 128MB, the actual stream's offset in the log was 135272596.
   135272596 - (128 \* 1024 \* 1024) = 6292, which is the same as the size of the trespass recorded in logs.

##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/KeyValueStreamDataChannel.java
##########
@@ -18,17 +18,100 @@
 
 package org.apache.hadoop.ozone.container.keyvalue.impl;
 
+import com.google.common.base.Preconditions;
 import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos;
+import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProto;
+import org.apache.hadoop.hdds.ratis.ContainerCommandRequestMessage;
+import org.apache.hadoop.hdds.scm.ByteStringConversion;
 import org.apache.hadoop.hdds.scm.container.common.helpers.StorageContainerException;
 import org.apache.hadoop.ozone.container.common.helpers.ContainerMetrics;
 import org.apache.hadoop.ozone.container.common.impl.ContainerData;
+import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
+import org.apache.ratis.thirdparty.io.netty.buffer.ByteBuf;
+import org.apache.ratis.thirdparty.io.netty.buffer.Unpooled;
 
 import java.io.File;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.Deque;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.function.Function;
+
+import org.apache.hadoop.hdds.scm.storage.BlockDataStreamOutput;
 
 /**
  * This class is used to get the DataChannel for streaming.
  */
-class KeyValueStreamDataChannel extends StreamDataChannelBase {
+public class KeyValueStreamDataChannel extends StreamDataChannelBase {
+  private static final ByteBuffer[] EMPTY = {};
+  private static final Function<ByteBuffer, ByteString> CONVERTER
+      // TODO: pass OzoneConfigKeys.OZONE_UNSAFEBYTEOPERATIONS_ENABLED
+      = ByteStringConversion.createByteBufferConversion(false);
+
+  /**
+   * Keep the last {@link Buffers#max} bytes in the buffer
+   * in order to create putBlockRequest
+   * at {@link #closeBuffers(Buffers, WriteMethod)}}.
+   */
+  static class Buffers {
+    private final Deque<ByteBuffer> deque = new LinkedList<>();
+    private final int max;
+    private int length;
+
+    Buffers(int max) {
+      this.max = max;
+    }
+
+    private boolean isExtra(int n) {
+      return length - n >= max;

Review comment:
       Hi @szetszwo , what is the purpose of being greater than 1M?
   
   In the test, writing file size is 128MB, the actual stream's offset in the log was 135272596.
   135272596 - (129 \* 1024 \* 1024) = 6292, which is the same as the size of the trespass recorded in logs.




-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   > @captainzmc , I suspect the synchronization problem is in Ratis. Could you try apply below to Ratis and test it?
   
   @szetszwo I just update ratis and retest, and still get same error.  
   
   


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   @captainzmc , let try buffer copying; see https://issues.apache.org/jira/secure/attachment/13041304/3195_buffer_copying.patch


-- 
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] guohao-rosicky commented on pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on pull request #3195:
URL: https://github.com/apache/ozone/pull/3195#issuecomment-1077324524


   > @guohao-rosicky , since you have done some initial code refactoring (the common code here and #2860 , could you push it in this pull request so that we can merge it?
   > 
   > Then, I will file another JIRA for the putBlockRequest serialization and the buffering in KeyValueStreamDataChannel.
   
   Ok, I have been in the https://github.com/apache/ozone/pull/2860 in the abstract


-- 
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] guohao-rosicky commented on a change in pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on a change in pull request #3195:
URL: https://github.com/apache/ozone/pull/3195#discussion_r834010609



##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockDataStreamOutput.java
##########
@@ -438,12 +439,39 @@ private void executePutBlock(boolean close,
       byteBufferList = null;
     }
     waitFuturesComplete();
+    BlockData blockData = containerBlockData.build();
     if (close) {
-      dataStreamCloseReply = out.closeAsync();
+      out.writeAsync(ByteBuffer.allocateDirect(0).asReadOnlyBuffer())

Review comment:
       > 
   
   Hi, @szetszwo 
   I have a question, about 
   ```
   For example, the sender may write 3 times with lengths 3, 4, 5. Since there could be buffers/encoder/decoder in between, the receiver may receive a single buffer with length 12.
   ```
   I noticed that in HDFS, marking the end of stream is also represented by an empty package.
   In Ratis Stream, sticky packets have already been dealt with.
   
   org.apache.ratis.netty.NettyDataStreamUtils#encodeDataStreamRequestHeader
   https://github.com/apache/ratis/blob/master/ratis-netty/src/main/java/org/apache/ratis/netty/NettyDataStreamUtils.java#L90
   
   @captainzmc  tested the code from some time ago and did not find this problem




-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   > I used the new Ratis Patch to test again
   
   @guohao-rosicky , this still needs more work.  Would you mind if I take over this?


-- 
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] guohao-rosicky commented on a change in pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on a change in pull request #3195:
URL: https://github.com/apache/ozone/pull/3195#discussion_r834010609



##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockDataStreamOutput.java
##########
@@ -438,12 +439,39 @@ private void executePutBlock(boolean close,
       byteBufferList = null;
     }
     waitFuturesComplete();
+    BlockData blockData = containerBlockData.build();
     if (close) {
-      dataStreamCloseReply = out.closeAsync();
+      out.writeAsync(ByteBuffer.allocateDirect(0).asReadOnlyBuffer())

Review comment:
       > 
   
   Hi, @szetszwo 
   I have a question, about 
   ```
   For example, the sender may write 3 times with lengths 3, 4, 5. Since there could be buffers/encoder/decoder in between, the receiver may receive a single buffer with length 12.
   ```
   I noticed that in HDFS, marking the end of stream is also represented by an empty package.
   In Ratis Stream, sticky packets have already been dealt with.
   
   org.apache.ratis.netty.NettyDataStreamUtils#encodeDataStreamRequestHeader
   https://github.com/apache/ratis/blob/master/ratis-netty/src/main/java/org/apache/ratis/netty/NettyDataStreamUtils.java#L90
   




-- 
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] guohao-rosicky commented on pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on pull request #3195:
URL: https://github.com/apache/ozone/pull/3195#issuecomment-1078933927


   Has been modified. @szetszwo Please take a look.Thanks.


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   @guohao-rosicky , thanks a lot for testing it.  Does it fix the bug?


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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






-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/KeyValueStreamDataChannel.java
##########
@@ -18,17 +18,100 @@
 
 package org.apache.hadoop.ozone.container.keyvalue.impl;
 
+import com.google.common.base.Preconditions;
 import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos;
+import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProto;
+import org.apache.hadoop.hdds.ratis.ContainerCommandRequestMessage;
+import org.apache.hadoop.hdds.scm.ByteStringConversion;
 import org.apache.hadoop.hdds.scm.container.common.helpers.StorageContainerException;
 import org.apache.hadoop.ozone.container.common.helpers.ContainerMetrics;
 import org.apache.hadoop.ozone.container.common.impl.ContainerData;
+import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
+import org.apache.ratis.thirdparty.io.netty.buffer.ByteBuf;
+import org.apache.ratis.thirdparty.io.netty.buffer.Unpooled;
 
 import java.io.File;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.Deque;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.function.Function;
+
+import org.apache.hadoop.hdds.scm.storage.BlockDataStreamOutput;
 
 /**
  * This class is used to get the DataChannel for streaming.
  */
-class KeyValueStreamDataChannel extends StreamDataChannelBase {
+public class KeyValueStreamDataChannel extends StreamDataChannelBase {
+  private static final ByteBuffer[] EMPTY = {};
+  private static final Function<ByteBuffer, ByteString> CONVERTER
+      // TODO: pass OzoneConfigKeys.OZONE_UNSAFEBYTEOPERATIONS_ENABLED
+      = ByteStringConversion.createByteBufferConversion(false);
+
+  /**
+   * Keep the last {@link Buffers#max} bytes in the buffer
+   * in order to create putBlockRequest
+   * at {@link #closeBuffers(Buffers, WriteMethod)}}.
+   */
+  static class Buffers {
+    private final Deque<ByteBuffer> deque = new LinkedList<>();
+    private final int max;
+    private int length;
+
+    Buffers(int max) {
+      this.max = max;
+    }
+
+    private boolean isExtra(int n) {
+      return length - n >= max;

Review comment:
       > ... what is the purpose of being greater than 1M?
   
   We write putBlockRequestProto at the end of the stream but the putBlockRequestProto should not be written to the file.
   The last 1M bytes is kept in the buffer so that we can extract the putBlockRequestProto when the channel is closed.
   




-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   @captainzmc , thanks a lot for testing it.  Let's try adding `synchronized`; see https://issues.apache.org/jira/secure/attachment/13041294/3195_synchronized.patch
   
   If this still fails, I will try reproducing 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 commented on pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   @szetszwo  Here is my datanode log. https://issues.apache.org/jira/secure/attachment/13041276/ozone-root-datanode-9-29-173-57.log


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   > @szetszwo Here is my datanode log. https://issues.apache.org/jira/secure/attachment/13041276/ozone-root-datanode-9-29-173-57.log
   
   


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/KeyValueStreamDataChannel.java
##########
@@ -18,17 +18,90 @@
 
 package org.apache.hadoop.ozone.container.keyvalue.impl;
 
+import com.google.common.base.Preconditions;
 import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos;
+import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProto;
+import org.apache.hadoop.hdds.ratis.ContainerCommandRequestMessage;
+import org.apache.hadoop.hdds.scm.ByteStringConversion;
 import org.apache.hadoop.hdds.scm.container.common.helpers.StorageContainerException;
 import org.apache.hadoop.ozone.container.common.helpers.ContainerMetrics;
 import org.apache.hadoop.ozone.container.common.impl.ContainerData;
+import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
+import org.apache.ratis.thirdparty.io.netty.buffer.ByteBuf;
+import org.apache.ratis.thirdparty.io.netty.buffer.Unpooled;
 
 import java.io.File;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.Deque;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.function.Function;
+
+import org.apache.hadoop.hdds.scm.storage.BlockDataStreamOutput;
 
 /**
  * This class is used to get the DataChannel for streaming.
  */
-class KeyValueStreamDataChannel extends StreamDataChannelBase {
+public class KeyValueStreamDataChannel extends StreamDataChannelBase {
+  private static final ByteBuffer[] EMPTY = {};
+  private static final Function<ByteBuffer, ByteString> CONVERTER
+      // TODO: pass OzoneConfigKeys.OZONE_UNSAFEBYTEOPERATIONS_ENABLED
+      = ByteStringConversion.createByteBufferConversion(false);
+
+  /**
+   * Keep the last {@link BlockDataStreamOutput#PUT_BLOCK_REQUEST_LENGTH_MAX}
+   * bytes in the buffer in order to create putBlockRequest at {@link #close()}.
+   */
+  private static class Buffers {
+    private final Deque<ByteBuffer> deque = new LinkedList<>();
+    private int length;
+
+    private boolean isExtra(int n) {
+      return length - n >= BlockDataStreamOutput.PUT_BLOCK_REQUEST_LENGTH_MAX;

Review comment:
       Hi @szetszwo , what is the purpose of being greater than 1M?




-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   @captainzmc , please test https://issues.apache.org/jira/secure/attachment/13041289/3195_bugfix2.patch .  Thanks!


-- 
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] guohao-rosicky commented on pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on pull request #3195:
URL: https://github.com/apache/ozone/pull/3195#issuecomment-1074856821


   > > I used the new Ratis Patch to test again
   > 
   > @guohao-rosicky , this still needs more work. Would you mind if I take over this?
   
   @szetszwo  Ok, please continue the work.
   


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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



##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/KeyDataStreamOutput.java
##########
@@ -142,6 +113,10 @@ public KeyDataStreamOutput(
       String uploadID, int partNumber, boolean isMultipart,
       boolean unsafeByteBufferConversion
   ) {
+    super(HddsClientUtils.getExceptionList()
+        .stream()
+        .collect(Collectors.toMap(Function.identity(),
+            e -> RetryPolicies.TRY_ONCE_THEN_FAIL)));

Review comment:
       The map is passed incorrectly.  Below is the original map.
   ```
   -    this.retryPolicyMap = HddsClientUtils.getRetryPolicyByException(
   -        config.getMaxRetryCount(), config.getRetryInterval());
   ```
   




-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   > @captainzmc , @guohao-rosicky , just found a bug in BlockDataStreamOutput. Could you test https://issues.apache.org/jira/secure/attachment/13041279/3195_bugfix.patch to see if it can fix the bug?
   
   Thanks @szetszwo for update this. I had just test this patch, we still get same error. [Just as the same as this.](https://issues.apache.org/jira/secure/attachment/13041276/ozone-root-datanode-9-29-173-57.log)


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/KeyValueStreamDataChannel.java
##########
@@ -18,17 +18,100 @@
 
 package org.apache.hadoop.ozone.container.keyvalue.impl;
 
+import com.google.common.base.Preconditions;
 import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos;
+import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProto;
+import org.apache.hadoop.hdds.ratis.ContainerCommandRequestMessage;
+import org.apache.hadoop.hdds.scm.ByteStringConversion;
 import org.apache.hadoop.hdds.scm.container.common.helpers.StorageContainerException;
 import org.apache.hadoop.ozone.container.common.helpers.ContainerMetrics;
 import org.apache.hadoop.ozone.container.common.impl.ContainerData;
+import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
+import org.apache.ratis.thirdparty.io.netty.buffer.ByteBuf;
+import org.apache.ratis.thirdparty.io.netty.buffer.Unpooled;
 
 import java.io.File;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.Deque;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.function.Function;
+
+import org.apache.hadoop.hdds.scm.storage.BlockDataStreamOutput;
 
 /**
  * This class is used to get the DataChannel for streaming.
  */
-class KeyValueStreamDataChannel extends StreamDataChannelBase {
+public class KeyValueStreamDataChannel extends StreamDataChannelBase {
+  private static final ByteBuffer[] EMPTY = {};
+  private static final Function<ByteBuffer, ByteString> CONVERTER
+      // TODO: pass OzoneConfigKeys.OZONE_UNSAFEBYTEOPERATIONS_ENABLED
+      = ByteStringConversion.createByteBufferConversion(false);
+
+  /**
+   * Keep the last {@link Buffers#max} bytes in the buffer
+   * in order to create putBlockRequest
+   * at {@link #closeBuffers(Buffers, WriteMethod)}}.
+   */
+  static class Buffers {
+    private final Deque<ByteBuffer> deque = new LinkedList<>();
+    private final int max;
+    private int length;
+
+    Buffers(int max) {
+      this.max = max;
+    }
+
+    private boolean isExtra(int n) {
+      return length - n >= max;

Review comment:
       Hi @szetszwo , what is the purpose of being greater than 1M?
   
   In the test, writing file size is 128MB, the actual stream's offset in the log was 135272596.
   135272596 - (129 \* 1024 \* 1024) = 6292, which is the same as the size of the trespass recorded in logs.




-- 
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 removed a comment on pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

Posted by GitBox <gi...@apache.org>.
szetszwo removed a comment on pull request #3195:
URL: https://github.com/apache/ozone/pull/3195#issuecomment-1072104649


   @captainzmc , @guohao-rosicky , I am able to reproduce the problem.  I believe the bug is because Netty reuses the buffers.  Since we keep the buffer after write and Ratis will release it, the content of the buffer may change when Netty reuses 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 commented on pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


    > @captainzmc , please test https://issues.apache.org/jira/secure/attachment/13041289/3195_bugfix2.patch . Thanks!
   
   Thanks @szetszwo, I just test this patch. New patch besides IndexOutOfBoundsException there will be some "Bad address IOException". [See the log here.](https://issues.apache.org/jira/secure/attachment/13041292/ozone-root-datanode-9-29-173-57.log2.log).
   Do we need to add some logs to debug the cause of the bug?


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   @captainzmc , thanks again for check it.  If it works, we can change Ratis to support reference-counted buffer to avoid buffer-copying.  Let me file a Ratis JIRA.


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   @captainzmc , thanks.  Let me test the serialization/deserialization code.


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   @szetszwo  Here is my datanode log. https://issues.apache.org/jira/secure/attachment/13041276/ozone-root-datanode-9-29-173-57.log
   In the test, I wrote 30 files concurrently. Each file is 128MB and the chunk size is 1MB


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   @captainzmc , @guohao-rosicky , just found a bug in BlockDataStreamOutput.  Could you test https://issues.apache.org/jira/secure/attachment/13041279/3195_bugfix.patch to see if it can fix the bug?


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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



##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockDataStreamOutput.java
##########
@@ -438,12 +439,39 @@ private void executePutBlock(boolean close,
       byteBufferList = null;
     }
     waitFuturesComplete();
+    BlockData blockData = containerBlockData.build();
     if (close) {
-      dataStreamCloseReply = out.closeAsync();
+      out.writeAsync(ByteBuffer.allocateDirect(0).asReadOnlyBuffer())

Review comment:
       Writing an empty buffer may not work as expected.  In the streaming model, data is continuous but not block-by-block.  Both sender and receiver must consider that the data is a stream of bytes.
   
   For example, the sender may write 3 times with lengths 3, 4, 5.  Since there could be buffers/encoder/decoder in between, the receiver may receive a single buffer with length 12.  




-- 
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] guohao-rosicky commented on pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on pull request #3195:
URL: https://github.com/apache/ozone/pull/3195#issuecomment-1077493971


   
   > Of course, Ozone may choose to add a header for each call but it will add more overhead and need more code changes.
   
   @szetszwo 
   In Ratis Stream, sticky packets have already been dealt with.
   
   org.apache.ratis.netty.NettyDataStreamUtils#encodeDataStreamRequestHeader
   https://github.com/apache/ratis/blob/master/ratis-netty/src/main/java/org/apache/ratis/netty/NettyDataStreamUtils.java#L90


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
##########
@@ -179,13 +179,25 @@ public VolumeChoosingPolicy getVolumeChoosingPolicyForTesting() {
 
   @Override
   public StateMachine.DataChannel getStreamDataChannel(
-          Container container, ContainerCommandRequestProto msg)
-          throws StorageContainerException {
+      Container container, ContainerCommandRequestProto msg)
+      throws StorageContainerException {
     KeyValueContainer kvContainer = (KeyValueContainer) container;
     checkContainerOpen(kvContainer);
-    BlockID blockID = BlockID.getFromProtobuf(msg.getWriteChunk().getBlockID());
-    return chunkManager.getStreamDataChannel(kvContainer,
-            blockID, metrics);
+
+    if (msg.hasWriteChunk()) {
+      BlockID blockID =
+          BlockID.getFromProtobuf(msg.getWriteChunk().getBlockID());
+
+      return chunkManager.getStreamDataChannel(kvContainer,
+          blockID, metrics);
+    } else {
+      LOG.error(
+          "Get stream data channel error Malformed request " +
+              "containerID: {} msg: {}",
+          container.getContainerData().getContainerID(), msg);

Review comment:
       Let's remove LOG.error(..) since it already throws an exception.

##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/KeyDataStreamOutput.java
##########
@@ -158,10 +139,6 @@ public KeyDataStreamOutput(
     // Retrieve the file encryption key info, null if file is not in
     // encrypted bucket.
     this.feInfo = info.getFileEncryptionInfo();
-    this.retryPolicyMap = HddsClientUtils.getRetryPolicyByException(
-        config.getMaxRetryCount(), config.getRetryInterval());

Review comment:
       We should pass the retryPolicyMap to the super constructor.

##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/ContainerStateMachine.java
##########
@@ -549,18 +550,42 @@ private ContainerCommandResponseProto runCommand(
 
   @Override
   public CompletableFuture<?> link(DataStream stream, LogEntryProto entry) {
-    return CompletableFuture.supplyAsync(() -> {
-      if (stream == null) {
-        return JavaUtils.completeExceptionally(
-            new IllegalStateException("DataStream is null"));
-      }
-      if (stream.getDataChannel().isOpen()) {
-        return JavaUtils.completeExceptionally(
-            new IllegalStateException(
-                "DataStream: " + stream + " is not closed properly"));
-      } else {
-        return CompletableFuture.completedFuture(null);
+    if (stream == null) {
+      return JavaUtils.completeExceptionally(new IllegalStateException(
+          "DataStream is null"));
+    }
+    final DataChannel dataChannel = stream.getDataChannel();
+    if (dataChannel.isOpen()) {
+      return JavaUtils.completeExceptionally(new IllegalStateException(
+          "DataStream: " + stream + " is not closed properly"));
+    }
+
+    final CompletableFuture<ContainerCommandResponseProto> f;
+    if (dataChannel instanceof KeyValueStreamDataChannel) {
+      f = CompletableFuture.completedFuture(null);
+    } else {
+      return JavaUtils.completeExceptionally(new IllegalStateException(
+          "Unexpected DataChannel " + dataChannel.getClass()));
+    }
+    return f.whenComplete((res, e) -> {
+      if (LOG.isDebugEnabled()) {
+        LOG.debug("PutBlock {} Term: {} Index: {}",
+            res.getResult(), entry.getTerm(), entry.getIndex());
       }
+    });
+  }
+
+  private CompletableFuture<ContainerCommandResponseProto> runCommandAsync(

Review comment:
       runCommandAsync is not used.  Let's add it later.

##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueHandler.java
##########
@@ -266,24 +278,23 @@ public BlockManager getBlockManager() {
   ContainerCommandResponseProto handleStreamInit(
       ContainerCommandRequestProto request, KeyValueContainer kvContainer,
       DispatcherContext dispatcherContext) {
-    if (!request.hasWriteChunk()) {
+    BlockID blockID;

Review comment:
       Add final.

##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/KeyDataStreamOutput.java
##########
@@ -97,12 +85,8 @@
    */
   @VisibleForTesting
   public KeyDataStreamOutput() {

Review comment:
       This method is not used anywhere.  Let's remove 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 closed pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   Hi @guohao-rosicky Can you paste the patch of the original version? I'd like to make sure if there is the same problem.


-- 
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] guohao-rosicky commented on pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on pull request #3195:
URL: https://github.com/apache/ozone/pull/3195#issuecomment-1072251468


   > @captainzmc , let try buffer copying; see https://issues.apache.org/jira/secure/attachment/13041304/3195_buffer_copying.patch
   
   Thanks @szetszwo,  I have a try
   


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   > @szetszwo yes here is my [test code](https://github.com/captainzmc/ozonerpc2/blob/master/src/main/java/org/apache/hadoop/ozone/examples/OzoneRpc.java). Is written according to the ratis Filestore test.
   
   Thanks for sharing the test code.  It looks good.
   


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/KeyValueStreamDataChannel.java
##########
@@ -18,17 +18,90 @@
 
 package org.apache.hadoop.ozone.container.keyvalue.impl;
 
+import com.google.common.base.Preconditions;
 import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos;
+import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProto;
+import org.apache.hadoop.hdds.ratis.ContainerCommandRequestMessage;
+import org.apache.hadoop.hdds.scm.ByteStringConversion;
 import org.apache.hadoop.hdds.scm.container.common.helpers.StorageContainerException;
 import org.apache.hadoop.ozone.container.common.helpers.ContainerMetrics;
 import org.apache.hadoop.ozone.container.common.impl.ContainerData;
+import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
+import org.apache.ratis.thirdparty.io.netty.buffer.ByteBuf;
+import org.apache.ratis.thirdparty.io.netty.buffer.Unpooled;
 
 import java.io.File;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.Deque;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.function.Function;
+
+import org.apache.hadoop.hdds.scm.storage.BlockDataStreamOutput;
 
 /**
  * This class is used to get the DataChannel for streaming.
  */
-class KeyValueStreamDataChannel extends StreamDataChannelBase {
+public class KeyValueStreamDataChannel extends StreamDataChannelBase {
+  private static final ByteBuffer[] EMPTY = {};
+  private static final Function<ByteBuffer, ByteString> CONVERTER
+      // TODO: pass OzoneConfigKeys.OZONE_UNSAFEBYTEOPERATIONS_ENABLED
+      = ByteStringConversion.createByteBufferConversion(false);
+
+  /**
+   * Keep the last {@link BlockDataStreamOutput#PUT_BLOCK_REQUEST_LENGTH_MAX}
+   * bytes in the buffer in order to create putBlockRequest at {@link #close()}.
+   */
+  private static class Buffers {
+    private final Deque<ByteBuffer> deque = new LinkedList<>();
+    private int length;
+
+    private boolean isExtra(int n) {
+      return length - n >= BlockDataStreamOutput.PUT_BLOCK_REQUEST_LENGTH_MAX;

Review comment:
       Hi @szetszwo , what is the purpose of being greater than 1M?
   
   In the test, writing file size is 128MB, the actual stream's offset in the log was 135272596.
   135272596 - (128 \* 1024 \* 1024) = 6292, which is the same as the size of the trespass recorded in logs.




-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


    > @captainzmc , please test https://issues.apache.org/jira/secure/attachment/13041289/3195_bugfix2.patch . Thanks!
   
   Thanks @szetszwo, I just test this patch. New patch besides IndexOutOfBoundsException there will be some "Bad address IOException". [See the log here.](https://issues.apache.org/jira/secure/attachment/13041292/ozone-root-datanode-9-29-173-57.log2.log).
   Do we need add some logs to debug the cause of the bug?


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   > @captainzmc , let try buffer copying; see https://issues.apache.org/jira/secure/attachment/13041304/3195_buffer_copying.patch
   
   Thanks @szetszwo ,I had test this patch, IndexOutOfBoundsException is no longer present in the test.  However, I found that after adding buffer copy, the memory of the DataNode becomes very large and datanode keeps reporting IllegalStateException. There are many timeouts in the client, and eventually the client will hung. [Here is log of datanode.](https://issues.apache.org/jira/secure/attachment/13041313/ozone-root-datanode-9-29-173-57.log4) I used 600 concurrent threads to write the file. 
   <img width="1167" alt="image" src="https://user-images.githubusercontent.com/13825159/158996837-c0487dae-9cb2-4436-a798-b20b57192661.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] szetszwo commented on pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   See https://issues.apache.org/jira/browse/RATIS-1556


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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






-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   > @captainzmc , @guohao-rosicky, Fixed the bug; see https://issues.apache.org/jira/secure/attachment/13041255/3195_TestStreamDataChannel.patch
   
   Thanks @szetszwo  for the update. Using new patch I found  IndexOutOfBoundsException still appear.  
   I found that the ERROR only appeared when writing with high concurrency. I was successful with 30 concurrent writes, but I get the exception with 600 concurrent writes.
   ![image](https://user-images.githubusercontent.com/13825159/158751660-57d9e0a0-8aa6-44bc-bd50-1f6a242b2ca9.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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   > @captainzmc , @guohao-rosicky, Fixed the bug; see https://issues.apache.org/jira/secure/attachment/13041255/3195_TestStreamDataChannel.patch
   
   Thanks @szetszwo  for the update. Using new patch I found  IndexOutOfBoundsException still appear.  
   I found that the above problem only appeared when writing with high concurrency. I was successful with 30 concurrent writes, but I get the above exception with 600 concurrent writes.
   ![image](https://user-images.githubusercontent.com/13825159/158751660-57d9e0a0-8aa6-44bc-bd50-1f6a242b2ca9.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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   > @captainzmc , @guohao-rosicky , just found a bug in BlockDataStreamOutput. Could you test https://issues.apache.org/jira/secure/attachment/13041279/3195_bugfix.patch to see if it can fix the bug?
   
   Thanks @szetszwo for update this.  I had just test this patch,  we still get same error. [Just as the same as this.](https://issues.apache.org/jira/secure/attachment/13041276/ozone-root-datanode-9-29-173-57.log)


-- 
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 removed a comment on pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   > @captainzmc , @guohao-rosicky , just found a bug in BlockDataStreamOutput. Could you test https://issues.apache.org/jira/secure/attachment/13041279/3195_bugfix.patch to see if it can fix the bug?
   
   Thanks @szetszwo for update this.  I had just test this patch,  we still get same error. [Just as the same as this.](https://issues.apache.org/jira/secure/attachment/13041276/ozone-root-datanode-9-29-173-57.log)


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   @captainzmc , @guohao-rosicky , I am able to reproduce the problem.  I believe the bug is because Netty reuses the buffers.  Since we keep the buffer after write and Ratis will release it, the content of the buffer may change when Netty reuses 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] guohao-rosicky commented on a change in pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on a change in pull request #3195:
URL: https://github.com/apache/ozone/pull/3195#discussion_r834010609



##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockDataStreamOutput.java
##########
@@ -438,12 +439,39 @@ private void executePutBlock(boolean close,
       byteBufferList = null;
     }
     waitFuturesComplete();
+    BlockData blockData = containerBlockData.build();
     if (close) {
-      dataStreamCloseReply = out.closeAsync();
+      out.writeAsync(ByteBuffer.allocateDirect(0).asReadOnlyBuffer())

Review comment:
       > 
   
   Hi, @szetszwo I have a question,
   I noticed that in HDFS, marking the end of stream is also represented by an empty package.
   In Ratis Stream, sticky packets have already been dealt with.
   
   org.apache.ratis.netty.NettyDataStreamUtils#encodeDataStreamRequestHeader
   https://github.com/apache/ratis/blob/master/ratis-netty/src/main/java/org/apache/ratis/netty/NettyDataStreamUtils.java#L90
   




-- 
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] guohao-rosicky commented on pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on pull request #3195:
URL: https://github.com/apache/ozone/pull/3195#issuecomment-1078707376


   > 
   
   Ok, I'll change 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] guohao-rosicky commented on a change in pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

Posted by GitBox <gi...@apache.org>.
guohao-rosicky commented on a change in pull request #3195:
URL: https://github.com/apache/ozone/pull/3195#discussion_r827723384



##########
File path: hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/storage/BlockDataStreamOutput.java
##########
@@ -438,12 +439,39 @@ private void executePutBlock(boolean close,
       byteBufferList = null;
     }
     waitFuturesComplete();
+    BlockData blockData = containerBlockData.build();
     if (close) {
-      dataStreamCloseReply = out.closeAsync();
+      out.writeAsync(ByteBuffer.allocateDirect(0).asReadOnlyBuffer())

Review comment:
       Thanks for your comment @szetszwo .   I wrote a piece of pseudo-code, Ideas about the modification of `org.apache.ratis.netty.server.DataStreamManagement#writeTo`.   
   
   ![image](https://user-images.githubusercontent.com/5122636/158544128-f60c7e94-efa3-412a-85ac-7304e284c241.png)
   
   What do you think of this idea?




-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   ```
   Caused by: java.lang.IndexOutOfBoundsException: End index: 16842756 >= 6292
   	at org.apache.ratis.thirdparty.com.google.protobuf.ByteString.checkRange(ByteString.java:1272)
   	at org.apache.ratis.thirdparty.com.google.protobuf.ByteString$LiteralByteString.substring(ByteString.java:1343)
   	at org.apache.hadoop.hdds.ratis.ContainerCommandRequestMessage.toProto(ContainerCommandRequestMessage.java:68)
   	at org.apache.hadoop.ozone.container.keyvalue.impl.KeyValueStreamDataChannel.readPutBlockRequest(KeyValueStreamDataChannel.java:201)
   ```
   @captainzmc , could you try removing line 201 as below?  That line actually is not need.
   ```
   +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/KeyValueStreamDataChannel.java
   @@ -187,7 +187,7 @@ static ContainerCommandRequestProto readPutBlockRequest(ByteBuf b)
        final int protoLength = readProtoLength(b.duplicate(), lengthIndex);
        final int protoIndex = lengthIndex - protoLength;
        final ContainerCommandRequestProto proto = readPutBlockRequest(
   -        b.duplicate(), protoIndex, lengthIndex);
   +        b.duplicate(), protoIndex);
    
        // set index for reading data
        b.writerIndex(protoIndex);
   @@ -196,9 +196,8 @@ static ContainerCommandRequestProto readPutBlockRequest(ByteBuf b)
      }
    
      private static ContainerCommandRequestProto readPutBlockRequest(
   -      ByteBuf b, int protoIndex, int lengthIndex) throws IOException {
   +      ByteBuf b, int protoIndex) throws IOException {
        b.readerIndex(protoIndex);
   -    b.writerIndex(lengthIndex);
        final ByteString byteString = CONVERTER.apply(b.nioBuffer());
    
        final ContainerCommandRequestProto request =
   ```


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   > Do we need add some logs to debug the cause of the bug?
   
   We already added the indices in the exception message as below.  The indices are totally wrong.  It seems there are some synchronization problem
   ```
   Caused by: java.io.IOException: Failed to readPutBlockRequest from ReadOnlyByteBuf(ridx: 0, widx: 1054872, cap: 1054872,
    unwrapped: CompositeByteBuf(ridx: 0, widx: 1054872, cap: 1054872, components=25)):
    readerIndex=0, protoIndex=0, protoLength=0, lengthIndex=1054868
   ```
   Please feel free to add more log messages for debugging.
   
   @captainzmc , I suspect the synchronization problem is in Ratis.  Could you try apply below to Ratis and test it?
   ```
   +++ b/ratis-netty/src/main/java/org/apache/ratis/netty/server/DataStreamManagement.java
   @@ -279,9 +279,7 @@ public class DataStreamManagement {
    
      static <T> CompletableFuture<T> composeAsync(AtomicReference<CompletableFuture<T>> future, Executor executor,
          Function<T, CompletableFuture<T>> function) {
   -    final CompletableFuture<T> composed = future.get().thenComposeAsync(function, executor);
   -    future.set(composed);
   -    return composed;
   +    return future.updateAndGet(previous -> previous.thenComposeAsync(function, executor));
      }
    
      static CompletableFuture<Long> writeToAsync(ByteBuf buf, WriteOption[] options, DataStream stream,
   ```
   


-- 
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 removed a comment on pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

Posted by GitBox <gi...@apache.org>.
szetszwo removed a comment on pull request #3195:
URL: https://github.com/apache/ozone/pull/3195#issuecomment-1072104649


   @captainzmc , @guohao-rosicky , I am able to reproduce the problem.  I believe the bug is because Netty reuses the buffers.  Since we keep the buffer after write and Ratis will release it, the content of the buffer may change when Netty reuses 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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   > @captainzmc , thanks a lot for testing it. Let's try adding `synchronized`; see https://issues.apache.org/jira/secure/attachment/13041294/3195_synchronized.patch
   > 
   > If this still fails, I will try reproducing it.
   
   Thanks @szetszwo, I just test this patch. New patch besides IndexOutOfBoundsException there will be some NPE. [See the log here.](https://issues.apache.org/jira/secure/attachment/13041299/ozone-root-datanode-9-29-173-57.log3.log).


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   > @captainzmc , thanks a lot for testing it. Let's try adding `synchronized`; see https://issues.apache.org/jira/secure/attachment/13041294/3195_synchronized.patch
   > 
   > If this still fails, I will try reproducing it.
   
   Thanks @szetszwo, I just test this patch. New patch besides IndexOutOfBoundsException there will be some NPE. [See the log here.](https://issues.apache.org/jira/secure/attachment/13041292/ozone-root-datanode-9-29-173-57.log3.log).


-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   Found the bug:  ByteBu.nioBuffers(index, length) somehow is not working.  Let me 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@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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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



##########
File path: hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/impl/KeyValueStreamDataChannel.java
##########
@@ -18,17 +18,90 @@
 
 package org.apache.hadoop.ozone.container.keyvalue.impl;
 
+import com.google.common.base.Preconditions;
 import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos;
+import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos.ContainerCommandRequestProto;
+import org.apache.hadoop.hdds.ratis.ContainerCommandRequestMessage;
+import org.apache.hadoop.hdds.scm.ByteStringConversion;
 import org.apache.hadoop.hdds.scm.container.common.helpers.StorageContainerException;
 import org.apache.hadoop.ozone.container.common.helpers.ContainerMetrics;
 import org.apache.hadoop.ozone.container.common.impl.ContainerData;
+import org.apache.ratis.thirdparty.com.google.protobuf.ByteString;
+import org.apache.ratis.thirdparty.io.netty.buffer.ByteBuf;
+import org.apache.ratis.thirdparty.io.netty.buffer.Unpooled;
 
 import java.io.File;
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.util.Deque;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.function.Function;
+
+import org.apache.hadoop.hdds.scm.storage.BlockDataStreamOutput;
 
 /**
  * This class is used to get the DataChannel for streaming.
  */
-class KeyValueStreamDataChannel extends StreamDataChannelBase {
+public class KeyValueStreamDataChannel extends StreamDataChannelBase {
+  private static final ByteBuffer[] EMPTY = {};
+  private static final Function<ByteBuffer, ByteString> CONVERTER
+      // TODO: pass OzoneConfigKeys.OZONE_UNSAFEBYTEOPERATIONS_ENABLED
+      = ByteStringConversion.createByteBufferConversion(false);
+
+  /**
+   * Keep the last {@link BlockDataStreamOutput#PUT_BLOCK_REQUEST_LENGTH_MAX}
+   * bytes in the buffer in order to create putBlockRequest at {@link #close()}.
+   */
+  private static class Buffers {
+    private final Deque<ByteBuffer> deque = new LinkedList<>();
+    private int length;
+
+    private boolean isExtra(int n) {
+      return length - n >= BlockDataStreamOutput.PUT_BLOCK_REQUEST_LENGTH_MAX;

Review comment:
       Hi @szetszwo , what is the purpose of being greater than 1M?
   
   In the test, writing file size is 128MB, the actual stream's offset in the log was 135272596.
   135272596 - (128 \* 1024 \* 1024) = 6292, which is the same as the size of the trespass recorded in logs.




-- 
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 #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   @guohao-rosicky , 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] szetszwo commented on pull request #3195: HDDS-6137. [Ozone-Streaming] Optimized the putBlock method of BlockDataStreamOutput

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


   > I noticed that in HDFS, marking the end of stream is also represented by an empty package.
   
   @guohao-rosicky , HDFS pipeline?  HDFS use TPC to implement packet streams. There is a header for each packet.  But Ratis exposes byte streams.
   
   Of course, Ozone may choose to add a header for each call but it will add more overhead and need more code changes.
   
   > Ok, I have been in the https://github.com/apache/ozone/pull/2860 in the abstract
   
   BTW, I have filed #3229 (HDDS-6500) for buffering the PutBlockRequest at the end of the stream.
   
   Once we have putBlockRequest added to the end of the stream, #2860 becomes much simpler since we can write everything in a single call and don't have to change the server code much.


-- 
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