[GitHub] [ozone] umamaheswararao commented on a change in pull request #3124: HDDS-6348: EC: PartialStripe failure handling logic is writing padding bytes also to DNs

[GitBox <gi...@apache.org>]

umamaheswararao commented on a change in pull request #3124:
URL: https://github.com/apache/ozone/pull/3124#discussion_r812591677



##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/ECKeyOutputStream.java
##########
@@ -380,42 +380,49 @@ void writeParityCells(int parityCellSize) throws IOException {
   }
 
   private int handleDataWrite(int currIdx, byte[] b, int off, long len,
-      boolean isFullCell) throws IOException {
+      boolean isFullCell) {
     int pos = ecChunkBufferCache.addToDataBuffer(currIdx, b, off, (int) len);
-    handleOutputStreamWrite(currIdx, len, isFullCell, false);
 
     if (pos == ecChunkSize) {
+      handleOutputStreamWrite(currIdx, pos, isFullCell, false);
       blockOutputStreamEntryPool.getCurrentStreamEntry().useNextBlockStream();
     }
     return pos;
   }
 
-  private void handleParityWrite(int currIdx, long len, boolean isFullCell) {
+  private void handleParityWrite(int currIdx, int len, boolean isFullCell) {
     handleOutputStreamWrite(currIdx, len, isFullCell, true);
     blockOutputStreamEntryPool.getCurrentStreamEntry().useNextBlockStream();
   }
 
-  private void handleOutputStreamWrite(int currIdx, long len,
+  private void handleOutputStreamWrite(int currIdx, int len,
       boolean isFullCell, boolean isParity) {
 
-    BlockOutputStreamEntry current =
+    ECBlockOutputStreamEntry current =
         blockOutputStreamEntryPool.getCurrentStreamEntry();
 
     if (isFullCell) {
       ByteBuffer bytesToWrite = isParity ?
           ecChunkBufferCache.getParityBuffers()[currIdx - numDataBlks] :
           ecChunkBufferCache.getDataBuffers()[currIdx];
       try {
-        // Since it's a fullcell, let's write all content from buffer.
-        writeToOutputStream(current, len, bytesToWrite.array(),
-            bytesToWrite.limit(), 0, isParity);
+        // Since it's a full cell, let's write all content from buffer.
+        // At a time we write max cell size in EC. So, it should safe to cast
+        // the len to int to use the super class defined write API.
+        // The len cannot be bigger than cell buffer size.
+        assert len <= ecChunkSize : " The len: " + len + ". EC chunk size: "
+            + ecChunkSize;
+        assert len <= bytesToWrite
+            .limit() : " The len: " + len + ". Chunk buffer limit: "
+            + bytesToWrite.limit();
+        writeToOutputStream(current, bytesToWrite.array(), len, 0, isParity);

Review comment:
       @guihecheng Thanks for checking the patch. Yes, I think we can cleanup few things. I cleaned to remove redundant isFullCell 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