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)