lhotari opened a new pull request #12853: URL: https://github.com/apache/pulsar/pull/12853
### Motivation - Prevent the intended cancellation of scheduled tasks in Pulsar code base. - [ScheduledExecutorService#scheduleAtFixedRate](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/concurrent/ScheduledExecutorService.html#scheduleAtFixedRate(java.lang.Runnable,long,long,java.util.concurrent.TimeUnit)) (and [scheduleWithFixedDelay](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/concurrent/ScheduledExecutorService.html#scheduleWithFixedDelay(java.lang.Runnable,long,long,java.util.concurrent.TimeUnit))) won't schedule the next execution if running the task throws an exception. This is explained [in the javadoc](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/concurrent/ScheduledExecutorService.html#scheduleAtFixedRate(java.lang.Runnable,long,long,java.util.concurrent.TimeUnit)). This can lead to unintended cancellation of the scheduled task in failure scenarios. ### Additional context This is a cross-cutting change that is about the correct usage of ScheduleExecutorService API. The problem and solution is explained in this StackOverflow Answer: https://stackoverflow.com/a/24902026. Bookkeeper client includes a solution called SafeRunnable (in 2 locations: in https://github.com/apache/bookkeeper/blob/master/bookkeeper-common/src/main/java/org/apache/bookkeeper/common/util/SafeRunnable.java and https://github.com/apache/bookkeeper/blob/master/bookkeeper-server/src/main/java/org/apache/bookkeeper/util/SafeRunnable.java). The SafeRunnable.safeRun method is used extensively in many parts of Bookkeeper code. Some parts of Pulsar code base uses SafeRunnable.safeRun to catch and log exceptions. The concept of "SafeRunnable" is confusing. Instead of copying the same solution that Bookkeeper uses, it is intentional that there is no "safe runnable" and the method is called "Runnables.catchingAndLoggingThrowables" to make it explicit that the Runnable is wrapped with handling that catches and logs throwables. There is no "magic". ### Modifications - Add new static method `org.apache.pulsar.common.util.Runnables.catchingAndLoggingThrowables` to pulsar-common which wraps a `Runnable` with try-catch block to catch and log all Throwables. - Use this wrapper in most usages of ScheduledExecutorService scheduleAtFixedRate and scheduleWithFixedDelay methods. -- 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]
