apache / accumulo

Apache Accumulo
https://accumulo.apache.org
Apache License 2.0
1.06k stars 445 forks source link

MiniAccumuloClusterControl.stop(ServerType server, String hostname) does nothing with hostname #4055

Open dlmarion opened 9 months ago

dlmarion commented 9 months ago

Describe the bug Calling the method will stop all servers of the type

Versions (OS, Maven, Java, and others, as appropriate):

ctubbsii commented 9 months ago

This is not surprising, because MiniAccumuloClusterControl was written as a MiniAccumuloCluster implementation of the more generic ClusterControl that was intended to create an abstraction layer so that ITs could be written to run on an existing standalone cluster, or on mini, and be relatively agnostic to which they were run on. I believe at the time it was created, mini only used one server of each type, so some of the options that were added to support a larger standalone cluster (like using the hostname) were left unimplemented in the mini implementation, and don't really apply (since the hostname for all mini is effectively "localhost").

Most of our tests do not really use these features, and I do not know if the standalone capability works. I suspect it hasn't really worked since prior to 2.0, because that was the last time I remember anybody trying to use it. A lot of this is cruft that has stuck around as previous attempts to add features to our integration tests, but time consuming to rip out or polish up to simplify the ITs. In addition to the cluster control stuff, there is the AccumuloCluster interface, and the AccumuloClusterHarness/MiniClusterHarness, and the AccumuloITBase and its various subclasses, SharedMiniClusterBase and ConfigurableMacBase, that most of the ITs extend. Very few of them call any of the ClusterControl stuff at all.

I would probably advocate for aggressive ripping out and simplifying, rather than trying to implement all the features of each of these framework classes.