diff --git a/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplitThread.java b/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplitThread.java index f78d691..a56dad2 100644 --- a/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplitThread.java +++ b/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplitThread.java @@ -149,7 +149,7 @@ class CompactSplitThread extends Thread { throws IOException { final HRegionInfo oldRegionInfo = region.getRegionInfo(); final long startTime = System.currentTimeMillis(); - final HRegion[] newRegions = region.splitRegion(midKey); + final HRegion [] newRegions = region.splitRegion(midKey); if (newRegions == null) { // Didn't need to be split return; diff --git a/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java index 06e022c..ee4eedb 100644 --- a/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java +++ b/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java @@ -613,7 +613,6 @@ public class HRegion implements HeapSize { // , Writable{ } /** @return the last time the region was flushed */ - @SuppressWarnings({"UnusedDeclaration"}) public long getLastFlushTime() { return this.lastFlushTime; } @@ -637,57 +636,71 @@ public class HRegion implements HeapSize { // , Writable{ return size; } - /* - * Split the HRegion to create two brand-new ones. This also closes - * current HRegion. Split should be fast since we don't rewrite store files - * but instead create new 'reference' store files that read off the top and - * bottom ranges of parent store files. - * @param splitRow row on which to split region - * @return two brand-new HRegions or null if a split is not needed + /** + * Prepare to run the split region "transaction". + * Caller must hold the {@link #splitLock}. If this method fails, no + * damage done, can continue. + * @param splitRow Row to split on. + * @return Daughter HRegionInfos or null if setup failed (region is closed, etc). * @throws IOException + * @see #executeSplitRegionTransaction + * @see #rollbackSplitRegionTransaction */ - HRegion [] splitRegion(final byte [] splitRow) throws IOException { + Pair prepareSplitRegionTransaction(final byte [] splitRow) + throws IOException { prepareToSplit(); - synchronized (splitLock) { - if (closed.get()) { - return null; - } - // Add start/end key checking: hbase-428. - byte [] startKey = this.regionInfo.getStartKey(); - byte [] endKey = this.regionInfo.getEndKey(); - if (this.comparator.matchingRows(startKey, 0, startKey.length, - splitRow, 0, splitRow.length)) { - LOG.debug("Startkey and midkey are same, not splitting"); - return null; - } - if (this.comparator.matchingRows(splitRow, 0, splitRow.length, - endKey, 0, endKey.length)) { - LOG.debug("Endkey and midkey are same, not splitting"); - return null; - } + if (closed.get()) { + return null; + } + // Add start/end key checking: hbase-428. + byte [] startKey = this.regionInfo.getStartKey(); + byte [] endKey = this.regionInfo.getEndKey(); + if (this.comparator.matchingRows(startKey, 0, startKey.length, + splitRow, 0, splitRow.length)) { + LOG.debug("Startkey and midkey are same, not splitting"); + return null; + } + if (this.comparator.matchingRows(splitRow, 0, splitRow.length, + endKey, 0, endKey.length)) { + LOG.debug("Endkey and midkey are same, not splitting"); + return null; + } + + // Calculate regionid to use. Can't be less than that of parent else + // it'll insert into wrong location over in .META. table: HBASE-710. + long rid = EnvironmentEdgeManager.currentTimeMillis(); + if (rid < this.regionInfo.getRegionId()) { + LOG.warn("Clock skew; parent regions id is " + + this.regionInfo.getRegionId() + " but current time here is " + rid); + rid = this.regionInfo.getRegionId() + 1; + } + HRegionInfo a = new HRegionInfo(this.regionInfo.getTableDesc(), + startKey, splitRow, false, rid); + HRegionInfo b = new HRegionInfo(this.regionInfo.getTableDesc(), + splitRow, endKey, false, rid); + return new Pair(a, b); + } + + /** + * Run the split region transaction. If we fail this method, call the + * {@link #rollbackSplitRegionTransaction()}. Changes in this method need + * to have rollback equivalent registered in {@link #rollbackSplitRegionTransaction()}. + * Caller must hold the {@link #splitLock}. + * @return two brand-new HRegions or null if a split is not needed + * @throws IOException + */ + HRegion [] executeSplitRegionTransaction(final Pair hris) + throws IOException { LOG.info("Starting split of region " + this); Path splits = new Path(this.regiondir, SPLITDIR); if(!this.fs.exists(splits)) { this.fs.mkdirs(splits); } - // Calculate regionid to use. Can't be less than that of parent else - // it'll insert into wrong location over in .META. table: HBASE-710. - long rid = EnvironmentEdgeManager.currentTimeMillis(); - if (rid < this.regionInfo.getRegionId()) { - LOG.warn("Clock skew; parent regions id is " + - this.regionInfo.getRegionId() + " but current time here is " + rid); - rid = this.regionInfo.getRegionId() + 1; - } - HRegionInfo regionAInfo = new HRegionInfo(this.regionInfo.getTableDesc(), - startKey, splitRow, false, rid); - Path dirA = getSplitDirForDaughter(splits, regionAInfo); - HRegionInfo regionBInfo = new HRegionInfo(this.regionInfo.getTableDesc(), - splitRow, endKey, false, rid); - Path dirB = getSplitDirForDaughter(splits, regionBInfo); + Path dirA = getSplitDirForDaughter(splits, hris.getFirst()); + Path dirB = getSplitDirForDaughter(splits, hris.getSecond()); // Now close the HRegion. Close returns all store files or null if not - // supposed to close (? What to do in this case? Implement abort of close?) - // Close also does wait on outstanding rows and calls a flush just-in-case. + // supposed to close. List hstoreFilesToSplit = close(false); if (hstoreFilesToSplit == null) { LOG.warn("Close came back null (Implement abort of close?)"); @@ -719,6 +732,22 @@ public class HRegion implements HeapSize { // , Writable{ } } + /** + * Undo failed {@link #executeSplitRegionTransaction()}. + * Caller must hold the {@link #splitLock}. If we fail, must shutdown the + * hosting regionserver. Otherwise, could be left with regions offlined or + * not correctly registered in .META. + * @throws IOException + */ + private void rollbackSplitRegionTransaction() throws IOException { + Path splits = new Path(this.regiondir, SPLITDIR); + if (this.fs.exists(splits)) { + if (!this.fs.delete(splits, true)) { + throw new IOException("Failed delete of " + splits); + } + } + } + /* * Get the daughter directories in the splits dir. The splits dir is under * the parent regions' directory. diff --git a/src/test/java/org/apache/hadoop/hbase/io/TestImmutableBytesWritable.java b/src/test/java/org/apache/hadoop/hbase/io/TestImmutableBytesWritable.java index 43fa6dd..77c4506 100644 --- a/src/test/java/org/apache/hadoop/hbase/io/TestImmutableBytesWritable.java +++ b/src/test/java/org/apache/hadoop/hbase/io/TestImmutableBytesWritable.java @@ -40,6 +40,13 @@ public class TestImmutableBytesWritable extends TestCase { new ImmutableBytesWritable(Bytes.toBytes("xxabc"), 2, 2).hashCode()); } + public void testSpecificCompare() { + ImmutableBytesWritable ibw1 = new ImmutableBytesWritable(new byte[]{0x0f}); + ImmutableBytesWritable ibw2 = new ImmutableBytesWritable(new byte[]{0x00, 0x00}); + ImmutableBytesWritable.Comparator c = new ImmutableBytesWritable.Comparator(); + assertFalse("ibw1 < ibw2", c.compare( ibw1, ibw2 ) < 0 ); + } + public void testComparison() throws Exception { runTests("aa", "b", -1); runTests("aa", "aa", 0);