You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by st...@apache.org on 2020/01/29 02:45:13 UTC

[hbase] branch branch-2 updated: HBASE-23753 Update of errorprone generated failures

This is an automated email from the ASF dual-hosted git repository.

stack pushed a commit to branch branch-2
in repository https://gitbox.apache.org/repos/asf/hbase.git


The following commit(s) were added to refs/heads/branch-2 by this push:
     new 97e8218  HBASE-23753 Update of errorprone generated failures
97e8218 is described below

commit 97e82184a11b3e354911ce1905f9cc5b8fa01688
Author: stack <st...@apache.org>
AuthorDate: Tue Jan 28 14:13:06 2020 -0800

    HBASE-23753 Update of errorprone generated failures
    
    hbase-client/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
    hbase-server/src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java
     Complains about mismatch in types when Compare. Implement Compare in
     base Interface.
    
    hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
     Complains pbs never return null.
    
    hbase-server/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSinkManager.java
     Needed redo because errorprone complains can't mock Service from guava.
    
    hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionReplicasWithRestartScenarios.java
    hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController.java
     Unrelated...adding one-liner debug statements chasing other test
     failures.
---
 .../java/org/apache/hadoop/hbase/HRegionInfo.java  | 11 +---
 .../org/apache/hadoop/hbase/client/RegionInfo.java |  6 ++-
 .../hadoop/hbase/client/RegionInfoBuilder.java     | 13 ++---
 .../apache/hadoop/hbase/protobuf/ProtobufUtil.java |  2 +-
 .../apache/hadoop/hbase/executor/EventHandler.java | 16 +++---
 .../TestRegionReplicasWithRestartScenarios.java    |  1 +
 .../regionserver/TestReplicationSinkManager.java   | 61 +++++++++++++---------
 .../TestSnapshotScannerHDFSAclController.java      |  3 +-
 8 files changed, 57 insertions(+), 56 deletions(-)

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
index 77602d5..6edd3fa 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
@@ -74,7 +74,7 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos;
  */
 @Deprecated
 @InterfaceAudience.Public
