[GitHub] [ozone] fapifta commented on a change in pull request #2849: HDDS-5955: EC: Track the failed servers to add into the excludeList when invoking allocateBlock

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

fapifta commented on a change in pull request #2849:
URL: https://github.com/apache/ozone/pull/2849#discussion_r754745133



##########
File path: hadoop-ozone/client/src/main/java/org/apache/hadoop/ozone/client/io/BlockOutputStreamEntry.java
##########
@@ -113,8 +113,13 @@ public void write(int b) throws IOException {
   @Override
   public void write(byte[] b, int off, int len) throws IOException {
     checkStream();
-    getOutputStream().write(b, off, len);
-    incCurrentPosition(len);
+    try {
+      getOutputStream().write(b, off, len);
+    }finally {
+      //Even though actual write failed we will move the position and when

Review comment:
       I am really not sure about this statement...
   The first things comes to my mind is: If so, then we should do similarly in the write(int) method, and possibly in writeOnRetry as well.
   
   A second point I am not sure about is that, resetToAckedDataLength is the method that sets back the position to the acked position. In KeyOutputStream it is called from handleException, but there are not other calls to the reset method and ECKeyOutputStream's handleException does not call the reset. What is the mechanism I don't see now, that resets the position in case of EC? As ECBlockOutputStreamEntry relies on these same write methods.




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