ctubbsii commented on code in PR #5246:
URL: https://github.com/apache/accumulo/pull/5246#discussion_r1914221231
##########
minicluster/src/main/java/org/apache/accumulo/miniclusterImpl/MiniAccumuloClusterImpl.java:
##########
@@ -824,9 +825,46 @@ public synchronized void stop() throws IOException,
InterruptedException {
control.stop(ServerType.GARBAGE_COLLECTOR, null);
control.stop(ServerType.MANAGER, null);
+ control.stop(ServerType.COMPACTION_COORDINATOR);
control.stop(ServerType.TABLET_SERVER, null);
+ control.stop(ServerType.COMPACTOR, null);
+ control.stop(ServerType.SCAN_SERVER, null);
+
+ // The method calls above kill the server
+ // Clean up the locks in ZooKeeper fo that if the cluster
+ // is restarted, then the processes will start right away
+ // and not wait for the old locks to be cleaned up.
+ try {
+ new ZooZap().zap(getServerContext().getSiteConfiguration(), new String[]
{"-manager",
+ "-compaction-coordinators", "-tservers", "-compactors",
"-sservers"});
+ } catch (Exception e) {
+ log.error("Error zapping zookeeper locks", e);
+ }
Review Comment:
Should catch exceptions more narrowly. The stop() method in which these
exist is already throwing some that we don't need to bother catching.
##########
minicluster/src/main/java/org/apache/accumulo/miniclusterImpl/MiniAccumuloClusterControl.java:
##########
@@ -278,6 +279,13 @@ public synchronized void stop(ServerType server, String
hostname) throws IOExcep
if (managerProcess != null) {
try {
cluster.stopProcessWithTimeout(managerProcess, 30,
TimeUnit.SECONDS);
+ try {
+ new
ZooZap().zap(cluster.getServerContext().getSiteConfiguration(),
+ new String[] {"-manager"});
Review Comment:
This would probably look nicer if ZooZap took varargs instead of a `String[]`
Should this go in the finally block instead of in the same block that's
handling the 30 second timeout?
##########
minicluster/src/main/java/org/apache/accumulo/miniclusterImpl/MiniAccumuloClusterControl.java:
##########
@@ -278,6 +279,13 @@ public synchronized void stop(ServerType server, String
hostname) throws IOExcep
if (managerProcess != null) {
try {
cluster.stopProcessWithTimeout(managerProcess, 30,
TimeUnit.SECONDS);
+ try {
+ new
ZooZap().zap(cluster.getServerContext().getSiteConfiguration(),
+ new String[] {"-manager"});
+ } catch (Exception e) {
+ log.error("Error zapping Manager zookeeper lock", e);
+ }
Review Comment:
This should catch exceptions more narrowly
##########
server/base/src/main/java/org/apache/accumulo/server/util/ZooZap.java:
##########
@@ -90,6 +90,19 @@ public static void main(String[] args) throws Exception {
@Override
public void execute(String[] args) throws Exception {
+ try {
+ var siteConf = SiteConfiguration.auto();
+ // Login as the server on secure HDFS
+ if (siteConf.getBoolean(Property.INSTANCE_RPC_SASL_ENABLED)) {
+ SecurityUtil.serverLogin(siteConf);
+ }
+ zap(siteConf, args);
+ } finally {
+ SingletonManager.setMode(Mode.CLOSED);
+ }
+ }
+
+ public void zap(SiteConfiguration siteConf, String[] args) {
Review Comment:
```suggestion
public void zap(SiteConfiguration siteConf, String ... args) {
```
--
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]
