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:41:45 UTC
svn commit: r1183541 - in /hbase/trunk: ./
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:41:45 2011
New Revision: 1183541
URL: http://svn.apache.org/viewvc?rev=1183541&view=rev
Log:
HBASE-4589 CacheOnWrite broken in some cases because it can conflict with evictOnClose (jgray)
Modified:
hbase/trunk/CHANGES.txt
hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java
hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java
hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java
hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java
hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java
hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java
Modified: hbase/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/hbase/trunk/CHANGES.txt?rev=1183541&r1=1183540&r2=1183541&view=diff
==============================================================================
--- hbase/trunk/CHANGES.txt (original)
+++ hbase/trunk/CHANGES.txt Fri Oct 14 22:41:45 2011
@@ -363,6 +363,8 @@ Release 0.92.0 - Unreleased
Hadoop 0.23 (todd)
HBASE-3446 ProcessServerShutdown fails if META moves, orphaning lots of
regions
+ HBASE-4589 CacheOnWrite broken in some cases because it can conflict
+ with evictOnClose (jgray)
TESTS
HBASE-4450 test for number of blocks read: to serve as baseline for expected
Modified: hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java
URL: http://svn.apache.org/viewvc/hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java?rev=1183541&r1=1183540&r2=1183541&view=diff
==============================================================================
--- hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java (original)
+++ hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java Fri Oct 14 22:41:45 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/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java
URL: http://svn.apache.org/viewvc/hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java?rev=1183541&r1=1183540&r2=1183541&view=diff
==============================================================================
--- hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java (original)
+++ hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV1.java Fri Oct 14 22:41:45 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/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java
URL: http://svn.apache.org/viewvc/hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java?rev=1183541&r1=1183540&r2=1183541&view=diff
==============================================================================
--- hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java (original)
+++ hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileReaderV2.java Fri Oct 14 22:41:45 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/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java
URL: http://svn.apache.org/viewvc/hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java?rev=1183541&r1=1183540&r2=1183541&view=diff
==============================================================================
--- hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java (original)
+++ hbase/trunk/src/main/java/org/apache/hadoop/hbase/io/hfile/HFileWriterV2.java Fri Oct 14 22:41:45 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/trunk/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java
URL: http://svn.apache.org/viewvc/hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java?rev=1183541&r1=1183540&r2=1183541&view=diff
==============================================================================
--- hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java (original)
+++ hbase/trunk/src/main/java/org/apache/hadoop/hbase/mapreduce/LoadIncrementalHFiles.java Fri Oct 14 22:41:45 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/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java
URL: http://svn.apache.org/viewvc/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java?rev=1183541&r1=1183540&r2=1183541&view=diff
==============================================================================
--- hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java (original)
+++ hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/Store.java Fri Oct 14 22:41:45 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/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
URL: http://svn.apache.org/viewvc/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java?rev=1183541&r1=1183540&r2=1183541&view=diff
==============================================================================
--- hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java (original)
+++ hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java Fri Oct 14 22:41:45 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/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java
URL: http://svn.apache.org/viewvc/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java?rev=1183541&r1=1183540&r2=1183541&view=diff
==============================================================================
--- hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java (original)
+++ hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestCompoundBloomFilter.java Fri Oct 14 22:41:45 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/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java
URL: http://svn.apache.org/viewvc/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java?rev=1183541&r1=1183540&r2=1183541&view=diff
==============================================================================
--- hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java (original)
+++ hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestFSErrorsExposed.java Fri Oct 14 22:41:45 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/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java
URL: http://svn.apache.org/viewvc/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java?rev=1183541&r1=1183540&r2=1183541&view=diff
==============================================================================
--- hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java (original)
+++ hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java Fri Oct 14 22:41:45 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());