diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ReversedStoreScanner.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ReversedStoreScanner.java index 7bd580d..ede8e26 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ReversedStoreScanner.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ReversedStoreScanner.java @@ -121,25 +121,14 @@ class ReversedStoreScanner extends StoreScanner implements KeyValueScanner { } @Override - public boolean seekToPreviousRow(KeyValue key) throws IOException { - lock.lock(); - try { + synchronized public boolean seekToPreviousRow(KeyValue key) throws IOException { checkReseek(); return this.heap.seekToPreviousRow(key); - } finally { - lock.unlock(); - } - } @Override - public boolean backwardSeek(KeyValue key) throws IOException { - lock.lock(); - try { + synchronized public boolean backwardSeek(KeyValue key) throws IOException { checkReseek(); return this.heap.backwardSeek(key); - } finally { - lock.unlock(); - } } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java index fc034ed..7070ad1 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java @@ -25,7 +25,6 @@ import java.util.ArrayList; import java.util.List; import java.util.NavigableSet; import java.util.concurrent.CountDownLatch; -import java.util.concurrent.locks.ReentrantLock; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -101,7 +100,6 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner // A flag whether use pread for scan private boolean scanUsePread = false; - protected ReentrantLock lock = new ReentrantLock(); private final long readPt; @@ -380,16 +378,11 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner } @Override - public KeyValue peek() { - lock.lock(); - try { + synchronized public KeyValue peek() { if (this.heap == null) { return this.lastTop; } return this.heap.peek(); - } finally { - lock.unlock(); - } } @Override @@ -399,9 +392,7 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner } @Override - public void close() { - lock.lock(); - try { + synchronized public void close() { if (this.closing) return; this.closing = true; // under test, we dont have a this.store @@ -411,21 +402,13 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner this.heap.close(); this.heap = null; // CLOSED! this.lastTop = null; // If both are null, we are closed. - } finally { - lock.unlock(); - } } @Override - public boolean seek(KeyValue key) throws IOException { - lock.lock(); - try { + synchronized public boolean seek(KeyValue key) throws IOException { // reset matcher state, in case that underlying store changed checkReseek(); return this.heap.seek(key); - } finally { - lock.unlock(); - } } /** @@ -435,9 +418,7 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner * @return true if there are more rows, false if scanner is done */ @Override - public boolean next(List outResult, int limit) throws IOException { - lock.lock(); - try { + synchronized public boolean next(List outResult, int limit) throws IOException { if (checkReseek()) { return true; } @@ -570,9 +551,6 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner // No more keys close(); return false; - } finally { - lock.unlock(); - } } /* @@ -614,9 +592,7 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner // Implementation of ChangedReadersObserver @Override - public void updateReaders() throws IOException { - lock.lock(); - try { + synchronized public void updateReaders() throws IOException { if (this.closing) return; // All public synchronized API calls will call 'checkReseek' which will cause @@ -636,9 +612,6 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner this.heap = null; // the re-seeks could be slow (access HDFS) free up memory ASAP // Let the next() call handle re-creating and seeking - } finally { - lock.unlock(); - } } /** @@ -727,9 +700,7 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner } @Override - public boolean reseek(KeyValue kv) throws IOException { - lock.lock(); - try { + synchronized public boolean reseek(KeyValue kv) throws IOException { //Heap will not be null, if this is called from next() which. //If called from RegionScanner.reseek(...) make sure the scanner //stack is reset if needed. @@ -738,9 +709,6 @@ public class StoreScanner extends NonReversedNonLazyKeyValueScanner return heap.requestSeek(kv, true, useRowColBloom); } return heap.reseek(kv); - } finally { - lock.unlock(); - } } @Override diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanFilterPerformance.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanFilterPerformance.java new file mode 100644 index 0000000..d52fda1 --- /dev/null +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestScanFilterPerformance.java @@ -0,0 +1,83 @@ +package org.apache.hadoop.hbase.regionserver; + +import java.io.IOException; + +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; +import org.apache.hadoop.hbase.Cell; +import org.apache.hadoop.hbase.HBaseTestingUtility; +import org.apache.hadoop.hbase.KeyValue; +import org.apache.hadoop.hbase.client.HTable; +import org.apache.hadoop.hbase.client.Put; +import org.apache.hadoop.hbase.client.Result; +import org.apache.hadoop.hbase.client.ResultScanner; +import org.apache.hadoop.hbase.client.Scan; +import org.apache.hadoop.hbase.filter.BinaryComparator; +import org.apache.hadoop.hbase.filter.CompareFilter.CompareOp; +import org.apache.hadoop.hbase.filter.Filter; +import org.apache.hadoop.hbase.filter.ValueFilter; +import org.apache.hadoop.hbase.util.Bytes; +import org.junit.Test; + +import static org.junit.Assert.fail; + + +public class TestScanFilterPerformance { + private static byte[] TAB = Bytes.toBytes("tallTable"); + private static byte[] FAM = Bytes.toBytes("f"); + + @Test + public void testScanFilterPerformance() throws Exception { + HBaseTestingUtility util = new HBaseTestingUtility(); + util.startMiniCluster(1); + HTable table = util.createTable(TAB, FAM); + seedTable(table); + util.getHBaseAdmin().flush(TAB); + + // warmup + scanTable(table); + scanTable(table); + + int N = 10; + long[] runTimes = new long[N]; + long sum = 0; + for (int i=0; i