Github user vrozov commented on a diff in the pull request:
https://github.com/apache/drill/pull/1023#discussion_r149234360
--- Diff:
exec/java-exec/src/main/java/org/apache/drill/exec/work/WorkManager.java ---
@@ -158,38 +165,49 @@ public DrillbitContext getContext() {
return dContext;
}
- private ExtendedLatch exitLatch = null; // used to wait to exit when
things are still running
-
/**
* Waits until it is safe to exit. Blocks until all currently running
fragments have completed.
- *
- * <p>This is intended to be used by {@link
org.apache.drill.exec.server.Drillbit#close()}.</p>
+ * This is intended to be used by {@link
org.apache.drill.exec.server.Drillbit#close()}.
*/
public void waitToExit() {
- synchronized(this) {
- if (queries.isEmpty() && runningFragments.isEmpty()) {
- return;
+ final long startTime = System.currentTimeMillis();
+ final long endTime = startTime + EXIT_TIMEOUT;
+
+ exitLock.lock();
+
+ try {
+ long currentTime;
+ while ((currentTime = System.currentTimeMillis()) < endTime) {
+ if (queries.isEmpty() && runningFragments.isEmpty()) {
+ break;
+ }
+
+ try {
+ if (!exitCondition.await(endTime - currentTime,
TimeUnit.MILLISECONDS)) {
+ break;
+ }
+ } catch (InterruptedException e) {
+ logger.error("Interrupted while waiting to exit");
+ }
}
- exitLatch = new ExtendedLatch();
- }
+ if (!(queries.isEmpty() && runningFragments.isEmpty())) {
+ logger.warn("Timed out after {} millis. Shutting down before all
fragments and foremen " +
+ "have completed.", EXIT_TIMEOUT);
+ }
- // Wait for at most 5 seconds or until the latch is released.
- exitLatch.awaitUninterruptibly(5000);
+ } finally {
+ exitLock.unlock();
+ }
}
/**
- * If it is safe to exit, and the exitLatch is in use, signals it so
that waitToExit() will
- * unblock.
+ * A thread calling the {@link #waitToExit()} method is notified when a
foreman is retired.
*/
private void indicateIfSafeToExit() {
- synchronized(this) {
- if (exitLatch != null) {
- if (queries.isEmpty() && runningFragments.isEmpty()) {
- exitLatch.countDown();
- }
- }
- }
+ exitLock.lock();
+ exitCondition.signal();
--- End diff --
I'd recommend adding try/finally and checking the condition before
signaling `exitCondition`. Consider renaming `exitCondition` to `isEmpty` or
`isQueriesAndFragmentsEmpty`.
---
