You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by jg...@apache.org on 2011/10/15 00:42:29 UTC

svn commit: r1183543 - in /hbase/branches/0.92: ./ src/main/java/org/apache/hadoop/hbase/io/hfile/ src/main/java/org/apache/hadoop/hbase/mapreduce/ src/main/java/org/apache/hadoop/hbase/regionserver/ src/test/java/org/apache/hadoop/hbase/regionserver/

Author: jgray
Date: Fri Oct 14 22:42:29 2011
New Revision: 1183543

URL: http://svn.apache.org/viewvc?rev=1183543&view=rev
Log:
HBASE-4589  CacheOnWrite broken in some cases because it can conflict with evictOnClose (jgray)

Modified:
    hbase/branches/0.92/CHANGES.txt
    hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java
    hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java
    hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
    hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java
    hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
    hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
    hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
    hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java
    hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java
    hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java

Modified: hbase/branches/0.92/CHANGES.txt
URL: http://svn.apache.org/viewvc/hbase/branches/0.92/CHANGES.txt?rev=1183543&r1=1183542&r2=1183543&view=diff
==============================================================================
--- hbase/branches/0.92/CHANGES.txt (original)
+++ hbase/branches/0.92/CHANGES.txt Fri Oct 14 22:42:29 2011
@@ -347,6 +347,8 @@ Release 0.92.0 - Unreleased
    HBASE-4492  TestRollingRestart fails intermittently
    HBASE-4512  JVMClusterUtil throwing wrong exception when master thread cannot be created (Ram)
    HBASE-4479  TestMasterFailover failure in Hbase-0.92#17 (Ram)
+   HBASE-4589  CacheOnWrite broken in some cases because it can conflict
+               with evictOnClose (jgray)
 
   IMPROVEMENTS
    HBASE-3290  Max Compaction Size (Nicolas Spiegelberg via Stack)  

