ndimiduk commented on a change in pull request #2731:
URL: https://github.com/apache/hbase/pull/2731#discussion_r534357346
##########
File path:
hbase-server/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestRefreshRecoveredReplication.java
##########
@@ -121,22 +127,25 @@ public void testReplicationRefreshSource() throws
Exception {
table1.put(new Put(r).addColumn(famName, famName, r));
}
- // kill rs holding table region
- Optional<RegionServerThread> server =
UTIL1.getMiniHBaseCluster().getLiveRegionServerThreads()
- .stream()
+ // Kill rs holding table region. There are only TWO servers. We depend on
it.
+ List<RegionServerThread> rss =
UTIL1.getMiniHBaseCluster().getLiveRegionServerThreads();
+ assertEquals(2, rss.size());
+ Optional<RegionServerThread> server = rss.stream()
.filter(rst ->
CollectionUtils.isNotEmpty(rst.getRegionServer().getRegions(tablename)))
.findAny();
Assert.assertTrue(server.isPresent());
+ HRegionServer otherServer = rss.get(0).getRegionServer() ==
server.get().getRegionServer()?
+ rss.get(1).getRegionServer(): rss.get(0).getRegionServer();
server.get().getRegionServer().abort("stopping for test");
+ // waiting for recovered peer to appear.
+ Replication replication =
(Replication)otherServer.getReplicationSourceService();
+ UTIL1.waitFor(60000, () ->
!replication.getReplicationManager().getOldSources().isEmpty());
+ // Wait on only one server being up.
UTIL1.waitFor(60000, () ->
- UTIL1.getMiniHBaseCluster().getLiveRegionServerThreads().size() ==
NUM_SLAVES1 - 1);
+ // Have to go back to source here because getLiveRegionServerThreads
makes new array each time
+ UTIL1.getMiniHBaseCluster().getLiveRegionServerThreads().size() ==
NUM_SLAVES1 - 1);
Review comment:
Instead of waiting on strict equality by count, can you wait for
`getLiveRegionServerThreads()` to contain only `otherServer` by identity?
##########
File path:
hbase-server/src/test/java/org/apache/hadoop/hbase/replication/regionserver/TestRefreshRecoveredReplication.java
##########
@@ -75,6 +80,7 @@
@BeforeClass
public static void setUpBeforeClass() throws Exception {
+ // NUM_SLAVES1 is presumed 2 in below.
Review comment:
Can `NUM_SLAVES1` be made final, and referenced wherever it's presumed
to be 2? Maybe rename it if you're in there making that change...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
