From 67b0e967f0e13eb6bed123fc7ba4cce0dcca198f Mon Sep 17 00:00:00 2001 From: Lei Xu Date: Fri, 25 Sep 2015 16:01:41 -0700 Subject: [PATCH] HDFS-9133. ExternalBlockReader and ReplicaAccessor need to return -1 on read when at EOF. (Colin Patrick McCabe via Lei (Eddy) Xu) --- .../hadoop/hdfs/ExternalBlockReader.java | 6 ++++++ .../apache/hadoop/hdfs/ReplicaAccessor.java | 10 ++++++---- hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt | 3 +++ .../hadoop/hdfs/TestExternalBlockReader.java | 19 +++++++++++++++---- 4 files changed, 30 insertions(+), 8 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ExternalBlockReader.java b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ExternalBlockReader.java index e135d8e41a..3711a9d7d8 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ExternalBlockReader.java +++ b/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 int read(byte[] buf, int off, int len) throws IOException { @Override public int read(ByteBuffer buf) throws IOException { int nread = accessor.read(pos, buf); + if (nread < 0) { + return nread; + } pos += nread; return nread; } diff --git a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessor.java b/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessor.java index 720e6a1415..e0b21e8f58 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-client/src/main/java/org/apache/hadoop/hdfs/ReplicaAccessor.java +++ b/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 int read(long pos, byte[] buf, int off, int len) * * @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; diff --git a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt index 218f155e2b..e3d9660dc7 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt +++ b/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt @@ -971,6 +971,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 diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestExternalBlockReader.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestExternalBlockReader.java index 3a0e8e63ad..e039145512 100644 --- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestExternalBlockReader.java +++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestExternalBlockReader.java @@ -190,8 +190,8 @@ public synchronized int read(long pos, byte[] buf, int off, int len) "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 synchronized int read(long pos, byte[] buf, int off, int len) 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 synchronized int read(long pos, ByteBuffer buf) throws IOException { "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 synchronized int read(long pos, ByteBuffer buf) throws IOException { nread++; totalRead++; } + if ((nread == 0) && (ipos >= contents.length)) { + return -1; + } return nread; } @@ -304,6 +310,11 @@ public void testExternalBlockReader() throws Exception { 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();