From 886a03bc4d9fd36c50d9d07cdebb4606b845447d Mon Sep 17 00:00:00 2001 From: chenheng Date: Tue, 20 Oct 2015 01:03:24 +0800 Subject: [PATCH] HBASE-14643 Avoid Splits from once again opening a closed reader for fetching the first and last key --- .../hbase/regionserver/HRegionFileSystem.java | 73 ++++++++++--------- .../hbase/regionserver/SplitTransactionImpl.java | 82 ++++++++++++++++------ 2 files changed, 99 insertions(+), 56 deletions(-) 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 b16738f..fa162dd 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 @@ -30,6 +30,14 @@ import java.util.UUID; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.hadoop.hbase.Cell; +import org.apache.hadoop.hbase.CellComparator; +import org.apache.hadoop.hbase.HColumnDescriptor; +import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.HRegionInfo; +import org.apache.hadoop.hbase.HTableDescriptor; +import org.apache.hadoop.hbase.KeyValue; +import org.apache.hadoop.hbase.KeyValueUtil; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FSDataInputStream; @@ -39,13 +47,6 @@ import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.FileUtil; import org.apache.hadoop.fs.Path; import org.apache.hadoop.fs.permission.FsPermission; -import org.apache.hadoop.hbase.Cell; -import org.apache.hadoop.hbase.HColumnDescriptor; -import org.apache.hadoop.hbase.HConstants; -import org.apache.hadoop.hbase.HRegionInfo; -import org.apache.hadoop.hbase.HTableDescriptor; -import org.apache.hadoop.hbase.KeyValue; -import org.apache.hadoop.hbase.KeyValueUtil; import org.apache.hadoop.hbase.backup.HFileArchiver; import org.apache.hadoop.hbase.fs.HFileSystem; import org.apache.hadoop.hbase.io.Reference; @@ -579,43 +580,47 @@ public class HRegionFileSystem { Path splitStoreFile(final HRegionInfo hri, final String familyName, final StoreFile f, final byte[] splitRow, final boolean top, RegionSplitPolicy splitPolicy) throws IOException { + try { + return splitStoreFile(hri, familyName, f.getReader().getFirstKey(), f.getReader().getLastKey(), + splitRow, top, splitPolicy, f.getPath()); + } finally { + f.closeReader(true); + } + } + Path splitStoreFile(final HRegionInfo hri, final String familyName, final Cell firstKey, final Cell lastKey, + final byte[] splitRow, final boolean top, RegionSplitPolicy splitPolicy, + final Path storeFilePath) throws IOException { if (splitPolicy == null || !splitPolicy.skipStoreFileRangeCheck(familyName)) { // Check whether the split row lies in the range of the store file // If it is outside the range, return directly. - try { - if (top) { - //check if larger than last key. - KeyValue splitKey = KeyValueUtil.createFirstOnRow(splitRow); - Cell lastKey = f.createReader().getLastKey(); - // If lastKey is null means storefile is empty. - if (lastKey == null) { - return null; - } - if (f.getReader().getComparator().compare(splitKey, lastKey) > 0) { - return null; - } - } else { - //check if smaller than first key - KeyValue splitKey = KeyValueUtil.createLastOnRow(splitRow); - Cell firstKey = f.createReader().getFirstKey(); - // If firstKey is null means storefile is empty. - if (firstKey == null) { - return null; - } - if (f.getReader().getComparator().compare(splitKey, firstKey) < 0) { - return null; - } + if (top) { + //check if larger than last key. + KeyValue splitKey = KeyValueUtil.createFirstOnRow(splitRow); + // If lastKey is null means storefile is empty. + if (lastKey == null) { + return null; + } + if (CellComparator.COMPARATOR.compare(splitKey, lastKey) > 0) { + return null; + } + } else { + //check if smaller than first key + KeyValue splitKey = KeyValueUtil.createLastOnRow(splitRow); + // If firstKey is null means storefile is empty. + if (firstKey == null) { + return null; + } + if (CellComparator.COMPARATOR.compare(splitKey, firstKey) < 0) { + return null; } - } finally { - f.closeReader(true); } } Path splitDir = new Path(getSplitsDir(hri), familyName); // A reference to the bottom half of the hsf store file. Reference r = - top ? Reference.createTopReference(splitRow): Reference.createBottomReference(splitRow); + top ? Reference.createTopReference(splitRow): Reference.createBottomReference(splitRow); // Add the referred-to regions name as a dot separated suffix. // See REF_NAME_REGEX regex above. The referred-to regions name is // up in the path of the passed in f -- parentdir is family, @@ -623,7 +628,7 @@ public class HRegionFileSystem { String parentRegionName = regionInfoForFs.getEncodedName(); // Write reference with same file id only with the other region name as // suffix and into the new region location (under same family). - Path p = new Path(splitDir, f.getPath().getName() + "." + parentRegionName); + Path p = new Path(splitDir, storeFilePath.getName() + "." + parentRegionName); return r.write(fs, p); } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransactionImpl.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransactionImpl.java index fbfea8e..9666841 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransactionImpl.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransactionImpl.java @@ -25,6 +25,7 @@ import java.util.ArrayList; import java.util.List; import java.util.ListIterator; import java.util.Map; +import java.util.HashMap; import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; import java.util.concurrent.Executors; @@ -36,11 +37,12 @@ import java.util.concurrent.TimeUnit; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.fs.Path; -import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.HRegionInfo; import org.apache.hadoop.hbase.Server; import org.apache.hadoop.hbase.ServerName; +import org.apache.hadoop.hbase.Cell; +import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.client.Mutation; import org.apache.hadoop.hbase.client.Put; import org.apache.hadoop.hbase.protobuf.generated.RegionServerStatusProtos.RegionStateTransition.TransitionCode; @@ -125,6 +127,29 @@ public class SplitTransactionImpl implements SplitTransaction { */ private final ArrayList listeners = new ArrayList(); + private class StoreFileWrapper { + private StoreFile storeFile; + private Cell startKey; + private Cell endKey; + + public StoreFileWrapper(StoreFile sf, Cell startKey, Cell endKey) { + this.storeFile = sf; + this.startKey = startKey; + this.endKey = endKey; + } + + public StoreFile getStoreFile() { + return storeFile; + } + + public Cell getStartKey() { + return startKey; + } + + public Cell getEndKey() { + return endKey; + } + } /** * Constructor * @param r Region to split @@ -352,14 +377,25 @@ public class SplitTransactionImpl implements SplitTransaction { transition(SplitTransactionPhase.CREATE_SPLIT_DIR); - Map> hstoreFilesToSplit = null; + Map> storeFilesToSplit = new HashMap>(); Exception exceptionToThrow = null; try{ - hstoreFilesToSplit = this.parent.close(false); + this.parent.flush(true); + List stores = this.parent.getStores(); + for (Store store : stores) { + List storeFileWrappers = new ArrayList<>(); + + for (StoreFile storeFile : store.getStorefiles()) { + storeFileWrappers.add(new StoreFileWrapper(storeFile, + storeFile.getReader().getFirstKey(), storeFile.getReader().getLastKey())); + } + storeFilesToSplit.put(store.getFamily().getName(), storeFileWrappers); + } + this.parent.close(false); } catch (Exception e) { exceptionToThrow = e; } - if (exceptionToThrow == null && hstoreFilesToSplit == null) { + if (exceptionToThrow == null && storeFilesToSplit.isEmpty()) { // The region was closed by a concurrent thread. We can't continue // with the split, instead we must just abandon the split. If we // reopen or split this could cause problems because the region has @@ -386,7 +422,7 @@ public class SplitTransactionImpl implements SplitTransaction { // splitStoreFiles creates daughter region dirs under the parent splits dir // Nothing to unroll here if failure -- clean up of CREATE_SPLIT_DIR will // clean this up. - Pair expectedReferences = splitStoreFiles(hstoreFilesToSplit); + Pair expectedReferences = splitStoreFiles(storeFilesToSplit); // Log to the journal that we are creating region A, the first daughter // region. We could fail halfway through. If we do, we could have left @@ -637,9 +673,9 @@ public class SplitTransactionImpl implements SplitTransaction { * @throws IOException */ private Pair splitStoreFiles( - final Map> hstoreFilesToSplit) + final Map> storeFilesToSplit) throws IOException { - if (hstoreFilesToSplit == null) { + if (storeFilesToSplit.isEmpty()) { // Could be null because close didn't succeed -- for now consider it fatal throw new IOException("Close returned empty list of StoreFiles"); } @@ -647,7 +683,7 @@ public class SplitTransactionImpl implements SplitTransaction { // there's files to split. It then fires up everything, waits for // completion and finally checks for any exception int nbFiles = 0; - for (Map.Entry> entry: hstoreFilesToSplit.entrySet()) { + for (Map.Entry> entry: storeFilesToSplit.entrySet()) { nbFiles += entry.getValue().size(); } if (nbFiles == 0) { @@ -672,9 +708,9 @@ public class SplitTransactionImpl implements SplitTransaction { List>> futures = new ArrayList>> (nbFiles); // Split each store file. - for (Map.Entry> entry: hstoreFilesToSplit.entrySet()) { - for (StoreFile sf: entry.getValue()) { - StoreFileSplitter sfs = new StoreFileSplitter(entry.getKey(), sf); + for (Map.Entry> entry: storeFilesToSplit.entrySet()) { + for (StoreFileWrapper storeFileWrapper: entry.getValue()) { + StoreFileSplitter sfs = new StoreFileSplitter(entry.getKey(), storeFileWrapper); futures.add(threadPool.submit(sfs)); } } @@ -720,22 +756,24 @@ public class SplitTransactionImpl implements SplitTransaction { return new Pair(created_a, created_b); } - private Pair splitStoreFile(final byte[] family, final StoreFile sf) + private Pair splitStoreFile(final byte[] family, StoreFileWrapper storeFileWrapper) throws IOException { + Path storeFilePath = storeFileWrapper.getStoreFile().getPath(); if (LOG.isDebugEnabled()) { - LOG.debug("Splitting started for store file: " + sf.getPath() + " for region: " + - this.parent); + LOG.debug("Splitting started for store file: " + storeFilePath + " for region: " + this.parent); } HRegionFileSystem fs = this.parent.getRegionFileSystem(); String familyName = Bytes.toString(family); Path path_a = - fs.splitStoreFile(this.hri_a, familyName, sf, this.splitrow, false, - this.parent.getSplitPolicy()); + fs.splitStoreFile(this.hri_a, familyName, storeFileWrapper.getStartKey(), + storeFileWrapper.getEndKey(), this.splitrow, false, + this.parent.getSplitPolicy(), storeFilePath); Path path_b = - fs.splitStoreFile(this.hri_b, familyName, sf, this.splitrow, true, - this.parent.getSplitPolicy()); + fs.splitStoreFile(this.hri_b, familyName, storeFileWrapper.getStartKey(), + storeFileWrapper.getEndKey(), this.splitrow, true, + this.parent.getSplitPolicy(), storeFilePath); if (LOG.isDebugEnabled()) { - LOG.debug("Splitting complete for store file: " + sf.getPath() + " for region: " + + LOG.debug("Splitting complete for store file: " + storeFilePath + " for region: " + this.parent); } return new Pair(path_a, path_b); @@ -747,15 +785,15 @@ public class SplitTransactionImpl implements SplitTransaction { */ private class StoreFileSplitter implements Callable> { private final byte[] family; - private final StoreFile sf; + private final StoreFileWrapper sf; /** * Constructor that takes what it needs to split * @param family Family that contains the store file * @param sf which file */ - public StoreFileSplitter(final byte[] family, final StoreFile sf) { - this.sf = sf; + public StoreFileSplitter(final byte[] family, StoreFileWrapper storeFileWrapper) { + this.sf = storeFileWrapper; this.family = family; } -- 2.3.8 (Apple Git-58)