You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by an...@apache.org on 2015/07/02 08:28:43 UTC
hbase git commit: HBASE-13998 Remove
CellComparator#compareRows(byte[] left, int loffset, int llength, byte[] right,
int roffset, int rlength).
Repository: hbase
Updated Branches:
refs/heads/master 20e855f28 -> 62f569449
HBASE-13998 Remove CellComparator#compareRows(byte[] left, int loffset, int llength, byte[] right, int roffset, int rlength).
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/62f56944
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/62f56944
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/62f56944
Branch: refs/heads/master
Commit: 62f56944919b436036dcac740d8a21c56289a164
Parents: 20e855f
Author: anoopsjohn <an...@gmail.com>
Authored: Thu Jul 2 11:58:25 2015 +0530
Committer: anoopsjohn <an...@gmail.com>
Committed: Thu Jul 2 11:58:25 2015 +0530
----------------------------------------------------------------------
.../apache/hadoop/hbase/client/MetaCache.java | 16 ++---
.../hbase/client/TestClientNoCluster.java | 3 +-
.../org/apache/hadoop/hbase/CellComparator.java | 66 ++++++--------------
.../hadoop/hbase/mapreduce/SyncTable.java | 5 +-
.../regionserver/StripeStoreFileManager.java | 14 ++++-
5 files changed, 45 insertions(+), 59 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/hbase/blob/62f56944/hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetaCache.java
----------------------------------------------------------------------
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..8e1c93c 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.
@@ -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.
http://git-wip-us.apache.org/repos/asf/hbase/blob/62f56944/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestClientNoCluster.java
----------------------------------------------------------------------
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);
}
}
http://git-wip-us.apache.org/repos/asf/hbase/blob/62f56944/hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparator.java
----------------------------------------------------------------------
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<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,33 +399,12 @@ 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
* cells the {{@link #META_COMPARATOR} should be used
@@ -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 {
http://git-wip-us.apache.org/repos/asf/hbase/blob/62f56944/hbase-server/src/main/java/org/apache/hadoop/hbase/mapreduce/SyncTable.java
----------------------------------------------------------------------
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);
}
}
http://git-wip-us.apache.org/repos/asf/hbase/blob/62f56944/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeStoreFileManager.java
----------------------------------------------------------------------
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);
}
/**