From 08a25598511998e0d7da5904c210db96b1e0ca16 Mon Sep 17 00:00:00 2001 From: Rajeshbabu Chintaguntla Date: Tue, 9 Dec 2014 19:00:37 +0530 Subject: [PATCH] HBASE-12583 Allow creating reference files even the split row not lies in the storefile range if required(Rajeshbabu) --- .../hadoop/hbase/regionserver/HRegion.java | 7 +++ .../hbase/regionserver/HRegionFileSystem.java | 45 ++++++++++--------- .../hbase/regionserver/RegionSplitPolicy.java | 13 ++++++ .../hbase/regionserver/SplitTransaction.java | 8 +++- .../TestSplitTransactionOnCluster.java | 45 +++++++++++++++++++ .../hbase/regionserver/TestStoreFile.java | 2 +- 6 files changed, 96 insertions(+), 24 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java index 1a7fa19e4d9..4989bd592ef 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java @@ -6715,4 +6715,11 @@ public class HRegion implements HeapSize, PropagatingConfigurationObserver { // configurationManager.get().deregisterObserver(s); } } + + /** + * @return split policy for this region. + */ + public RegionSplitPolicy getSplitPolicy() { + return this.splitPolicy; + } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionFileSystem.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionFileSystem.java index 4f728aee1f2..599572e026d 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionFileSystem.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionFileSystem.java @@ -568,32 +568,35 @@ public class HRegionFileSystem { * @param splitRow Split Row * @param top True if we are referring to the top half of the hfile. * @return Path to created reference. + * @param splitPolicy * @throws IOException */ - Path splitStoreFile(final HRegionInfo hri, final String familyName, - final StoreFile f, final byte[] splitRow, final boolean top) throws IOException { + Path splitStoreFile(final HRegionInfo hri, final String familyName, final StoreFile f, + final byte[] splitRow, final boolean top, RegionSplitPolicy splitPolicy) throws IOException { - // Check whether the split row lies in the range of the store file - // If it is outside the range, return directly. - if (top) { - //check if larger than last key. - KeyValue splitKey = KeyValueUtil.createFirstOnRow(splitRow); - byte[] lastKey = f.createReader().getLastKey(); - // If lastKey is null means storefile is empty. - if (lastKey == null) return null; - if (f.getReader().getComparator().compareFlatKey(splitKey.getBuffer(), + if (splitPolicy == null || !splitPolicy.skipStoreFileRangeCheck()) { + // Check whether the split row lies in the range of the store file + // If it is outside the range, return directly. + if (top) { + //check if larger than last key. + KeyValue splitKey = KeyValueUtil.createFirstOnRow(splitRow); + byte[] lastKey = f.createReader().getLastKey(); + // If lastKey is null means storefile is empty. + if (lastKey == null) return null; + if (f.getReader().getComparator().compareFlatKey(splitKey.getBuffer(), splitKey.getKeyOffset(), splitKey.getKeyLength(), lastKey, 0, lastKey.length) > 0) { - return null; - } - } else { - //check if smaller than first key - KeyValue splitKey = KeyValueUtil.createLastOnRow(splitRow); - byte[] firstKey = f.createReader().getFirstKey(); - // If firstKey is null means storefile is empty. - if (firstKey == null) return null; - if (f.getReader().getComparator().compareFlatKey(splitKey.getBuffer(), + return null; + } + } else { + //check if smaller than first key + KeyValue splitKey = KeyValueUtil.createLastOnRow(splitRow); + byte[] firstKey = f.createReader().getFirstKey(); + // If firstKey is null means storefile is empty. + if (firstKey == null) return null; + if (f.getReader().getComparator().compareFlatKey(splitKey.getBuffer(), splitKey.getKeyOffset(), splitKey.getKeyLength(), firstKey, 0, firstKey.length) < 0) { - return null; + return null; + } } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionSplitPolicy.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionSplitPolicy.java index 53979af8a0b..66883d60a3b 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionSplitPolicy.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionSplitPolicy.java @@ -125,4 +125,17 @@ public abstract class RegionSplitPolicy extends Configured { e); } } + + /** + * In {@link HRegionFileSystem#splitStoreFile(org.apache.hadoop.hbase.HRegionInfo, + * String, StoreFile, byte[], boolean)} we are not creating the split reference if split row + * not lies in the StoreFile range. But some use cases we may need to create the split reference + * even the split row not lies in the range. + * This method can be used to whether to skip the the StoreRile range check or not. + * @return whether to skip the StoreFile range check or or not + */ + protected boolean skipStoreFileRangeCheck() { + return false; + } + } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java index d28f4771a85..06e726f6949 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java @@ -753,8 +753,12 @@ public class SplitTransaction { HRegionFileSystem fs = this.parent.getRegionFileSystem(); String familyName = Bytes.toString(family); - Path path_a = fs.splitStoreFile(this.hri_a, familyName, sf, this.splitrow, false); - Path path_b = fs.splitStoreFile(this.hri_b, familyName, sf, this.splitrow, true); + Path path_a = + fs.splitStoreFile(this.hri_a, familyName, sf, this.splitrow, false, + this.parent.getSplitPolicy()); + Path path_b = + fs.splitStoreFile(this.hri_b, familyName, sf, this.splitrow, true, + this.parent.getSplitPolicy()); return new Pair(path_a, path_b); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java index 19f30d8d8ca..ee9cd407e27 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java @@ -28,6 +28,7 @@ import static org.junit.Assert.fail; import java.io.IOException; import java.io.InterruptedIOException; +import java.util.Collection; import java.util.List; import java.util.Map; import java.util.concurrent.CountDownLatch; @@ -1130,6 +1131,37 @@ public class TestSplitTransactionOnCluster { TESTING_UTIL.deleteTable(tableName); } } + + @Test + public void testStoreFileReferenceCreationWhenSplitPolicySaysToSkipRangeCheck() + throws Exception { + final TableName tableName = + TableName.valueOf("testStoreFileReferenceCreationWhenSplitPolicySaysToSkipRangeCheck"); + try { + HTableDescriptor htd = new HTableDescriptor(tableName); + htd.addFamily(new HColumnDescriptor("f")); + htd.setRegionSplitPolicyClassName(CustomSplitPolicy.class.getName()); + admin.createTable(htd); + List regions = awaitTableRegions(tableName); + HRegion region = regions.get(0); + for(int i = 3;i<9;i++) { + Put p = new Put(Bytes.toBytes("row"+i)); + p.add(Bytes.toBytes("f"), Bytes.toBytes("q"), Bytes.toBytes("value"+i)); + region.put(p); + } + region.flushcache(); + Store store = region.getStore(Bytes.toBytes("f")); + Collection storefiles = store.getStorefiles(); + assertEquals(storefiles.size(), 1); + assertFalse(region.hasReferences()); + Path referencePath = region.getRegionFileSystem().splitStoreFile(region.getRegionInfo(), "f", + storefiles.iterator().next(), Bytes.toBytes("row1"), false, region.getSplitPolicy()); + assertNotNull(referencePath); + } finally { + TESTING_UTIL.deleteTable(tableName); + } + } + public static class MockedCoordinatedStateManager extends ZkCoordinatedStateManager { public void initialize(Server server, HRegion region) { @@ -1445,5 +1477,18 @@ public class TestSplitTransactionOnCluster { } } + + static class CustomSplitPolicy extends RegionSplitPolicy { + + @Override + protected boolean shouldSplit() { + return true; + } + + @Override + public boolean skipStoreFileRangeCheck() { + return true; + } + } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java index 2072623914d..36a7f77539c 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestStoreFile.java @@ -940,7 +940,7 @@ public class TestStoreFile extends HBaseTestCase { final String family, final StoreFile sf, final byte[] splitKey, boolean isTopRef) throws IOException { FileSystem fs = regionFs.getFileSystem(); - Path path = regionFs.splitStoreFile(hri, family, sf, splitKey, isTopRef); + Path path = regionFs.splitStoreFile(hri, family, sf, splitKey, isTopRef, null); if (null == path) { return null; }