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:
[email protected]
