Repository: hbase Updated Branches: refs/heads/branch-1 3feb87b00 -> 1f1ab8c87
HBASE-18587 Fix flaky TestFileIOEngine This short circuits reads and writes with 0 length and also removes flakiness in TestFileIOEngine Signed-off-by: Michael Stack <st...@apache.org> Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/1f1ab8c8 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/1f1ab8c8 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/1f1ab8c8 Branch: refs/heads/branch-1 Commit: 1f1ab8c873b193675766969df83db91213137d72 Parents: 3feb87b Author: Zach York <zy...@amazon.com> Authored: Thu Aug 10 16:55:28 2017 -0700 Committer: Michael Stack <st...@apache.org> Committed: Wed Aug 16 14:50:11 2017 -0700 ---------------------------------------------------------------------- .../hbase/io/hfile/bucket/FileIOEngine.java | 8 +- .../hbase/io/hfile/bucket/TestFileIOEngine.java | 122 +++++++++++-------- 2 files changed, 79 insertions(+), 51 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/1f1ab8c8/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/FileIOEngine.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/FileIOEngine.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/FileIOEngine.java index a7d6956..3419587 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/FileIOEngine.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/FileIOEngine.java @@ -102,7 +102,10 @@ public class FileIOEngine implements IOEngine { */ @Override public int read(ByteBuffer dstBuffer, long offset) throws IOException { - return accessFile(readAccessor, dstBuffer, offset); + if (dstBuffer.remaining() != 0) { + return accessFile(readAccessor, dstBuffer, offset); + } + return 0; } /** @@ -113,6 +116,9 @@ public class FileIOEngine implements IOEngine { */ @Override public void write(ByteBuffer srcBuffer, long offset) throws IOException { + if (!srcBuffer.hasRemaining()) { + return; + } accessFile(writeAccessor, srcBuffer, offset); } http://git-wip-us.apache.org/repos/asf/hbase/blob/1f1ab8c8/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestFileIOEngine.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestFileIOEngine.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestFileIOEngine.java index 8c71c09..a03818b 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestFileIOEngine.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestFileIOEngine.java @@ -18,7 +18,7 @@ */ package org.apache.hadoop.hbase.io.hfile.bucket; -import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertArrayEquals; import java.io.File; import java.io.IOException; @@ -27,6 +27,8 @@ import java.util.ArrayList; import java.util.List; import org.apache.hadoop.hbase.testclassification.SmallTests; +import org.junit.After; +import org.junit.Before; import org.junit.Test; import org.junit.experimental.categories.Category; @@ -35,61 +37,81 @@ import org.junit.experimental.categories.Category; */ @Category(SmallTests.class) public class TestFileIOEngine { - @Test - public void testFileIOEngine() throws IOException { - long totalCapacity = 6 * 1024 * 1024; // 6 MB - String[] filePaths = { "testFileIOEngine1", "testFileIOEngine2", - "testFileIOEngine3" }; - long sizePerFile = totalCapacity / filePaths.length; // 2 MB per File - List<Long> boundaryStartPositions = new ArrayList<Long>(); + + private static final long TOTAL_CAPACITY = 6 * 1024 * 1024; // 6 MB + private static final String[] FILE_PATHS = {"testFileIOEngine1", "testFileIOEngine2", + "testFileIOEngine3"}; + private static final long SIZE_PER_FILE = TOTAL_CAPACITY / FILE_PATHS.length; // 2 MB per File + private final static List<Long> boundaryStartPositions = new ArrayList<Long>(); + private final static List<Long> boundaryStopPositions = new ArrayList<Long>(); + + private FileIOEngine fileIOEngine; + + static { boundaryStartPositions.add(0L); - for (int i = 1; i < filePaths.length; i++) { - boundaryStartPositions.add(sizePerFile * i - 1); - boundaryStartPositions.add(sizePerFile * i); - boundaryStartPositions.add(sizePerFile * i + 1); + for (int i = 1; i < FILE_PATHS.length; i++) { + boundaryStartPositions.add(SIZE_PER_FILE * i - 1); + boundaryStartPositions.add(SIZE_PER_FILE * i); + boundaryStartPositions.add(SIZE_PER_FILE * i + 1); } - List<Long> boundaryStopPositions = new ArrayList<Long>(); - for (int i = 1; i < filePaths.length; i++) { - boundaryStopPositions.add(sizePerFile * i - 1); - boundaryStopPositions.add(sizePerFile * i); - boundaryStopPositions.add(sizePerFile * i + 1); + for (int i = 1; i < FILE_PATHS.length; i++) { + boundaryStopPositions.add(SIZE_PER_FILE * i - 1); + boundaryStopPositions.add(SIZE_PER_FILE * i); + boundaryStopPositions.add(SIZE_PER_FILE * i + 1); } - boundaryStopPositions.add(sizePerFile * filePaths.length - 1); - FileIOEngine fileIOEngine = new FileIOEngine(totalCapacity, filePaths); - try { - for (int i = 0; i < 500; i++) { - int len = (int) Math.floor(Math.random() * 100); - long offset = (long) Math.floor(Math.random() * totalCapacity % (totalCapacity - len)); - if (i < boundaryStartPositions.size()) { - // make the boundary start positon - offset = boundaryStartPositions.get(i); - } else if ((i - boundaryStartPositions.size()) < boundaryStopPositions.size()) { - // make the boundary stop positon - offset = boundaryStopPositions.get(i - boundaryStartPositions.size()) - len + 1; - } else if (i % 2 == 0) { - // make the cross-files block writing/reading - offset = Math.max(1, i % filePaths.length) * sizePerFile - len / 2; - } - byte[] data1 = new byte[len]; - for (int j = 0; j < data1.length; ++j) { - data1[j] = (byte) (Math.random() * 255); - } - byte[] data2 = new byte[len]; - fileIOEngine.write(ByteBuffer.wrap(data1), offset); - fileIOEngine.read(ByteBuffer.wrap(data2), offset); - for (int j = 0; j < data1.length; ++j) { - assertTrue(data1[j] == data2[j]); - } + boundaryStopPositions.add(SIZE_PER_FILE * FILE_PATHS.length - 1); + } + + @Before + public void setUp() throws IOException { + fileIOEngine = new FileIOEngine(TOTAL_CAPACITY, FILE_PATHS); + } + + @After + public void cleanUp() throws IOException { + fileIOEngine.shutdown(); + for (String filePath : FILE_PATHS) { + File file = new File(filePath); + if (file.exists()) { + file.delete(); } - } finally { - fileIOEngine.shutdown(); - for (String filePath : filePaths) { - File file = new File(filePath); - if (file.exists()) { - file.delete(); - } + } + } + + @Test + public void testFileIOEngine() throws IOException { + for (int i = 0; i < 500; i++) { + int len = (int) Math.floor(Math.random() * 100) + 1; + long offset = (long) Math.floor(Math.random() * TOTAL_CAPACITY % (TOTAL_CAPACITY - len)); + if (i < boundaryStartPositions.size()) { + // make the boundary start positon + offset = boundaryStartPositions.get(i); + } else if ((i - boundaryStartPositions.size()) < boundaryStopPositions.size()) { + // make the boundary stop positon + offset = boundaryStopPositions.get(i - boundaryStartPositions.size()) - len + 1; + } else if (i % 2 == 0) { + // make the cross-files block writing/reading + offset = Math.max(1, i % FILE_PATHS.length) * SIZE_PER_FILE - len / 2; + } + byte[] data1 = new byte[len]; + for (int j = 0; j < data1.length; ++j) { + data1[j] = (byte) (Math.random() * 255); } + byte[] data2 = new byte[len]; + fileIOEngine.write(ByteBuffer.wrap(data1), offset); + fileIOEngine.read(ByteBuffer.wrap(data2), offset); + assertArrayEquals(data1, data2); } + } + + @Test + public void testFileIOEngineHandlesZeroLengthInput() throws IOException { + byte[] data1 = new byte[0]; + + byte[] data2 = new byte[0]; + fileIOEngine.write(ByteBuffer.wrap(data1), 0); + fileIOEngine.read(ByteBuffer.wrap(data2), 0); + assertArrayEquals(data1, data2); } }
