You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@samza.apache.org by pm...@apache.org on 2018/03/26 23:08:10 UTC

samza git commit: ScheduleAfterDebounceTime should catch all Throwable to avoid losing unhandled Errors

Repository: samza
Updated Branches:
  refs/heads/master 272aa32e3 -> b0603c3c7


ScheduleAfterDebounceTime should catch all Throwable to avoid losing unhandled Errors

If an Action executed in the scheduler throws an Error (or other Throwable?) besides Exception, it is silently lost since the Action/Runnable wrapper only catches Exception, not Throwable. This made my troubleshooting of an issue very difficult. Made it seem like the code was "hung" when really it had thrown a "NoSuchMethodError" (Error instead of Exception) due to a simple dependency issue on my side.

Catching Throwable instead ensures this is handled and propagated properly.

Author: thunderstumpges <ts...@ntent.com>

Reviewers: Prateek Maheshwari <pm...@apache.org>

Closes #450 from thunderstumpges/scheduler-catch-throwable


Project: http://git-wip-us.apache.org/repos/asf/samza/repo
Commit: http://git-wip-us.apache.org/repos/asf/samza/commit/b0603c3c
Tree: http://git-wip-us.apache.org/repos/asf/samza/tree/b0603c3c
Diff: http://git-wip-us.apache.org/repos/asf/samza/diff/b0603c3c

Branch: refs/heads/master
Commit: b0603c3c7ca155ba507a76fc4f3d5407b7628c30
Parents: 272aa32
Author: thunderstumpges <ts...@ntent.com>
Authored: Mon Mar 26 16:08:00 2018 -0700
Committer: Prateek Maheshwari <pm...@linkedin.com>
Committed: Mon Mar 26 16:08:00 2018 -0700

----------------------------------------------------------------------
 .../apache/samza/zk/ScheduleAfterDebounceTime.java  | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/samza/blob/b0603c3c/samza-core/src/main/java/org/apache/samza/zk/ScheduleAfterDebounceTime.java
----------------------------------------------------------------------
diff --git a/samza-core/src/main/java/org/apache/samza/zk/ScheduleAfterDebounceTime.java b/samza-core/src/main/java/org/apache/samza/zk/ScheduleAfterDebounceTime.java
index 3a7dca9..b53d245 100644
--- a/samza-core/src/main/java/org/apache/samza/zk/ScheduleAfterDebounceTime.java
+++ b/samza-core/src/main/java/org/apache/samza/zk/ScheduleAfterDebounceTime.java
@@ -93,6 +93,8 @@ public class ScheduleAfterDebounceTime {
    * and all pending enqueued tasks will be cancelled.
    */
   public synchronized void stopScheduler() {
+    LOG.info("Stopping Scheduler");
+
     scheduledExecutorService.shutdownNow();
 
     // Clear the existing future handles.
@@ -142,27 +144,27 @@ public class ScheduleAfterDebounceTime {
         } else {
           LOG.debug("Action: {} completed successfully.", actionName);
         }
-      } catch (Exception exception) {
-        LOG.error("Execution of action: {} failed.", actionName, exception);
-        doCleanUpOnTaskException(exception);
+      } catch (Throwable t) {
+        LOG.error("Execution of action: {} failed.", actionName, t);
+        doCleanUpOnTaskException(t);
       }
     };
   }
 
   /**
-   * Handler method to invoke on a exception during an scheduled task execution and which
+   * Handler method to invoke on a throwable during an scheduled task execution and which
    * the following operations in sequential order.
    * <ul>
    *   <li> Stop the scheduler. If the task execution fails or a task is interrupted, scheduler will not accept/execute any new tasks.</li>
    *   <li> Invokes the onError handler method if taskCallback is defined.</li>
    * </ul>
    *
-   * @param exception the exception happened during task execution.
+   * @param throwable the throwable that happened during task execution.
    */
-  private void doCleanUpOnTaskException(Exception exception) {
+  private void doCleanUpOnTaskException(Throwable throwable) {
     stopScheduler();
 
-    scheduledTaskCallback.ifPresent(callback -> callback.onError(exception));
+    scheduledTaskCallback.ifPresent(callback -> callback.onError(throwable));
   }
 
   /**