hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From raw...@apache.org
Subject svn commit: r955784 - in /hbase/trunk: CHANGES.txt src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreScanner.java
Date Thu, 17 Jun 2010 23:20:41 GMT
Author: rawson
Date: Thu Jun 17 23:20:40 2010
New Revision: 955784

URL: http://svn.apache.org/viewvc?rev=955784&view=rev
Log:
HBASE-2740  NPE in ReadWriteConsistencyControl


Modified:
    hbase/trunk/CHANGES.txt
    hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
    hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreScanner.java

Modified: hbase/trunk/CHANGES.txt
URL: http://svn.apache.org/viewvc/hbase/trunk/CHANGES.txt?rev=955784&r1=955783&r2=955784&view=diff
==============================================================================
--- hbase/trunk/CHANGES.txt (original)
+++ hbase/trunk/CHANGES.txt Thu Jun 17 23:20:40 2010
@@ -394,6 +394,7 @@ Release 0.21.0 - Unreleased
    HBASE-2738  TestTimeRangeMapRed updated now that we keep multiple cells with
                same timestamp in MemStore
    HBASE-2725  Shutdown hook management is gone in trunk; restore
+   HBASE-2740  NPE in ReadWriteConsistencyControl
 
   IMPROVEMENTS
    HBASE-1760  Cleanup TODOs in HTable

Modified: hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
URL: http://svn.apache.org/viewvc/hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java?rev=955784&r1=955783&r2=955784&view=diff
==============================================================================
--- hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java (original)
+++ hbase/trunk/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java Thu Jun
17 23:20:40 2010
@@ -31,7 +31,7 @@ import java.util.List;
 import java.util.NavigableSet;
 
 /**
- * Scanner scans both the memstore and the HStore. Coaleace KeyValue stream
+ * Scanner scans both the memstore and the HStore. Coalesce KeyValue stream
  * into List<KeyValue> for a single row.
  */
 class StoreScanner implements KeyValueScanner, InternalScanner, ChangedReadersObserver {
@@ -194,6 +194,8 @@ class StoreScanner implements KeyValueSc
       this.store.deleteChangedReaderObserver(this);
     if (this.heap != null)
       this.heap.close();
+    this.heap = null; // CLOSED!
+    this.lastTop = null; // If both are null, we are closed.
   }
 
   public synchronized boolean seek(KeyValue key) throws IOException {
@@ -298,6 +300,13 @@ class StoreScanner implements KeyValueSc
   public synchronized void updateReaders() throws IOException {
     if (this.closing) return;
 
+    // All public synchronized API calls will call 'checkReseek' which will cause
+    // the scanner stack to reseek if this.heap==null && this.lastTop != null.
+    // But if two calls to updateReaders() happen without a 'next' or 'peek' then we
+    // will end up calling this.peek() which would cause a reseek in the middle of a updateReaders
+    // which is NOT what we want, not to mention could cause an NPE. So we early out here.
+    if (this.heap == null) return;
+
     // this could be null.
     this.lastTop = this.peek();
 

Modified: hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreScanner.java
URL: http://svn.apache.org/viewvc/hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreScanner.java?rev=955784&r1=955783&r2=955784&view=diff
==============================================================================
--- hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreScanner.java (original)
+++ hbase/trunk/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreScanner.java Thu
Jun 17 23:20:40 2010
@@ -109,7 +109,6 @@ public class TestStoreScanner extends Te
     results = new ArrayList<KeyValue>();
     assertEquals(true, scan.next(results));
     assertEquals(3, results.size());
-
   }
 
   public void testScanSameTimestamp() throws IOException {
@@ -285,6 +284,7 @@ public class TestStoreScanner extends Te
     assertEquals(kvs[0], results.get(0));
     assertEquals(kvs[1], results.get(1));
   }
+
   public void testWildCardScannerUnderDeletes() throws IOException {
     KeyValue [] kvs = new KeyValue [] {
         KeyValueTestUtil.create("R1", "cf", "a", 2, KeyValue.Type.Put, "dont-care"), // inc
@@ -317,6 +317,7 @@ public class TestStoreScanner extends Te
     assertEquals(kvs[6], results.get(3));
     assertEquals(kvs[7], results.get(4));
   }
+
   public void testDeleteFamily() throws IOException {
     KeyValue [] kvs = new KeyValue[] {
         KeyValueTestUtil.create("R1", "cf", "a", 100, KeyValue.Type.DeleteFamily, "dont-care"),
@@ -363,8 +364,7 @@ public class TestStoreScanner extends Te
     assertEquals(kvs[3], results.get(0));
   }
 
-  public void testSkipColumn() throws IOException {
-    KeyValue [] kvs = new KeyValue[] {
+  private static final  KeyValue [] kvs = new KeyValue[] {
         KeyValueTestUtil.create("R1", "cf", "a", 11, KeyValue.Type.Put, "dont-care"),
         KeyValueTestUtil.create("R1", "cf", "b", 11, KeyValue.Type.Put, "dont-care"),
         KeyValueTestUtil.create("R1", "cf", "c", 11, KeyValue.Type.Put, "dont-care"),
@@ -376,7 +376,11 @@ public class TestStoreScanner extends Te
         KeyValueTestUtil.create("R1", "cf", "i", 11, KeyValue.Type.Put, "dont-care"),
         KeyValueTestUtil.create("R2", "cf", "a", 11, KeyValue.Type.Put, "dont-care"),
     };
-    List<KeyValueScanner> scanners = scanFixture(kvs);
+
+  public void testSkipColumn() throws IOException {
+    KeyValueScanner [] scanners = new KeyValueScanner[] {
+        new KeyValueScanFixture(KeyValue.COMPARATOR, kvs)
+    };
     StoreScanner scan =
       new StoreScanner(new Scan(), CF, Long.MAX_VALUE, KeyValue.COMPARATOR,
           getCols("a", "d"), scanners);
@@ -395,9 +399,9 @@ public class TestStoreScanner extends Te
     results.clear();
     assertEquals(false, scan.next(results));
   }
-  
+
   /*
-   * Test expiration of KeyValues in combination with a configured TTL for 
+   * Test expiration of KeyValues in combination with a configured TTL for
    * a column family (as should be triggered in a major compaction).
    */
   public void testWildCardTtlScan() throws IOException {
@@ -435,6 +439,24 @@ public class TestStoreScanner extends Te
 
     assertEquals(false, scanner.next(results));
   }
+
+  public void testScannerReseekDoesntNPE() throws Exception {
+    KeyValueScanner [] scanners = new KeyValueScanner[] {
+        new KeyValueScanFixture(KeyValue.COMPARATOR, kvs)
+    };
+    StoreScanner scan =
+        new StoreScanner(new Scan(), CF, Long.MAX_VALUE, KeyValue.COMPARATOR,
+            getCols("a", "d"), scanners);
+
+
+    // Previously a updateReaders twice in a row would cause an NPE.  In test this would
also
+    // normally cause an NPE because scan.store is null.  So as long as we get through these
+    // two calls we are good and the bug was quashed.
+
+    scan.updateReaders();
+
+    scan.updateReaders();
+  }
     
   
   /**



Mime
View raw message