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 <[email protected]> Reviewers: Prateek Maheshwari <[email protected]> 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 <[email protected]> Authored: Mon Mar 26 16:08:00 2018 -0700 Committer: Prateek Maheshwari <[email protected]> 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)); } /**
