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 77602d521ad..6edd3fa3973 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 { +public class HRegionInfo implements RegionInfo { private static final Logger LOG = LoggerFactory.getLogger(HRegionInfo.class); /** @@ -701,15 +701,6 @@ public class HRegionInfo implements RegionInfo, Comparable { 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 a9713322ad3..538d744d676 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 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 e79a7b76cdb..78a06bd27e2 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 { + 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 326e78abf6d..5bba1e1beca 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 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 eb94744299f..df84e004503 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 { +public abstract class EventHandler implements Runnable, Comparable { 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 { * 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; } - return (this.seqid < eh.seqid) ? -1 : 1; + if(getPriority() != o.getPriority()) { + return (getPriority() < o.getPriority()) ? -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 dc0a918f381..dbe508bcbd0 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 onlineRegions = getRS().getOnlineRegionsLocalContext(); + LOG.info("ASSERT DISTRIBUTED {}", onlineRegions); boolean res = checkDuplicates(onlineRegions); assertFalse(res); Collection 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 39dabb4dab0..3bc9d9d134b 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 regionServers; + + @Override + public boolean replicate(ReplicateContext replicateContext) { + return false; + } + + @Override + public synchronized void setRegionServers(List regionServers) { + this.regionServers = regionServers; + } + + @Override + public List 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 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 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 ab7c70db426..7ef4cf83ee7 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 +}