You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by ns...@apache.org on 2011/10/11 04:20:58 UTC

svn commit: r1181567 - in /hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/io/hfile: AbstractHFileReader.java HFileReaderV1.java HFileReaderV2.java

Author: nspiegelberg
Date: Tue Oct 11 02:20:57 2011
New Revision: 1181567

URL: http://svn.apache.org/viewvc?rev=1181567&view=rev
Log:
Correctly specifying pread and isCompaction in block read API calls

Summary:
This fixes incorrectly specified pread (positional-read) and isCompaction
(determining whether compaction or regular counters will be incremented)
parameters when calling block reader API functions. These bugs are specific to
HFile format v2, and were found during compaction workload testing with the
HFileReadWriteTest tool (266598).

Also refactoring HFile scanner hierarchy to prevent accidental modifications of
pread and isCompaction flags.

Test Plan:
Run the HFileReadWriteTest tool (266598), and make sure that the number of
preads is 0 and the number of seek + read operations per second is the same as
the number of blocks read per second, as reported by the tool.

Reviewed By: kannan
Reviewers: kannan, liyintang, pritam
CC: hbase@lists, , kannan, mbautin
Revert Plan:
OK

Differential Revision: 267302

Modified:
    hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java
    hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java
    hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java

Modified: hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java?rev=1181567&r1=1181566&r2=1181567&view=diff
==============================================================================
--- hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java (original)
+++ hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java Tue Oct 11 02:20:57 2011
@@ -327,14 +327,22 @@ public abstract class AbstractHFileReade
     protected ByteBuffer blockBuffer;
 
     protected boolean cacheBlocks;
-    protected boolean pread;
-    protected boolean isCompaction;
+    protected final boolean pread;
+    protected final boolean isCompaction;
 
     protected int currKeyLen;
     protected int currValueLen;
 
     protected int blockFetches;
 
