hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mbau...@apache.org
Subject svn commit: r1239904 - in /hbase/branches/0.89-fb/src: main/java/org/apache/hadoop/hbase/io/hfile/ main/java/org/apache/hadoop/hbase/mapreduce/ main/java/org/apache/hadoop/hbase/regionserver/ test/java/org/apache/hadoop/hbase/regionserver/
Date Thu, 02 Feb 2012 22:53:50 GMT
Author: mbautin
Date: Thu Feb  2 22:53:49 2012
New Revision: 1239904

URL: http://svn.apache.org/viewvc?rev=1239904&view=rev
Log:
[jira] [HBASE-4589] [89-fb] CacheOnWrite broken in some cases because it can
conflict with evictOnClose

Summary:
Porting jgray's fix at https://issues.apache.org/jira/browse/HBASE-4589 to
hbase-89-internal. We open and close a newly flushed StoreFile for verification,
and the close operation happens to evict all cached-on-write blocks of the file.
The fix adds a boolean parameter to the HFile close method.

Submitting as an internal diff because HBASE-4542 is blocking updates to
hbase-89-fb-apache at the moment, and CacheConfig is part of the delta,
preventing this patch from applying.

Test Plan: Unit tests, dev cluster

Reviewers: liyintang, kannan, nspiegelberg

Reviewed By: kannan

CC: hbase-eng@lists

Differential Revision: https://phabricator.fb.com/D399740

Revert Plan: OK

Modified:
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
    hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
    hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/HFileReadWriteTest.java
    hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java
    hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java
    hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java

Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java?rev=1239904&r1=1239903&r2=1239904&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
(original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
Thu Feb  2 22:53:49 2012
@@ -310,7 +310,7 @@ public class HFileReaderV2 extends Abstr
 
   @Override
   public void close(boolean evictOnClose) throws IOException {
-    if (cacheConf.shouldEvictOnClose()) {
+    if (evictOnClose && cacheConf.shouldEvictOnClose()) {
       int numEvicted = cacheConf.getBlockCache().evictBlocksByPrefix(name
           + HFile.CACHE_KEY_SEPARATOR);
       LOG.debug("On close of file " + name + " evicted " + numEvicted

Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java?rev=1239904&r1=1239903&r2=1239904&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java
(original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java
Thu Feb  2 22:53:49 2012
@@ -27,6 +27,8 @@ import java.net.InetSocketAddress;
 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;
@@ -44,6 +46,7 @@ import org.apache.hadoop.io.WritableUtil
  * Writes HFile format version 2.
  */
 public class HFileWriterV2 extends AbstractHFileWriter {
+  static final Log LOG = LogFactory.getLog(HFileWriterV2.class);
 
   /** Max memstore (rwcc) timestamp in FileInfo */
   public static final byte[] MAX_MEMSTORE_TS_KEY =
@@ -211,6 +214,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.89-fb/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java?rev=1239904&r1=1239903&r2=1239904&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
(original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
Thu Feb  2 22:53:49 2012
@@ -296,7 +296,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.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java?rev=1239904&r1=1239903&r2=1239904&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java (original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java Thu
Feb  2 22:53:49 2012
@@ -493,7 +493,7 @@ public class Store extends SchemaConfigu
         for (final StoreFile f : result) {
           completionService.submit(new Callable<Void>() {
             public Void call() throws IOException {
-              f.closeReader();
+              f.closeReader(true);
               return null;
             }
           });
@@ -1357,7 +1357,7 @@ public class Store extends SchemaConfigu
       throw e;
     } finally {
       if (storeFile != null) {
-        storeFile.closeReader();
+        storeFile.closeReader(false);
       }
     }
   }

Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java?rev=1239904&r1=1239903&r2=1239904&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
(original)
+++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
Thu Feb  2 22:53:49 2012
@@ -492,12 +492,10 @@ public class StoreFile {
     return this.reader;
   }
 
-  /**
-   * @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;
     }
   }
@@ -507,7 +505,7 @@ public class StoreFile {
    * @throws IOException
    */
   public void deleteReader() throws IOException {
-    closeReader();
+    closeReader(true);
     this.fs.delete(getPath(), true);
   }
 
@@ -1136,6 +1134,10 @@ public class StoreFile {
       return reader.getScanner(cacheBlocks, pread, isCompaction);
     }
 
+    public void close(boolean evictOnClose) throws IOException {
+      reader.close(evictOnClose);
+    }
+
     public void close() throws IOException {
       reader.close();
     }

Modified: hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/HFileReadWriteTest.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/HFileReadWriteTest.java?rev=1239904&r1=1239903&r2=1239904&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/HFileReadWriteTest.java
(original)
+++ hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/HFileReadWriteTest.java
Thu Feb  2 22:53:49 2012
@@ -777,7 +777,7 @@ public class HFileReadWriteTest {
 
       }
     } finally {
-      storeFile.closeReader();
+      storeFile.closeReader(true);
       exec.shutdown();
 
       LruBlockCache c = (LruBlockCache) new CacheConfig(conf).getBlockCache();

Modified: hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java?rev=1239904&r1=1239903&r2=1239904&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java
(original)
+++ hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java
Thu Feb  2 22:53:49 2012
@@ -267,7 +267,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.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java?rev=1239904&r1=1239903&r2=1239904&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java
(original)
+++ hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java
Thu Feb  2 22:53:49 2012
@@ -100,7 +100,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.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java?rev=1239904&r1=1239903&r2=1239904&view=diff
==============================================================================
--- hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java
(original)
+++ hbase/branches/0.89-fb/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java
Thu Feb  2 22:53:49 2012
@@ -326,10 +326,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);
     }
@@ -375,7 +375,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);
@@ -525,7 +525,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);
@@ -735,7 +735,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);
@@ -755,7 +755,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,
@@ -790,9 +790,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);
@@ -800,7 +800,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());
@@ -814,7 +814,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());



Mime
View raw message