[GitHub] [accumulo] DomGarguilo commented on a change in pull request #1888: Fixes #1791 - Flaky test: SuspendedTabletsIT

[GitBox <gi...@apache.org>]

DomGarguilo commented on a change in pull request #1888:
URL: https://github.com/apache/accumulo/pull/1888#discussion_r579428538



##########
File path: test/src/main/java/org/apache/accumulo/test/manager/SuspendedTabletsIT.java
##########
@@ -117,7 +117,12 @@ public void shutdownAndResumeTserver() throws Exception {
     suspensionTestBody((ctx, locs, count) -> {
       Set<TServerInstance> tserversSet = new HashSet<>();
       for (TabletLocationState tls : locs.locationStates.values()) {
-        if (tls.current != null) {
+
+        TabletLocator tl = TabletLocator.getLocator(ctx, MetadataTable.ID);
+        String metadataTserver =
+            tl.locateTablet(ctx, tls.extent.toMetaRow(), false, false).tablet_location;
+        // if the server does not hold the metadata, add it to the list to be shutdown
+        if (tls.current != null && !tls.current.toString().startsWith(metadataTserver)) {

Review comment:
       >1. the tserver could still be added to the list if another tablet adds it, so it could still cause problems, and
   
   I don't see how this could happen. The name of the tserver with the metadata is compared with the tserver that the current tablet is on. Therefore any tablet on that server should yield the same result.
    
   >2. this could result in no tservers shutting down, therefore not actually checking the conditions the test is intended to check
   
   This would only happen if the metadata was not on any of the servers. I'm not sure this could happen since the metadata would need to be on one of the available servers from my understanding.
   
   It may be a good idea to add an assert to make sure two servers are on track to be shutdown.
   




----------------------------------------------------------------
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:
users@infra.apache.org