HBASE-19428 Deprecate the compareTo(Row)

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>

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;

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.

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());

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

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

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

hbase-client/src/main/java/org/apache/hadoop/hbase/client/RowMutations.java

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

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()));
+    }
+  }
+}

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