-public class HRegionInfo implements RegionInfo, Comparable<HRegionInfo> {
+public class HRegionInfo implements RegionInfo {
   private static final Logger LOG = LoggerFactory.getLogger(HRegionInfo.class);
 
   /**
@@ -701,15 +701,6 @@ public class HRegionInfo implements RegionInfo, Comparable<HRegionInfo> {
     return this.hashCode;
   }
 
-  //
-  // Comparable
-  //
-
-  @Override
-  public int compareTo(HRegionInfo o) {
-    return RegionInfo.COMPARATOR.compare(this, o);
-  }
-
   /**
    * @return Comparator to use comparing {@link KeyValue}s.
    * @deprecated Use Region#getCellComparator().  deprecated for hbase 2.0, remove for hbase 3.0
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java
index a971332..538d744 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfo.java
@@ -68,7 +68,7 @@ import org.apache.hadoop.hbase.shaded.protobuf.generated.HBaseProtos;
  *
  */
 @InterfaceAudience.Public
-public interface RegionInfo {
+public interface RegionInfo extends Comparable<RegionInfo> {
   RegionInfo UNDEFINED = RegionInfoBuilder.newBuilder(TableName.valueOf("__UNDEFINED__")).build();
   /**
    * Separator used to demarcate the encodedName in a region name
@@ -822,4 +822,8 @@ public interface RegionInfo {
     }
     return Bytes.compareTo(getStartKey(), other.getEndKey()) < 0;
   }
+
+  default int compareTo(RegionInfo other) {
+    return RegionInfo.COMPARATOR.compare(this, other);
+  }
 }
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfoBuilder.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfoBuilder.java
index e79a7b7..78a06bd 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfoBuilder.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionInfoBuilder.java
@@ -1,5 +1,4 @@
-/**
- *
+/*
  * 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
@@ -130,7 +129,7 @@ public class RegionInfoBuilder {
    * An implementation of RegionInfo that adds mutable methods so can build a RegionInfo instance.
    */
   @InterfaceAudience.Private
-  static class MutableRegionInfo implements RegionInfo, Comparable<RegionInfo> {
+  static class MutableRegionInfo implements RegionInfo {
     /**
      * The new format for a region name contains its encodedName at the end.
      * The encoded name also serves as the directory name for the region
@@ -453,7 +452,7 @@ public class RegionInfoBuilder {
       if (!(o instanceof RegionInfo)) {
         return false;
       }
-      return this.compareTo((RegionInfo)o) == 0;
+      return compareTo((RegionInfo)o) == 0;
     }
 
     /**
@@ -463,11 +462,5 @@ public class RegionInfoBuilder {
     public int hashCode() {
       return this.hashCode;
     }
-
-    @Override
-    public int compareTo(RegionInfo other) {
-      return RegionInfo.COMPARATOR.compare(this, other);
-    }
-
   }
 }
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
index 326e78a..5bba1e1 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/ProtobufUtil.java
@@ -1344,7 +1344,7 @@ public final class ProtobufUtil {
     List<CellProtos.Cell> values = proto.getCellList();
 
     if (proto.hasExists()) {
-      if ((values != null && !values.isEmpty()) ||
+      if (!values.isEmpty() ||
           (proto.hasAssociatedCellCount() && proto.getAssociatedCellCount() > 0)) {
         throw new IllegalArgumentException("bad proto: exists with cells is no allowed " + proto);
       }
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java
index eb94744..df84e00 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/executor/EventHandler.java
@@ -1,4 +1,4 @@
-/**
+/*
  *
  * Licensed to the Apache Software Foundation (ASF) under one
  * or more contributor license agreements.  See the NOTICE file
@@ -52,7 +52,7 @@ import org.slf4j.LoggerFactory;
  * @see ExecutorService
  */
 @InterfaceAudience.Private
-public abstract class EventHandler implements Runnable, Comparable<Runnable> {
+public abstract class EventHandler implements Runnable, Comparable<EventHandler> {
   private static final Logger LOG = LoggerFactory.getLogger(EventHandler.class);
 
   // type of event this object represents
@@ -152,12 +152,14 @@ public abstract class EventHandler implements Runnable, Comparable<Runnable> {
    * priority beyond FIFO, they should override {@link #getPriority()}.
    */
   @Override
-  public int compareTo(Runnable o) {
-    EventHandler eh = (EventHandler)o;
-    if(getPriority() != eh.getPriority()) {
-      return (getPriority() < eh.getPriority()) ? -1 : 1;
+  public int compareTo(EventHandler o) {
+    if (o == null) {
+      return 1;
+    }
+    if(getPriority() != o.getPriority()) {
+      return (getPriority() < o.getPriority()) ? -1 : 1;
     }
-    return (this.seqid < eh.seqid) ? -1 : 1;
+    return (this.seqid < o.seqid) ? -1 : 1;
   }
 
   @Override
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionReplicasWithRestartScenarios.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionReplicasWithRestartScenarios.java
index dc0a918..dbe508b 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionReplicasWithRestartScenarios.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionReplicasWithRestartScenarios.java
@@ -137,6 +137,7 @@ public class TestRegionReplicasWithRestartScenarios {
 
   private void assertReplicaDistributed() throws Exception {
     Collection<HRegion> onlineRegions = getRS().getOnlineRegionsLocalContext();
+    LOG.info("ASSERT DISTRIBUTED {}", onlineRegions);
     boolean res = checkDuplicates(onlineRegions);
     assertFalse(res);
     Collection<HRegion> onlineRegions2 = getSecondaryRS().getOnlineRegionsLocalContext();
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSinkManager.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSinkManager.java
index 39dabb4..3bc9d9d 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSinkManager.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestReplicationSinkManager.java
@@ -1,4 +1,4 @@
-/**
+/*
  * 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
@@ -38,6 +38,7 @@ import org.junit.experimental.categories.Category;
 import org.apache.hbase.thirdparty.com.google.common.collect.Lists;
 
 import org.apache.hadoop.hbase.shaded.protobuf.generated.AdminProtos.AdminService;
+import org.mockito.Mockito;
 
 @Category({ReplicationTests.class, SmallTests.class})
 public class TestReplicationSinkManager {
@@ -48,14 +49,38 @@ public class TestReplicationSinkManager {
 
   private static final String PEER_CLUSTER_ID = "PEER_CLUSTER_ID";
 
-  private HBaseReplicationEndpoint replicationEndpoint;
   private ReplicationSinkManager sinkManager;
+  private HBaseReplicationEndpoint replicationEndpoint;
+
+  /**
+   * Manage the 'getRegionServers' for the tests below. Override the base class handling
+   * of Regionservers. We used to use a mock for this but updated guava/errorprone disallows
+   * mocking of classes that implement Service.
+   */
+  private static class SetServersHBaseReplicationEndpoint extends HBaseReplicationEndpoint {
+    List<ServerName> regionServers;
+
+    @Override
+    public boolean replicate(ReplicateContext replicateContext) {
+      return false;
+    }
+
+    @Override
+    public synchronized void setRegionServers(List<ServerName> regionServers) {
+      this.regionServers = regionServers;
+    }
+
+    @Override
+    public List<ServerName> getRegionServers() {
+      return this.regionServers;
+    }
+  }
 
   @Before
   public void setUp() {
-    replicationEndpoint = mock(HBaseReplicationEndpoint.class);
-    sinkManager = new ReplicationSinkManager(mock(ClusterConnection.class),
-                      PEER_CLUSTER_ID, replicationEndpoint, new Configuration());
+    this.replicationEndpoint = new SetServersHBaseReplicationEndpoint();
+    sinkManager = new ReplicationSinkManager(mock(ClusterConnection.class), PEER_CLUSTER_ID,
+      replicationEndpoint, new Configuration());
   }
 
   @Test
@@ -65,12 +90,8 @@ public class TestReplicationSinkManager {
     for (int i = 0; i < totalServers; i++) {
       serverNames.add(mock(ServerName.class));
     }
-
-    when(replicationEndpoint.getRegionServers())
-          .thenReturn(serverNames);
-
+    replicationEndpoint.setRegionServers(serverNames);
     sinkManager.chooseSinks();
-
     int expected = (int) (totalServers * ReplicationSinkManager.DEFAULT_REPLICATION_SOURCE_RATIO);
     assertEquals(expected, sinkManager.getNumSinks());
 
@@ -80,12 +101,8 @@ public class TestReplicationSinkManager {
   public void testChooseSinks_LessThanRatioAvailable() {
     List<ServerName> serverNames = Lists.newArrayList(mock(ServerName.class),
       mock(ServerName.class));
-
-    when(replicationEndpoint.getRegionServers())
-          .thenReturn(serverNames);
-
+    replicationEndpoint.setRegionServers(serverNames);
     sinkManager.chooseSinks();
-
     assertEquals(1, sinkManager.getNumSinks());
   }
 
@@ -93,9 +110,7 @@ public class TestReplicationSinkManager {
   public void testReportBadSink() {
     ServerName serverNameA = mock(ServerName.class);
     ServerName serverNameB = mock(ServerName.class);
-    when(replicationEndpoint.getRegionServers())
-      .thenReturn(Lists.newArrayList(serverNameA, serverNameB));
-
+    replicationEndpoint.setRegionServers(Lists.newArrayList(serverNameA, serverNameB));
     sinkManager.chooseSinks();
     // Sanity check
     assertEquals(1, sinkManager.getNumSinks());
@@ -120,10 +135,7 @@ public class TestReplicationSinkManager {
     for (int i = 0; i < totalServers; i++) {
       serverNames.add(mock(ServerName.class));
     }
-    when(replicationEndpoint.getRegionServers())
-          .thenReturn(serverNames);
-
-
+    replicationEndpoint.setRegionServers(serverNames);
     sinkManager.chooseSinks();
     // Sanity check
     int expected = (int) (totalServers * ReplicationSinkManager.DEFAULT_REPLICATION_SOURCE_RATIO);
@@ -175,10 +187,7 @@ public class TestReplicationSinkManager {
     for (int i = 0; i < totalServers; i++) {
       serverNames.add(mock(ServerName.class));
     }
-    when(replicationEndpoint.getRegionServers())
-          .thenReturn(serverNames);
-
-
+    replicationEndpoint.setRegionServers(serverNames);
     sinkManager.chooseSinks();
     // Sanity check
     List<ServerName> sinkList = sinkManager.getSinksForTesting();
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController.java
index ab7c70d..7ef4cf8 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestSnapshotScannerHDFSAclController.java
@@ -652,6 +652,7 @@ public class TestSnapshotScannerHDFSAclController {
       admin.restoreSnapshot(snapshot);
       admin.snapshot(snapshot3, table);
 
+      LOG.info("CHECK");
       TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser, snapshot, -1);
       TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser, snapshot2, -1);
       TestHDFSAclHelper.canUserScanSnapshot(TEST_UTIL, grantUser, snapshot3, -1);
@@ -1085,4 +1086,4 @@ final class TestHDFSAclHelper {
       return null;
     };
   }
-}
\ No newline at end of file
+}