Modified: hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java?rev=1183543&r1=1183542&r2=1183543&view=diff
==============================================================================
--- hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java (original)
+++ hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java Fri Oct 14 22:42:29 2011
@@ -322,6 +322,9 @@ public class HFile {
     DataInput getBloomFilterMetadata() throws IOException;
 
     Path getPath();
+
+    /** Close method with optional evictOnClose */
+    void close(boolean evictOnClose) throws IOException;
   }
 
   private static Reader pickReaderVersion(Path path, FSDataInputStream fsdis,

Modified: hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java?rev=1183543&r1=1183542&r2=1183543&view=diff
==============================================================================
--- hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java (original)
+++ hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java Fri Oct 14 22:42:29 2011
@@ -348,7 +348,12 @@ public class HFileReaderV1 extends Abstr
 
   @Override
   public void close() throws IOException {
-    if (cacheConf.shouldEvictOnClose()) {
+    close(cacheConf.shouldEvictOnClose());
+  }
+
+  @Override
+  public void close(boolean evictOnClose) throws IOException {
+    if (evictOnClose) {
       int numEvicted = 0;
       for (int i = 0; i < dataBlockIndexReader.getRootBlockCount(); i++) {
         if (cacheConf.getBlockCache().evictBlock(HFile.getBlockCacheKey(name,

Modified: hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java?rev=1183543&r1=1183542&r2=1183543&view=diff
==============================================================================
--- hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java (original)
+++ hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java Fri Oct 14 22:42:29 2011
@@ -293,7 +293,11 @@ public class HFileReaderV2 extends Abstr
 
   @Override
   public void close() throws IOException {
-    if (cacheConf.shouldEvictOnClose()) {
+    close(cacheConf.shouldEvictOnClose());
+  }
+
+  public void close(boolean evictOnClose) throws IOException {
+    if (evictOnClose) {
       int numEvicted = cacheConf.getBlockCache().evictBlocksByPrefix(name
           + HFile.CACHE_KEY_SEPARATOR);
       LOG.debug("On close of file " + name + " evicted " + numEvicted

Modified: hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java?rev=1183543&r1=1183542&r2=1183543&view=diff
==============================================================================
--- hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java (original)
+++ hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java Fri Oct 14 22:42:29 2011
@@ -26,6 +26,8 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.List;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FSDataOutputStream;
 import org.apache.hadoop.fs.FileSystem;
@@ -42,6 +44,7 @@ import org.apache.hadoop.io.Writable;
  * Writes HFile format version 2.
  */
 public class HFileWriterV2 extends AbstractHFileWriter {
+  static final Log LOG = LogFactory.getLog(HFileWriterV2.class);
 
   /** Inline block writers for multi-level block index and compound Blooms. */
   private List<InlineBlockWriter> inlineBlockWriters =
@@ -174,6 +177,8 @@ public class HFileWriterV2 extends Abstr
 
     // Meta data block index writer
     metaBlockIndexWriter = new HFileBlockIndex.BlockIndexWriter();
+
+    LOG.debug("HFileWriter initialized with " + cacheConf);
   }
 
   /**

Modified: hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java?rev=1183543&r1=1183542&r2=1183543&view=diff
==============================================================================
--- hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java (original)
+++ hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java Fri Oct 14 22:42:29 2011
@@ -409,7 +409,7 @@ public class LoadIncrementalHFiles exten
       }
     } finally {
       if (halfWriter != null) halfWriter.close();
-      if (halfReader != null) halfReader.close();
+      if (halfReader != null) halfReader.close(cacheConf.shouldEvictOnClose());
     }
   }
 

Modified: hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java?rev=1183543&r1=1183542&r2=1183543&view=diff
==============================================================================
--- hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java (original)
+++ hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java Fri Oct 14 22:42:29 2011
@@ -421,7 +421,7 @@ public class Store implements HeapSize {
       storefiles = ImmutableList.of();
 
       for (StoreFile f: result) {
-        f.closeReader();
+        f.closeReader(true);
       }
       LOG.debug("closed " + this.storeNameStr);
       return result;
@@ -1215,7 +1215,7 @@ public class Store implements HeapSize {
       throw e;
     } finally {
       if (storeFile != null) {
-        storeFile.closeReader();
+        storeFile.closeReader(false);
       }
     }
   }

Modified: hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java?rev=1183543&r1=1183542&r2=1183543&view=diff
==============================================================================
--- hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java (original)
+++ hbase/branches/0.92/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java Fri Oct 14 22:42:29 2011
@@ -526,11 +526,13 @@ public class StoreFile {
   }
 
   /**
+   * @param b 
    * @throws IOException
    */
-  public synchronized void closeReader() throws IOException {
+  public synchronized void closeReader(boolean evictOnClose)
+  throws IOException {
     if (this.reader != null) {
-      this.reader.close();
+      this.reader.close(evictOnClose);
       this.reader = null;
     }
   }
@@ -540,7 +542,7 @@ public class StoreFile {
    * @throws IOException
    */
   public void deleteReader() throws IOException {
-    closeReader();
+    closeReader(true);
     this.fs.delete(getPath(), true);
   }
 
@@ -1011,8 +1013,8 @@ public class StoreFile {
       return reader.getScanner(cacheBlocks, pread);
     }
 
-    public void close() throws IOException {
-      reader.close();
+    public void close(boolean evictOnClose) throws IOException {
+      reader.close(evictOnClose);
     }
 
     public boolean shouldSeek(Scan scan, final SortedSet<byte[]> columns) {

Modified: hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java?rev=1183543&r1=1183542&r2=1183543&view=diff
==============================================================================
--- hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java (original)
+++ hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java Fri Oct 14 22:42:29 2011
@@ -268,7 +268,7 @@ public class TestCompoundBloomFilter {
       }
     }
 
-    r.close();
+    r.close(true); // end of test so evictOnClose
   }
 
   private boolean isInBloom(StoreFileScanner scanner, byte[] row, BloomType bt,

Modified: hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java?rev=1183543&r1=1183542&r2=1183543&view=diff
==============================================================================
--- hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java (original)
+++ hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java Fri Oct 14 22:42:29 2011
@@ -101,7 +101,7 @@ public class TestFSErrorsExposed {
       LOG.info("Got expected exception", ioe);
       assertTrue(ioe.getMessage().contains("Fault"));
     }
-    reader.close();
+    reader.close(true); // end of test so evictOnClose
   }
 
   /**

Modified: hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java?rev=1183543&r1=1183542&r2=1183543&view=diff
==============================================================================
--- hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java (original)
+++ hbase/branches/0.92/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java Fri Oct 14 22:42:29 2011
@@ -321,10 +321,10 @@ public class TestStoreFile extends HBase
       assertTrue(count == 0);
     } finally {
       if (top != null) {
-        top.close();
+        top.close(true); // evict since we are about to delete the file
       }
       if (bottom != null) {
-        bottom.close();
+        bottom.close(true); // evict since we are about to delete the file
       }
       fs.delete(f.getPath(), true);
     }
@@ -370,7 +370,7 @@ public class TestStoreFile extends HBase
         if (exists) falsePos++;
       }
     }
-    reader.close();
+    reader.close(true); // evict because we are about to delete the file
     fs.delete(f, true);
     assertEquals("False negatives: " + falseNeg, 0, falseNeg);
     int maxFalsePos = (int) (2 * 2000 * err);
@@ -466,7 +466,7 @@ public class TestStoreFile extends HBase
           }
         }
       }
-      reader.close();
+      reader.close(true); // evict because we are about to delete the file
       fs.delete(f, true);
       System.out.println(bt[x].toString());
       System.out.println("  False negatives: " + falseNeg);
@@ -677,7 +677,7 @@ public class TestStoreFile extends HBase
     assertEquals(startEvicted, cs.getEvictedCount());
     startMiss += 3;
     scanner.close();
-    reader.close();
+    reader.close(cacheConf.shouldEvictOnClose());
 
     // Now write a StoreFile with three blocks, with cache on write on
     conf.setBoolean(CacheConfig.CACHE_BLOCKS_ON_WRITE_KEY, true);
@@ -697,7 +697,7 @@ public class TestStoreFile extends HBase
     assertEquals(startEvicted, cs.getEvictedCount());
     startHit += 3;
     scanner.close();
-    reader.close();
+    reader.close(cacheConf.shouldEvictOnClose());
 
     // Let's read back the two files to ensure the blocks exactly match
     hsf = new StoreFile(this.fs, pathCowOff, conf, cacheConf,
@@ -730,9 +730,9 @@ public class TestStoreFile extends HBase
     assertEquals(startEvicted, cs.getEvictedCount());
     startHit += 6;
     scannerOne.close();
-    readerOne.close();
+    readerOne.close(cacheConf.shouldEvictOnClose());
     scannerTwo.close();
-    readerTwo.close();
+    readerTwo.close(cacheConf.shouldEvictOnClose());
 
     // Let's close the first file with evict on close turned on
     conf.setBoolean("hbase.rs.evictblocksonclose", true);
@@ -740,7 +740,7 @@ public class TestStoreFile extends HBase
     hsf = new StoreFile(this.fs, pathCowOff, conf, cacheConf,
         StoreFile.BloomType.NONE);
     reader = hsf.createReader();
-    reader.close();
+    reader.close(cacheConf.shouldEvictOnClose());
 
     // We should have 3 new evictions
     assertEquals(startHit, cs.getHitCount());
@@ -754,7 +754,7 @@ public class TestStoreFile extends HBase
     hsf = new StoreFile(this.fs, pathCowOn, conf, cacheConf,
         StoreFile.BloomType.NONE);
     reader = hsf.createReader();
-    reader.close();
+    reader.close(cacheConf.shouldEvictOnClose());
 
     // We expect no changes
     assertEquals(startHit, cs.getHitCount());