tolbertam commented on code in PR #4157:
URL: https://github.com/apache/cassandra/pull/4157#discussion_r2093561304
##########
src/java/org/apache/cassandra/repair/autorepair/AutoRepair.java:
##########
@@ -566,4 +569,35 @@ public void progress(String tag, ProgressEvent event)
}
}
}
+
+ public synchronized void shutdownBlocking() throws ExecutionException,
InterruptedException
+ {
+ if (!isSetupDone)
+ {
+ // By default, executors within AutoRepair are not initialized as
the feature is opt-in.
+ // If the AutoRepair has not been set up, then there is no need to
worry about shutting it down
+ return;
+ }
+ if (isShutDown)
+ {
+ throw new IllegalStateException("AutoRepair has already been shut
down");
+ }
Review Comment:
This isn't super important and is more of a personal preference, but could
we make a subsequent shutdown a no op instead like this:
```suggestion
if (!isSetupDone || isShutdown)
{
// By default, executors within AutoRepair are not initialized
as the feature is opt-in.
// If the AutoRepair has not been set up, then there is no need
to worry about shutting it down
return;
}
```
I see that some things in the shutdown path `HintsService.shutdownBlocking`
would throw on subsequent calls, while others don't (e.g.
`CommitLog.instance.shutdownBlocking`). I like the behavior of not throwing
an exception, which is similar to how java's `Executor.shutdown()` behaves.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
