HDFS-9133. ExternalBlockReader and ReplicaAccessor need to return -1 on read when at EOF. (Colin Patrick McCabe via Lei (Eddy) Xu)

(cherry picked from commit 67b0e967f0e13eb6bed123fc7ba4cce0dcca198f)

hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ExternalBlockReader.java

@@ -45,6 +45,9 @@ public final class ExternalBlockReader implements BlockReader {
   @Override
   public int read(byte[] buf, int off, int len) throws IOException {
     int nread = accessor.read(pos, buf, off, len);
+    if (nread < 0) {
+      return nread;
+    }
     pos += nread;
     return nread;
   }
@@ -52,6 +55,9 @@ public final class ExternalBlockReader implements BlockReader {
   @Override
   public int read(ByteBuffer buf) throws IOException {
     int nread = accessor.read(pos, buf);
+    if (nread < 0) {
+      return nread;
+    }
     pos += nread;
     return nread;
   }

hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessor.java

@@ -40,8 +40,9 @@ public abstract class ReplicaAccessor {
    *
    * @return       The number of bytes read.  If the read extends past the end
    *                  of the replica, a short read count will be returned.  We
-   *                  will never return a negative number.  We will never
-   *                  return a short read count unless EOF is reached.
+   *                  will should return -1 if EOF is reached and no bytes
+   *                  can be returned.  We will never return a short read
+   *                  count unless EOF is reached.
    */
   public abstract int read(long pos, byte[] buf, int off, int len)
       throws IOException;
@@ -58,8 +59,9 @@ public abstract class ReplicaAccessor {
    *
    * @return       The number of bytes read.  If the read extends past the end
    *                 of the replica, a short read count will be returned.  We
-   *                 will never return a negative number.  We will never return
-   *                 a short read count unless EOF is reached.
+   *                 should return -1 if EOF is reached and no bytes can be
+   *                 returned.  We will never return a short read count unless
+   *                 EOF is reached.
    */
   public abstract int read(long pos, ByteBuffer buf) throws IOException;
 

hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt

@@ -623,6 +623,9 @@ Release 2.8.0 - UNRELEASED
     HDFS-9132. Pass genstamp to ReplicaAccessorBuilder. (Colin Patrick McCabe via
     Lei (Eddy) Xu)
 
+    HDFS-9133. ExternalBlockReader and ReplicaAccessor need to return -1 on read
+    when at EOF. (Colin Patrick McCabe via Lei (Eddy) Xu)
+
   OPTIMIZATIONS
 
     HDFS-8026. Trace FSOutputSummer#writeChecksumChunks rather than

hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestExternalBlockReader.java

@@ -190,8 +190,8 @@ public class TestExternalBlockReader {
             "than 0 at " + pos);
         return 0;
       }
-      int i = 0, nread = 0;
-      for (int ipos = (int)pos;
+      int i = 0, nread = 0, ipos;
+      for (ipos = (int)pos;
            (ipos < contents.length) && (nread < len);
            ipos++) {
         buf[i++] = contents[ipos];
@@ -199,6 +199,9 @@ public class TestExternalBlockReader {
         totalRead++;
         LOG.info("ipos = " + ipos + ", contents.length = " + contents.length + ", nread = " + nread + ", len = " + len);
       }
+      if ((nread == 0) && (ipos >= contents.length)) {
+        return -1;
+      }
       return nread;
     }
 
@@ -211,8 +214,8 @@ public class TestExternalBlockReader {
             "than 0 at " + pos);
         return 0;
       }
-      int i = 0, nread = 0;
-      for (int ipos = (int)pos;
+      int i = 0, nread = 0, ipos;
+      for (ipos = (int)pos;
            ipos < contents.length; ipos++) {
         try {
           buf.put(contents[ipos]);
@@ -222,6 +225,9 @@ public class TestExternalBlockReader {
         nread++;
         totalRead++;
       }
+      if ((nread == 0) && (ipos >= contents.length)) {
+        return -1;
+      }
       return nread;
     }
 
@@ -304,6 +310,11 @@ public class TestExternalBlockReader {
       Assert.assertEquals(1024L, accessor.totalRead);
       Assert.assertEquals("", accessor.getError());
       Assert.assertEquals(1, accessor.numCloses);
+      byte[] tempBuf = new byte[5];
+      Assert.assertEquals(-1, accessor.read(TEST_LENGTH,
+            tempBuf, 0, 0));
+      Assert.assertEquals(-1, accessor.read(TEST_LENGTH,
+            tempBuf, 0, tempBuf.length));
       accessors.remove(uuid);
     } finally {
       dfs.close();