You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by ra...@apache.org on 2010/06/17 20:47:24 UTC
svn commit: r955704 - in /hbase/branches/0.20: CHANGES.txt
src/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
src/test/org/apache/hadoop/hbase/regionserver/TestStoreScanner.java
Author: rawson
Date: Thu Jun 17 18:47:23 2010
New Revision: 955704
URL: http://svn.apache.org/viewvc?rev=955704&view=rev
Log:
HBASE-2740 NPE in ReadWriteConsistencyControl
Modified:
hbase/branches/0.20/CHANGES.txt
hbase/branches/0.20/src/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
hbase/branches/0.20/src/test/org/apache/hadoop/hbase/regionserver/TestStoreScanner.java
Modified: hbase/branches/0.20/CHANGES.txt
URL: http://svn.apache.org/viewvc/hbase/branches/0.20/CHANGES.txt?rev=955704&r1=955703&r2=955704&view=diff
==============================================================================
--- hbase/branches/0.20/CHANGES.txt (original)
+++ hbase/branches/0.20/CHANGES.txt Thu Jun 17 18:47:23 2010
@@ -30,6 +30,7 @@ Release 0.20.5 - Sat Jun 11 17:13:00 PD
(Stack via Todd Lipcon)
HBASE-2712 Cached region location that went stale won't recover if
asking for first row
+ HBASE-2740 NPE in ReadWriteConsistencyControl
IMPROVEMENTS
HBASE-2567 [stargate] minimize differences between 0.20 branch and trunk
Modified: hbase/branches/0.20/src/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.20/src/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java?rev=955704&r1=955703&r2=955704&view=diff
==============================================================================
--- hbase/branches/0.20/src/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java (original)
+++ hbase/branches/0.20/src/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java Thu Jun 17 18:47:23 2010
@@ -34,7 +34,7 @@ import org.apache.hadoop.hbase.io.hfile.
import org.apache.hadoop.hbase.io.hfile.HFileScanner;
/**
- * 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 {
@@ -161,6 +161,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) {
@@ -279,6 +281,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/branches/0.20/src/test/org/apache/hadoop/hbase/regionserver/TestStoreScanner.java
URL: http://svn.apache.org/viewvc/hbase/branches/0.20/src/test/org/apache/hadoop/hbase/regionserver/TestStoreScanner.java?rev=955704&r1=955703&r2=955704&view=diff
==============================================================================
--- hbase/branches/0.20/src/test/org/apache/hadoop/hbase/regionserver/TestStoreScanner.java (original)
+++ hbase/branches/0.20/src/test/org/apache/hadoop/hbase/regionserver/TestStoreScanner.java Thu Jun 17 18:47:23 2010
@@ -49,7 +49,7 @@ public class TestStoreScanner extends Te
}
return cols;
}
-
+
public void testScanTimeRange() throws IOException {
String r1 = "R1";
// returns only 1 of these 2 even though same timestamp
@@ -101,7 +101,6 @@ public class TestStoreScanner extends Te
results = new ArrayList<KeyValue>();
assertEquals(true, scan.next(results));
assertEquals(3, results.size());
-
}
public void testScanSameTimestamp() throws IOException {
@@ -288,6 +287,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
@@ -322,6 +322,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"),
@@ -335,7 +336,7 @@ public class TestStoreScanner extends Te
KeyValueTestUtil.create("R1", "cf", "g", 11, KeyValue.Type.Delete, "dont-care"),
KeyValueTestUtil.create("R1", "cf", "h", 11, KeyValue.Type.Put, "dont-care"),
KeyValueTestUtil.create("R1", "cf", "i", 11, KeyValue.Type.Put, "dont-care"),
- KeyValueTestUtil.create("R2", "cf", "a", 11, KeyValue.Type.Put, "dont-care"),
+ KeyValueTestUtil.create("R2", "cf", "a", 11, KeyValue.Type.Put, "dont-care"),
};
KeyValueScanner [] scanners = new KeyValueScanner[] {
new KeyValueScanFixture(KeyValue.COMPARATOR, kvs)
@@ -372,8 +373,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"),
@@ -385,6 +385,8 @@ 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"),
};
+
+ public void testSkipColumn() throws IOException {
KeyValueScanner [] scanners = new KeyValueScanner[] {
new KeyValueScanFixture(KeyValue.COMPARATOR, kvs)
};
@@ -406,9 +408,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 {
@@ -438,14 +440,32 @@ public class TestStoreScanner extends Te
assertEquals(kvs[1], results.get(0));
assertEquals(kvs[2], results.get(1));
results.clear();
-
+
assertEquals(true, scanner.next(results));
assertEquals(3, results.size());
assertEquals(kvs[4], results.get(0));
assertEquals(kvs[5], results.get(1));
assertEquals(kvs[6], results.get(2));
results.clear();
-
+
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();
+ }
}