HBASE-13998 Remove CellComparator#compareRows(byte[] left, int loffset, int llength, byte[] right, int roffset, int rlength).

This commit is contained in:
anoopsjohn 2015-07-02 11:58:25 +05:30
parent 20e855f282
commit 62f5694491
5 changed files with 45 additions and 59 deletions

View File

@ -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.
@ -85,9 +84,16 @@ public class MetaCache {
// HConstants.EMPTY_END_ROW, signifying that the region we're
// checking is actually the last region in the table.
byte[] endKey = possibleRegion.getRegionLocation().getRegionInfo().getEndKey();
// Here we do direct Bytes.compareTo and not doing CellComparator/MetaCellComparator path.
// MetaCellComparator is for comparing against data in META table which need special handling.
// Not doing that is ok for this case because
// 1. We are getting the Region location for the given row in non META tables only. The compare
// checks the given row is within the end key of the found region. So META regions are not
// coming in here.
// 2. Even if META region comes in, its end key will be empty byte[] and so Bytes.equals(endKey,
// HConstants.EMPTY_END_ROW) check itself will pass.
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 +101,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.

View File

@ -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);
}
}

View File

@ -124,7 +124,7 @@ public class CellComparator implements Comparator<Cell>, 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,32 +399,11 @@ public class CellComparator implements Comparator<Cell>, 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
@ -441,26 +420,14 @@ public class CellComparator implements Comparator<Cell>, 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<Cell>, 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<Cell>, 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<Cell>, 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 {

View File

@ -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);
}
}

View File

@ -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);
}
/**