[GitHub] [tez] ayushtkn commented on a diff in pull request #300: TEZ-4501: Fix TestLocalMode timeouts

["ayushtkn (via GitHub)" <gi...@apache.org>]

ayushtkn commented on code in PR #300:
URL: https://github.com/apache/tez/pull/300#discussion_r1260763370


##########
tez-common/src/main/java/org/apache/tez/common/AsyncDispatcher.java:
##########
@@ -364,10 +369,20 @@ public void handle(Event event) {
       }
       try {
         eventQueue.put(event);
+        if (LOG.isTraceEnabled()) {
+          LOG.trace("AsyncDispatcher put event: {}", event);
+        }
       } catch (InterruptedException e) {
         if (!stopped) {
-          LOG.warn("AsyncDispatcher thread interrupted", e);
+          LOG.warn("AsyncDispatcher thread interrupted (while putting event): {}", event, e);
         }
+        // this is needed because in serviceStop we're looping to drain the rest of the events and checking !drained
+        // in that case, an interruption here would lead to an empty queue with drained=false, leading to
+        // a useless loop with a timeout (this can even cause unit test timeouts)
+        drained = eventQueue.isEmpty();
+        LOG.info(
+            "Setting drained to {} due to interruption while putting a new event, stopped: {}, blockNewEvents: {}",
+            drained, stopped, blockNewEvents);

Review Comment:
   why in serviceStop we don't directly check ```eventQueue.isEmpty```?



-- 
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: issues-unsubscribe@tez.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org