+    public Scanner(final HFile.Reader reader, final boolean cacheBlocks,
+        final boolean pread, final boolean isCompaction) {
+      this.reader = reader;
+      this.cacheBlocks = cacheBlocks;
+      this.pread = pread;
+      this.isCompaction = isCompaction;
+    }
+
     @Override
     public Reader getReader() {
       return reader;

Modified: hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java?rev=1181567&r1=1181566&r2=1181567&view=diff
==============================================================================
--- hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java (original)
+++ hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java Tue Oct 11 02:20:57 2011
@@ -392,15 +392,13 @@ public class HFileReaderV1 extends Abstr
    * Implementation of {@link HFileScanner} interface.
    */
   protected static class ScannerV1 extends AbstractHFileReader.Scanner {
-    private final HFileReaderV1 reader;
+    private final HFileReaderV1 readerV1;
     private int currBlock;
 
     public ScannerV1(HFileReaderV1 reader, boolean cacheBlocks,
         final boolean pread, final boolean isCompaction) {
-      this.reader = reader;
-      this.cacheBlocks = cacheBlocks;
-      this.pread = pread;
-      this.isCompaction = isCompaction;
+      super(reader, cacheBlocks, pread, isCompaction);
+      readerV1 = reader;
     }
 
     @Override
@@ -470,7 +468,7 @@ public class HFileReaderV1 extends Abstr
           blockBuffer = null;
           return false;
         }
-        blockBuffer = reader.readBlockBuffer(currBlock, cacheBlocks, pread,
+        blockBuffer = readerV1.readBlockBuffer(currBlock, cacheBlocks, pread,
             isCompaction);
         currKeyLen = blockBuffer.getInt();
         currValueLen = blockBuffer.getInt();
@@ -490,7 +488,7 @@ public class HFileReaderV1 extends Abstr
 
     @Override
     public int seekTo(byte[] key, int offset, int length) throws IOException {
-      int b = reader.blockContainingKey(key, offset, length);
+      int b = readerV1.blockContainingKey(key, offset, length);
       if (b < 0) return -1; // falls before the beginning of the file! :-(
       // Avoid re-reading the same block (that'd be dumb).
       loadBlock(b, true);
@@ -516,7 +514,7 @@ public class HFileReaderV1 extends Abstr
         }
       }
 
-      int b = reader.blockContainingKey(key, offset, length);
+      int b = readerV1.blockContainingKey(key, offset, length);
       if (b < 0) {
         return -1;
       }
@@ -583,7 +581,7 @@ public class HFileReaderV1 extends Abstr
     @Override
     public boolean seekBefore(byte[] key, int offset, int length)
     throws IOException {
-      int b = reader.blockContainingKey(key, offset, length);
+      int b = readerV1.blockContainingKey(key, offset, length);
       if (b < 0)
         return false; // key is before the start of the file.
 
@@ -635,7 +633,7 @@ public class HFileReaderV1 extends Abstr
         return true;
       }
       currBlock = 0;
-      blockBuffer = reader.readBlockBuffer(currBlock, cacheBlocks, pread,
+      blockBuffer = readerV1.readBlockBuffer(currBlock, cacheBlocks, pread,
           isCompaction);
       currKeyLen = blockBuffer.getInt();
       currValueLen = blockBuffer.getInt();
@@ -645,13 +643,13 @@ public class HFileReaderV1 extends Abstr
 
     private void loadBlock(int bloc, boolean rewind) throws IOException {
       if (blockBuffer == null) {
-        blockBuffer = reader.readBlockBuffer(bloc, cacheBlocks, pread,
+        blockBuffer = readerV1.readBlockBuffer(bloc, cacheBlocks, pread,
             isCompaction);
         currBlock = bloc;
         blockFetches++;
       } else {
         if (bloc != currBlock) {
-          blockBuffer = reader.readBlockBuffer(bloc, cacheBlocks, pread,
+          blockBuffer = readerV1.readBlockBuffer(bloc, cacheBlocks, pread,
               isCompaction);
           currBlock = bloc;
           blockFetches++;

Modified: hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java?rev=1181567&r1=1181566&r2=1181567&view=diff
==============================================================================
--- hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java (original)
+++ hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java Tue Oct 11 02:20:57 2011
@@ -204,12 +204,20 @@ public class HFileReaderV2 extends Abstr
     }
   }
 
+  /**
+   * Implements the "basic block reader" API, used mainly by
+   * {@link HFileBlockIndex.BlockIndexReader} in
+   * {@link HFileBlockIndex.BlockIndexReader#seekToDataBlock(byte[], int, int,
+   * HFileBlock)} in a random-read access pattern.
+   */
   @Override
   public HFileBlock readBlockData(long offset, long onDiskSize,
       int uncompressedSize, boolean pread) throws IOException {
     if (onDiskSize >= Integer.MAX_VALUE) {
       throw new IOException("Invalid on-disk size: " + onDiskSize);
     }
+
+    // Assuming we are not doing a compaction.
     return readBlock(offset, (int) onDiskSize, true, pread, false);
   }
 
@@ -225,7 +233,7 @@ public class HFileReaderV2 extends Abstr
    * @throws IOException
    */
   public HFileBlock readBlock(long dataBlockOffset, int onDiskBlockSize,
-      boolean cacheBlock, final boolean pread, final boolean isCompaction)
+      boolean cacheBlock, boolean pread, final boolean isCompaction)
       throws IOException {
     if (dataBlockIndexReader == null) {
       throw new IOException("Block index not loaded");
@@ -273,7 +281,7 @@ public class HFileReaderV2 extends Abstr
       // Load block from filesystem.
       long now = System.currentTimeMillis();
       HFileBlock dataBlock = fsBlockReader.readBlockData(dataBlockOffset,
-          onDiskBlockSize, -1, true);
+          onDiskBlockSize, -1, pread);
 
       long delta = System.currentTimeMillis() - now;
       HFile.readTime += delta;
@@ -337,10 +345,7 @@ public class HFileReaderV2 extends Abstr
 
     public ScannerV2(HFileReaderV2 r, boolean cacheBlocks,
         final boolean pread, final boolean isCompaction) {
-      this.reader = r;
-      this.cacheBlocks = cacheBlocks;
-      this.pread = pread;
-      this.isCompaction = isCompaction;
+      super(r, cacheBlocks, pread, isCompaction);
     }
 
     @Override
@@ -449,7 +454,7 @@ public class HFileReaderV2 extends Abstr
         curBlock = reader.readBlock(curBlock.getOffset()
             + curBlock.getOnDiskSizeWithHeader(),
             curBlock.getNextBlockOnDiskSizeWithHeader(), cacheBlocks, pread,
-            false);
+            isCompaction);
       } while (!curBlock.getBlockType().equals(BlockType.DATA));
 
       return curBlock;