From 71f66bd591c670857b7aeca339c2f1bad3fef746 Mon Sep 17 00:00:00 2001 From: rahulgidwani Date: Wed, 25 Nov 2015 14:43:12 -0800 Subject: [PATCH] HBASE-14355 Scan different TimeRange for each column family - fixing memstore bug --- .../hadoop/hbase/regionserver/DefaultMemStore.java | 18 ++++++++---- .../hbase/regionserver/ScanQueryMatcher.java | 9 ++++-- .../hbase/regionserver/TestDefaultMemStore.java | 32 +++++++++++++--------- 3 files changed, 39 insertions(+), 20 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java index dfc5f67..89ae0d1 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/DefaultMemStore.java @@ -40,6 +40,7 @@ import org.apache.hadoop.hbase.KeyValue; import org.apache.hadoop.hbase.KeyValueUtil; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.client.Scan; +import org.apache.hadoop.hbase.io.TimeRange; import org.apache.hadoop.hbase.util.ByteRange; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.ClassSize; @@ -550,12 +551,19 @@ public class DefaultMemStore implements MemStore { /** * Check if this memstore may contain the required keys - * @param scan + * @param scan scan + * @param store holds reference to cf + * @param oldestUnexpiredTS * @return False if the key definitely does not exist in this Memstore */ - public boolean shouldSeek(Scan scan, long oldestUnexpiredTS) { - return (timeRangeTracker.includesTimeRange(scan.getTimeRange()) || - snapshotTimeRangeTracker.includesTimeRange(scan.getTimeRange())) + public boolean shouldSeek(Scan scan, Store store, long oldestUnexpiredTS) { + byte[] cf = store.getFamily().getName(); + TimeRange timeRange = scan.getColumnFamilyTimeRange().get(cf); + if (timeRange == null) { + timeRange = scan.getTimeRange(); + } + return (timeRangeTracker.includesTimeRange(timeRange) || + snapshotTimeRangeTracker.includesTimeRange(timeRange)) && (Math.max(timeRangeTracker.getMaximumTimestamp(), snapshotTimeRangeTracker.getMaximumTimestamp()) >= oldestUnexpiredTS); @@ -828,7 +836,7 @@ public class DefaultMemStore implements MemStore { @Override public boolean shouldUseScanner(Scan scan, Store store, long oldestUnexpiredTS) { - return shouldSeek(scan, oldestUnexpiredTS); + return shouldSeek(scan, store, oldestUnexpiredTS); } /** diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java index 0e9e637..47d8c8f 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java @@ -160,7 +160,12 @@ public class ScanQueryMatcher { public ScanQueryMatcher(Scan scan, ScanInfo scanInfo, NavigableSet columns, ScanType scanType, long readPointToUse, long earliestPutTs, long oldestUnexpiredTS, long now, RegionCoprocessorHost regionCoprocessorHost) throws IOException { - this.tr = scan.getTimeRange(); + TimeRange timeRange = scan.getColumnFamilyTimeRange().get(scanInfo.getFamily()); + if (timeRange == null) { + this.tr = scan.getTimeRange(); + } else { + this.tr = timeRange; + } this.rowComparator = scanInfo.getComparator(); this.regionCoprocessorHost = regionCoprocessorHost; this.deletes = instantiateDeleteTracker(); @@ -275,7 +280,7 @@ public class ScanQueryMatcher { * caused by a data corruption. */ public MatchCode match(Cell cell) throws IOException { - if (filter != null && filter.filterAllRemaining()) { + if (filter != null && filter.filterAllRemaining()) { return MatchCode.DONE_SCAN; } int ret = this.rowComparator.compareRows(curCell, cell); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java index f1e20d1..ec70740 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestDefaultMemStore.java @@ -61,6 +61,9 @@ import com.google.common.base.Joiner; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + /** memstore test case */ @Category({RegionServerTests.class, MediumTests.class}) public class TestDefaultMemStore extends TestCase { @@ -745,29 +748,32 @@ public class TestDefaultMemStore extends TestCase { /** * Test to ensure correctness when using Memstore with multiple timestamps */ - public void testMultipleTimestamps() throws IOException { + public void testMultipleTimestamps() throws Exception { long[] timestamps = new long[] {20,10,5,1}; Scan scan = new Scan(); for (long timestamp: timestamps) addRows(memstore,timestamp); - scan.setTimeRange(0, 2); - assertTrue(memstore.shouldSeek(scan, Long.MIN_VALUE)); + byte[] fam = Bytes.toBytes("fam"); + HColumnDescriptor hcd = mock(HColumnDescriptor.class); + when(hcd.getName()).thenReturn(fam); + Store store = mock(Store.class); + when(store.getFamily()).thenReturn(hcd); + scan.setColumnFamilyTimeRange(fam, 0, 2); + assertTrue(memstore.shouldSeek(scan, store, Long.MIN_VALUE)); - scan.setTimeRange(20, 82); - assertTrue(memstore.shouldSeek(scan, Long.MIN_VALUE)); + scan.setColumnFamilyTimeRange(fam, 20, 82); + assertTrue(memstore.shouldSeek(scan, store, Long.MIN_VALUE)); - scan.setTimeRange(10, 20); - assertTrue(memstore.shouldSeek(scan, Long.MIN_VALUE)); + scan.setColumnFamilyTimeRange(fam, 10, 20); + assertTrue(memstore.shouldSeek(scan, store, Long.MIN_VALUE)); - scan.setTimeRange(8, 12); - assertTrue(memstore.shouldSeek(scan, Long.MIN_VALUE)); + scan.setColumnFamilyTimeRange(fam, 8, 12); + assertTrue(memstore.shouldSeek(scan, store, Long.MIN_VALUE)); - /*This test is not required for correctness but it should pass when - * timestamp range optimization is on*/ - //scan.setTimeRange(28, 42); - //assertTrue(!memstore.shouldSeek(scan)); + scan.setColumnFamilyTimeRange(fam, 28, 42); + assertTrue(!memstore.shouldSeek(scan, store, Long.MIN_VALUE)); } //////////////////////////////////// -- 2.1.0