diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetaCache.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetaCache.java index 66ef546..e36a282 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetaCache.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetaCache.java @@ -36,7 +36,6 @@ import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.util.Bytes; -import org.apache.hadoop.hbase.CellComparator; /** * A cache implementation for region locations from meta. @@ -86,8 +85,7 @@ public class MetaCache { // checking is actually the last region in the table. byte[] endKey = possibleRegion.getRegionLocation().getRegionInfo().getEndKey(); if (Bytes.equals(endKey, HConstants.EMPTY_END_ROW) || - getRowComparator(tableName).compareRows( - endKey, 0, endKey.length, row, 0, row.length) > 0) { + Bytes.compareTo(endKey, 0, endKey.length, row, 0, row.length) > 0) { return possibleRegion; } @@ -95,10 +93,6 @@ public class MetaCache { return null; } - private CellComparator getRowComparator(TableName tableName) { - return TableName.META_TABLE_NAME.equals(tableName) ? CellComparator.META_COMPARATOR - : CellComparator.COMPARATOR; - } /** * Put a newly discovered HRegionLocation into the cache. * @param tableName The table name. diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestClientNoCluster.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestClientNoCluster.java index 29b32b3..66a80b0 100644 --- a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestClientNoCluster.java +++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestClientNoCluster.java @@ -39,7 +39,6 @@ import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.conf.Configured; -import org.apache.hadoop.hbase.CellComparator; import org.apache.hadoop.hbase.HBaseConfiguration; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.HRegionInfo; @@ -647,7 +646,7 @@ public class TestClientNoCluster extends Configured implements Tool { private final CellComparator delegate = CellComparator.META_COMPARATOR; @Override public int compare(byte[] left, byte[] right) { - return delegate.compareRows(left, 0, left.length, right, 0, right.length); + return delegate.compareRows(new KeyValue.KeyOnlyKeyValue(left), right, 0, right.length); } } diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparator.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparator.java index 67941bc..f9d0d09 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparator.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparator.java @@ -124,7 +124,7 @@ public class CellComparator implements Comparator, Serializable { if (!ignoreSequenceid) { // Negate following comparisons so later edits show up first // mvccVersion: later sorts first - return Longs.compare(b.getMvccVersion(), a.getMvccVersion()); + return Longs.compare(b.getSequenceId(), a.getSequenceId()); } else { return c; } @@ -399,33 +399,12 @@ public class CellComparator implements Comparator, Serializable { * @param right * @return 0 if both cells are equal, 1 if left cell is bigger than right, -1 otherwise */ - public final int compareRows(final Cell left, final Cell right) { - return compareRows(left.getRowArray(), left.getRowOffset(), left.getRowLength(), + public int compareRows(final Cell left, final Cell right) { + return Bytes.compareTo(left.getRowArray(), left.getRowOffset(), left.getRowLength(), right.getRowArray(), right.getRowOffset(), right.getRowLength()); } /** - * Compares the rows of two cells - * We explicitly pass the offset and length details of the cell to avoid re-parsing - * of the offset and length from the cell - * @param left the cell to be compared - * @param loffset the row offset of the left cell - * @param llength the row length of the left cell - * @param right the cell to be compared - * @param roffset the row offset of the right cell - * @param rlength the row length of the right cell - * @return 0 if both cells are equal, 1 if left cell is bigger than right, -1 otherwise - */ - private final int compareRows(Cell left, int loffset, int llength, Cell right, int roffset, - int rlength) { - // TODO : for BB based cells all the hasArray based checks would happen - // here. But we may have - // to end up in multiple APIs accepting byte[] and BBs - return compareRows(left.getRowArray(), loffset, llength, right.getRowArray(), roffset, - rlength); - } - - /** * Compares the row part of the cell with a simple plain byte[] like the * stopRow in Scan. This should be used with context where for hbase:meta * cells the {{@link #META_COMPARATOR} should be used @@ -441,26 +420,14 @@ public class CellComparator implements Comparator, Serializable { * @return 0 if both cell and the byte[] are equal, 1 if the cell is bigger * than byte[], -1 otherwise */ - public final int compareRows(Cell left, byte[] right, int roffset, + public int compareRows(Cell left, byte[] right, int roffset, int rlength) { // TODO : for BB based cells all the hasArray based checks would happen // here. But we may have // to end up in multiple APIs accepting byte[] and BBs - return compareRows(left.getRowArray(), left.getRowOffset(), left.getRowLength(), right, + return Bytes.compareTo(left.getRowArray(), left.getRowOffset(), left.getRowLength(), right, roffset, rlength); } - /** - * Do not use comparing rows from hbase:meta. Meta table Cells have schema (table,startrow,hash) - * so can't be treated as plain byte arrays as this method does. - */ - // TODO : CLEANUP : in order to do this we may have to modify some code - // HRegion.next() and will involve a - // Filter API change also. Better to do that later along with - // HBASE-11425/HBASE-13387. - public int compareRows(byte[] left, int loffset, int llength, byte[] right, int roffset, - int rlength) { - return Bytes.compareTo(left, loffset, llength, right, roffset, rlength); - } private static int compareWithoutRow(final Cell left, final Cell right) { // If the column is not specified, the "minimum" key type appears the @@ -542,11 +509,7 @@ public class CellComparator implements Comparator, Serializable { // compare a key against row/fam/qual/ts/type public final int compareKeyBasedOnColHint(Cell nextIndexedCell, Cell currentCell, int foff, int flen, byte[] colHint, int coff, int clen, long ts, byte type) { - - int compare = 0; - compare = compareRows(nextIndexedCell, nextIndexedCell.getRowOffset(), - nextIndexedCell.getRowLength(), currentCell, currentCell.getRowOffset(), - currentCell.getRowLength()); + int compare = compareRows(nextIndexedCell, currentCell); if (compare != 0) { return compare; } @@ -629,9 +592,20 @@ public class CellComparator implements Comparator, Serializable { * {@link KeyValue}s. */ public static class MetaCellComparator extends CellComparator { - + @Override - public int compareRows(byte[] left, int loffset, int llength, byte[] right, int roffset, + public int compareRows(final Cell left, final Cell right) { + return compareRows(left.getRowArray(), left.getRowOffset(), left.getRowLength(), + right.getRowArray(), right.getRowOffset(), right.getRowLength()); + } + + @Override + public int compareRows(Cell left, byte[] right, int roffset, int rlength) { + return compareRows(left.getRowArray(), left.getRowOffset(), left.getRowLength(), right, + roffset, rlength); + } + + private int compareRows(byte[] left, int loffset, int llength, byte[] right, int roffset, int rlength) { int leftDelimiter = Bytes.searchDelimiterIndex(left, loffset, llength, HConstants.DELIMITER); int rightDelimiter = Bytes @@ -662,7 +636,7 @@ public class CellComparator implements Comparator, Serializable { // Now compare middlesection of row. lpart = (leftFarDelimiter < 0 ? llength + loffset : leftFarDelimiter) - leftDelimiter; rpart = (rightFarDelimiter < 0 ? rlength + roffset : rightFarDelimiter) - rightDelimiter; - result = super.compareRows(left, leftDelimiter, lpart, right, rightDelimiter, rpart); + result = Bytes.compareTo(left, leftDelimiter, lpart, right, rightDelimiter, rpart); if (result != 0) { return result; } else { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/SyncTable.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/SyncTable.java index 3495ca9..20d6e24 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/SyncTable.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/SyncTable.java @@ -554,7 +554,6 @@ public class SyncTable extends Configured implements Tool { } } - private static final CellComparator cellComparator = new CellComparator(); /** * Compare row keys of the given Result objects. * Nulls are after non-nulls @@ -565,7 +564,9 @@ public class SyncTable extends Configured implements Tool { } else if (r2 == null) { return -1; // target missing row } else { - return cellComparator.compareRows(r1, 0, r1.length, r2, 0, r2.length); + // Sync on no META tables only. We can directly do what CellComparator is doing inside. + // Never the call going to MetaCellComparator. + return Bytes.compareTo(r1, 0, r1.length, r2, 0, r2.length); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeStoreFileManager.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeStoreFileManager.java index 4481a5f..45610fa 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeStoreFileManager.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeStoreFileManager.java @@ -36,6 +36,7 @@ import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.CellComparator; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.KeyValue; +import org.apache.hadoop.hbase.KeyValue.KeyOnlyKeyValue; import org.apache.hadoop.hbase.classification.InterfaceAudience; import org.apache.hadoop.hbase.regionserver.compactions.StripeCompactionPolicy; import org.apache.hadoop.hbase.util.Bytes; @@ -192,7 +193,7 @@ public class StripeStoreFileManager // level 0; we remove the stripe, and all subsequent ones, as soon as we find the // first one that cannot possibly have better candidates. if (!isInvalid(endKey) && !isOpen(endKey) - && (nonOpenRowCompare(endKey, targetKey.getRow()) <= 0)) { + && (nonOpenRowCompare(targetKey, endKey) >= 0)) { original.removeComponents(firstIrrelevant); break; } @@ -501,6 +502,10 @@ public class StripeStoreFileManager return key != null && key.length == 0; } + private static final boolean isOpen(Cell key) { + return key != null && key.getRowLength() == 0; + } + /** * Checks whether the key is invalid (e.g. from an L0 file, or non-stripe-compacted files). */ @@ -521,7 +526,12 @@ public class StripeStoreFileManager */ private final int nonOpenRowCompare(byte[] k1, byte[] k2) { assert !isOpen(k1) && !isOpen(k2); - return cellComparator.compareRows(k1, 0, k1.length, k2, 0, k2.length); + return cellComparator.compareRows(new KeyOnlyKeyValue(k1), k2, 0, k2.length); + } + + private final int nonOpenRowCompare(Cell k1, byte[] k2) { + assert !isOpen(k1) && !isOpen(k2); + return cellComparator.compareRows(k1, k2, 0, k2.length); } /**