HBASE-19428 Deprecate the compareTo(Row)
Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/e23f7afe Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/e23f7afe Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/e23f7afe Branch: refs/heads/HBASE-19397 Commit: e23f7afe5791c248f1ccdf031788ffc6df2c263b Parents: 8d0da1a Author: Chia-Ping Tsai <[email protected]> Authored: Fri Dec 29 20:03:39 2017 +0800 Committer: Chia-Ping Tsai <[email protected]> Committed: Fri Dec 29 20:03:39 2017 +0800 ---------------------------------------------------------------------- .../org/apache/hadoop/hbase/client/Delete.java | 2 +- .../org/apache/hadoop/hbase/client/Get.java | 3 +- .../apache/hadoop/hbase/client/Increment.java | 18 ++-- .../apache/hadoop/hbase/client/Mutation.java | 5 ++ .../org/apache/hadoop/hbase/client/Put.java | 2 +- .../client/RegionCoprocessorServiceExec.java | 3 +- .../org/apache/hadoop/hbase/client/Row.java | 10 +++ .../hadoop/hbase/client/RowMutations.java | 17 +++- .../hadoop/hbase/client/TestRowComparator.java | 94 ++++++++++++++++++++ .../hbase/regionserver/RSRpcServices.java | 5 +- 10 files changed, 143 insertions(+), 16 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/e23f7afe/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Delete.java ---------------------------------------------------------------------- diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Delete.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Delete.java index 33389cf..7c32a68 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Delete.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Delete.java @@ -65,7 +65,7 @@ import org.apache.yetus.audience.InterfaceAudience; * timestamp. The constructor timestamp is not referenced. */ @InterfaceAudience.Public -public class Delete extends Mutation implements Comparable<Row> { +public class Delete extends Mutation { /** * Create a Delete operation for the specified row. * <p> http://git-wip-us.apache.org/repos/asf/hbase/blob/e23f7afe/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Get.java ---------------------------------------------------------------------- diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Get.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Get.java index 80b8a22..9ed3b38 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Get.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Get.java @@ -64,8 +64,7 @@ import org.apache.hadoop.hbase.util.Bytes; * To add a filter, call {@link #setFilter(Filter) setFilter}. */ @InterfaceAudience.Public -public class Get extends Query - implements Row, Comparable<Row> { +public class Get extends Query implements Row { private static final Logger LOG = LoggerFactory.getLogger(Get.class); private byte [] row = null; http://git-wip-us.apache.org/repos/asf/hbase/blob/e23f7afe/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Increment.java ---------------------------------------------------------------------- diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Increment.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Increment.java index 5ab5b85..76208d6 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Increment.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Increment.java @@ -46,7 +46,7 @@ import org.apache.yetus.audience.InterfaceAudience; * {@link #addColumn(byte[], byte[], long)} method. */ @InterfaceAudience.Public -public class Increment extends Mutation implements Comparable<Row> { +public class Increment extends Mutation { private static final int HEAP_OVERHEAD = ClassSize.REFERENCE + ClassSize.TIMERANGE; private TimeRange tr = new TimeRange(); @@ -262,12 +262,11 @@ public class Increment extends Mutation implements Comparable<Row> { return sb.toString(); } - @Override - public int compareTo(Row i) { - // TODO: This is wrong. Can't have two the same just because on same row. - return Bytes.compareTo(this.getRow(), i.getRow()); - } - + /** + * @deprecated As of release 2.0.0, this will be removed in HBase 3.0.0. + * No replacement. + */ + @Deprecated @Override public int hashCode() { // TODO: This is wrong. Can't have two gets the same just because on same row. But it @@ -275,6 +274,11 @@ public class Increment extends Mutation implements Comparable<Row> { return Bytes.hashCode(this.getRow()); } + /** + * @deprecated As of release 2.0.0, this will be removed in HBase 3.0.0. + * Use {@link Row#COMPARATOR} instead + */ + @Deprecated @Override public boolean equals(Object obj) { // TODO: This is wrong. Can't have two the same just because on same row. http://git-wip-us.apache.org/repos/asf/hbase/blob/e23f7afe/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Mutation.java ---------------------------------------------------------------------- diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Mutation.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Mutation.java index b7c1769..1569d26 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Mutation.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Mutation.java @@ -331,6 +331,11 @@ public abstract class Mutation extends OperationWithAttributes implements Row, C return this.row; } + /** + * @deprecated As of release 2.0.0, this will be removed in HBase 3.0.0. + * Use {@link Row#COMPARATOR} instead + */ + @Deprecated @Override public int compareTo(final Row d) { return Bytes.compareTo(this.getRow(), d.getRow()); http://git-wip-us.apache.org/repos/asf/hbase/blob/e23f7afe/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Put.java ---------------------------------------------------------------------- diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Put.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Put.java index b817d70..db8eec5 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Put.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Put.java @@ -44,7 +44,7 @@ import org.apache.yetus.audience.InterfaceAudience; * setting the timestamp. */ @InterfaceAudience.Public -public class Put extends Mutation implements HeapSize, Comparable<Row> { +public class Put extends Mutation implements HeapSize { /** * Create a Put operation for the specified row. * @param row row key http://git-wip-us.apache.org/repos/asf/hbase/blob/e23f7afe/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionCoprocessorServiceExec.java ---------------------------------------------------------------------- diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionCoprocessorServiceExec.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionCoprocessorServiceExec.java index 4a6ee14..68a6927 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionCoprocessorServiceExec.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionCoprocessorServiceExec.java @@ -98,8 +98,7 @@ public class RegionCoprocessorServiceExec implements Row { if (obj == null || getClass() != obj.getClass()) { return false; } - Row other = (Row) obj; - return compareTo(other) == 0; + return compareTo((RegionCoprocessorServiceExec) obj) == 0; } @Override http://git-wip-us.apache.org/repos/asf/hbase/blob/e23f7afe/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Row.java ---------------------------------------------------------------------- diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Row.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Row.java index 6565674..3152f9e 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Row.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Row.java @@ -18,6 +18,8 @@ */ package org.apache.hadoop.hbase.client; +import java.util.Comparator; +import org.apache.hadoop.hbase.util.Bytes; import org.apache.yetus.audience.InterfaceAudience; /** @@ -25,8 +27,16 @@ import org.apache.yetus.audience.InterfaceAudience; */ @InterfaceAudience.Public public interface Row extends Comparable<Row> { + Comparator<Row> COMPARATOR = (v1, v2) -> Bytes.compareTo(v1.getRow(), v2.getRow()); /** * @return The row. */ byte [] getRow(); + + /** + * @deprecated As of release 2.0.0, this will be removed in HBase 3.0.0. + * Use {@link Row#COMPARATOR} instead + */ + @Deprecated + int compareTo(Row var1); } http://git-wip-us.apache.org/repos/asf/hbase/blob/e23f7afe/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RowMutations.java ---------------------------------------------------------------------- diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RowMutations.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RowMutations.java index 7320c33..4ff9eb1 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RowMutations.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RowMutations.java @@ -32,7 +32,7 @@ import org.apache.hadoop.hbase.util.Bytes; * * The mutations are performed in the order in which they * were added. - * + * * <p>We compare and equate mutations based off their row so be careful putting RowMutations * into Sets or using them as keys in Maps. */ @@ -87,11 +87,21 @@ public class RowMutations implements Row { mutations.add(m); } + /** + * @deprecated As of release 2.0.0, this will be removed in HBase 3.0.0. + * Use {@link Row#COMPARATOR} instead + */ + @Deprecated @Override public int compareTo(Row i) { return Bytes.compareTo(this.getRow(), i.getRow()); } + /** + * @deprecated As of release 2.0.0, this will be removed in HBase 3.0.0. + * No replacement + */ + @Deprecated @Override public boolean equals(Object obj) { if (obj == this) return true; @@ -102,6 +112,11 @@ public class RowMutations implements Row { return false; } + /** + * @deprecated As of release 2.0.0, this will be removed in HBase 3.0.0. + * No replacement + */ + @Deprecated @Override public int hashCode(){ return Arrays.hashCode(row); http://git-wip-us.apache.org/repos/asf/hbase/blob/e23f7afe/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestRowComparator.java ---------------------------------------------------------------------- diff --git a/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestRowComparator.java b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestRowComparator.java new file mode 100644 index 0000000..72c1b3e --- /dev/null +++ b/hbase-client/src/test/java/org/apache/hadoop/hbase/client/TestRowComparator.java @@ -0,0 +1,94 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase.client; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.function.Function; +import java.util.stream.Collectors; +import java.util.stream.IntStream; +import org.apache.hadoop.hbase.testclassification.ClientTests; +import org.apache.hadoop.hbase.testclassification.SmallTests; +import org.apache.hadoop.hbase.util.Bytes; +import org.junit.Test; +import org.junit.experimental.categories.Category; + +@Category({ClientTests.class, SmallTests.class}) +public class TestRowComparator { + private static final List<byte[]> DEFAULT_ROWS = IntStream.range(1, 9) + .mapToObj(String::valueOf).map(Bytes::toBytes).collect(Collectors.toList()); + + @Test + public void testPut() { + test(row -> new Put(row)); + } + + @Test + public void testDelete() { + test(row -> new Delete(row)); + } + + @Test + public void testAppend() { + test(row -> new Append(row)); + } + + @Test + public void testIncrement() { + test(row -> new Increment(row)); + } + + @Test + public void testGet() { + test(row -> new Get(row)); + } + + private static <T extends Row> void test(Function<byte[], T> f) { + List<T> rows = new ArrayList<T>(DEFAULT_ROWS.stream() + .map(f).collect(Collectors.toList())); + do { + Collections.shuffle(rows); + } while (needShuffle(rows)); + Collections.sort(rows, Row.COMPARATOR); + assertSort(rows); + } + + private static boolean needShuffle(List<? extends Row> rows) { + assertFalse(rows.isEmpty()); + assertEquals(DEFAULT_ROWS.size(), rows.size()); + for (int i = 0; i != DEFAULT_ROWS.size(); ++i) { + if (!Bytes.equals(DEFAULT_ROWS.get(i), rows.get(i).getRow())) { + return false; + } + } + return true; + } + + private static void assertSort(List<? extends Row> rows) { + assertFalse(rows.isEmpty()); + assertEquals(DEFAULT_ROWS.size(), rows.size()); + for (int i = 0; i != DEFAULT_ROWS.size(); ++i) { + assertTrue(Bytes.equals(DEFAULT_ROWS.get(i), rows.get(i).getRow())); + } + } +} http://git-wip-us.apache.org/repos/asf/hbase/blob/e23f7afe/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java index 01e1fd5..b6c0ebe 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java @@ -76,6 +76,7 @@ import org.apache.hadoop.hbase.client.Put; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.client.RegionReplicaUtil; import org.apache.hadoop.hbase.client.Result; +import org.apache.hadoop.hbase.client.Row; import org.apache.hadoop.hbase.client.RowMutations; import org.apache.hadoop.hbase.client.Scan; import org.apache.hadoop.hbase.client.TableDescriptor; @@ -969,7 +970,7 @@ public class RSRpcServices implements HBaseRPCErrorHandler, // Sort to improve lock efficiency for non-atomic batch of operations. If atomic (mostly // called from mutateRows()), order is preserved as its expected from the client if (!atomic) { - Arrays.sort(mArray); + Arrays.sort(mArray, (v1, v2) -> Row.COMPARATOR.compare(v1, v2)); } OperationStatus[] codes = region.batchMutate(mArray, atomic, HConstants.NO_NONCE, @@ -2091,7 +2092,7 @@ public class RSRpcServices implements HBaseRPCErrorHandler, if(edits!=null && !edits.isEmpty()) { // HBASE-17924 // sort to improve lock efficiency - Collections.sort(edits); + Collections.sort(edits, (v1, v2) -> Row.COMPARATOR.compare(v1.mutation, v2.mutation)); long replaySeqId = (entry.getKey().hasOrigSequenceNumber()) ? entry.getKey().getOrigSequenceNumber() : entry.getKey().getLogSequenceNumber(); OperationStatus[] result = doReplayBatchOp(region, edits, replaySeqId);
