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:38 UTC
svn commit: r1181563 -
/hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
Author: nspiegelberg
Date: Tue Oct 11 02:20:38 2011
New Revision: 1181563
URL: http://svn.apache.org/viewvc?rev=1181563&view=rev
Log:
HBASE-3987: Fix a Bloom Filter NPE on a failure to load Bloom filter data
Summary:
This is a fix for an NPE that happens in passesBloomFilter. The meta block
fails to load, and the catch block on lines 1066-1068 sets the Bloom filter to
null. Then all other threads waiting on the Bloom filter to load get a chance to
try to load the meta block, and one of them eventually succeeds and proceeds to
query the Bloom filter in StoreFile.passesBloomFilter, but bloomFilter has been
already set to null.
The fix (Nicolas's idea) is to cache the bloomFilter variable as a local
variable in passesBloomFilter.
A similar fix for the "earth" branch is D266546.
Test Plan:
Unit test (TestStoreFile)
Reviewed By: kannan
Reviewers: nspiegelberg, kannan
CC: hbase@lists, , kannan
Revert Plan:
OK
Differential Revision: 266571
Modified:
hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
Modified: hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java?rev=1181563&r1=1181562&r2=1181563&view=diff
==============================================================================
--- hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java (original)
+++ hbase/branches/0.89/src/main/java/org/apache/hadoop/hbase/regionserver/StoreFile.java Tue Oct 11 02:20:38 2011
@@ -1023,7 +1023,11 @@ public class StoreFile {
private boolean passesBloomFilter(Scan scan,
final SortedSet<byte[]> columns) {
- if (this.bloomFilter == null || !scan.isGetScan()) {
+ // Cache Bloom filter as a local variable in case it is set to null by
+ // another thread on an IO error.
+ BloomFilter bloomFilter = this.bloomFilter;
+
+ if (bloomFilter == null || !scan.isGetScan()) {
return true;
}
@@ -1086,13 +1090,13 @@ public class StoreFile {
exists = false;
} else {
exists =
- this.bloomFilter.contains(key, 0, key.length, bloom) ||
- this.bloomFilter.contains(rowBloomKey, 0, rowBloomKey.length,
+ bloomFilter.contains(key, 0, key.length, bloom) ||
+ bloomFilter.contains(rowBloomKey, 0, rowBloomKey.length,
bloom);
}
} else {
exists = !keyIsAfterLast
- && this.bloomFilter.contains(key, 0, key.length, bloom);
+ && bloomFilter.contains(key, 0, key.length, bloom);
}
if (exists)