clintropolis commented on code in PR #13499:
URL: https://github.com/apache/druid/pull/13499#discussion_r1040104987
##########
server/src/main/java/org/apache/druid/server/coordination/ChangeRequestHistory.java:
##########
@@ -74,11 +76,24 @@ public ChangeRequestHistory(int maxSize)
this.singleThreadedExecutor =
Execs.singleThreaded("SegmentChangeRequestHistory");
}
+ public void stop()
+ {
+ singleThreadedExecutor.shutdownNow();
+ final LinkedHashSet<CustomSettableFuture<?>> futures = new
LinkedHashSet<>(waitingFutures.keySet());
+ waitingFutures.clear();
+ for (CustomSettableFuture<?> theFuture : futures) {
+ theFuture.setException(new IllegalStateException("Server is shutting
down."));
+ }
+ }
+
/**
* Add batch of segment changes update.
*/
public synchronized void addChangeRequests(List<T> requests)
{
+ if (singleThreadedExecutor.isShutdown()) {
Review Comment:
it can, but I guess my thoughts are that it doesn't matter since the
`HttpLoadQueuePeon` is stopped by the server disappearing from discovery, so it
seemed better to silently ignore because its going down anyway and to avoid any
extra log spam and not have potentially multiple shutdown paths on the
`HttpLoadQueuePeon` side (same reason I opted to go with failing the future
with exception instead of a successful "im shutting dowm" response).
##########
server/src/main/java/org/apache/druid/server/coordination/ChangeRequestHistory.java:
##########
@@ -74,11 +76,24 @@ public ChangeRequestHistory(int maxSize)
this.singleThreadedExecutor =
Execs.singleThreaded("SegmentChangeRequestHistory");
}
+ public void stop()
+ {
+ singleThreadedExecutor.shutdownNow();
+ final LinkedHashSet<CustomSettableFuture<?>> futures = new
LinkedHashSet<>(waitingFutures.keySet());
+ waitingFutures.clear();
+ for (CustomSettableFuture<?> theFuture : futures) {
+ theFuture.setException(new IllegalStateException("Server is shutting
down."));
+ }
+ }
+
/**
* Add batch of segment changes update.
*/
public synchronized void addChangeRequests(List<T> requests)
{
+ if (singleThreadedExecutor.isShutdown()) {
Review Comment:
it can, but I guess my thoughts are that it doesn't matter since the
`HttpLoadQueuePeon` is stopped by the server disappearing from discovery, so it
seemed better to silently ignore because its going down anyway and to avoid any
extra log spam and not have potentially multiple shutdown paths on the
`HttpLoadQueuePeon` side (same reason I opted to go with failing the future
with exception instead of a successful "im shutting down" response).
--
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]
