You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by mb...@apache.org on 2012/02/02 23:53:50 UTC
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/
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());