dlmarion commented on code in PR #5745:
URL: https://github.com/apache/accumulo/pull/5745#discussion_r2220330060
##########
server/base/src/main/java/org/apache/accumulo/server/util/Admin.java:
##########
@@ -563,18 +554,89 @@ private static void stopServer(final ClientContext
context, final boolean tablet
client -> client.shutdown(TraceUtil.traceInfo(), context.rpcCreds(),
tabletServersToo));
}
- // Visible for tests
- public static void signalGracefulShutdown(final ClientContext context,
String address) {
+ private static void stopServers(final ServerContext context, List<String>
servers,
+ final boolean force)
+ throws AccumuloException, AccumuloSecurityException,
InterruptedException, KeeperException {
+ List<String> hostOnly = new ArrayList<>();
+ Set<HostAndPort> hostAndPort = new TreeSet<>();
- Objects.requireNonNull(address, "address not set");
- final HostAndPort hp = HostAndPort.fromString(address);
+ for (var server : servers) {
+ if (server.contains(":")) {
+ hostAndPort.add(HostAndPort.fromString(server));
+ } else {
+ hostOnly.add(server);
+ }
+ }
+
+ if (!hostOnly.isEmpty()) {
+ // TODO need to see how easy it is to use serverStatus to support
current functionality of
+ // specifying a host. Like what does the whole command to stop all
tservers on a host look
+ // like using admin serviceStatus and admin stop.
+ // TODO start deprecation warning in 4.0 instead of 2.1.x?
Review Comment:
maybe in 4.0 the port is optional, instead of deprecated. Understood that
you are trying to preserve behavior in the patch release when port is not
specified. In 4.0, we could just change the behavior such that if there is no
port, everything on the node gets shut down.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
