-
Notifications
You must be signed in to change notification settings - Fork 750
[GOBBLIN-2210] Wrap temporal heartbeat runnable #4119
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
[GOBBLIN-2210] Wrap temporal heartbeat runnable #4119
Conversation
b013117 to
523a382
Compare
gobblin-utility/src/main/java/org/apache/gobblin/util/ExecutorsUtils.java
Show resolved
Hide resolved
| } catch (Exception exception) { | ||
| // Catch all exceptions to prevent the thread from dying | ||
| // and log the exception | ||
| log.error("Caught exception in runnable {}", exception.getMessage()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should we log stacktrace?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logging stacktrace will bloat the log messages, only motivation to wrap the thread is to avoid killing of thread.
Added a log line in debug mode
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #4119 +/- ##
============================================
- Coverage 45.38% 42.82% -2.56%
+ Complexity 3192 2482 -710
============================================
Files 696 513 -183
Lines 26628 21748 -4880
Branches 2655 2478 -177
============================================
- Hits 12085 9314 -2771
+ Misses 13542 11479 -2063
+ Partials 1001 955 -46 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Dear Gobblin maintainers,
Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!
JIRA
Description
Temporal heartbeat is sent periodically using ScheduledExecutorService but in case any exception is thrown while sending heartbeat from temporal it will kill the heartbeat runnable task and even though activity might be still running it will be killed after sometime when server doesn't receive any heartbeat from activity, to avoid this we can wrap the runnable to print exception instead so that next heartbeat events can be sent properly
safeRunnableinExecutorsUtilssafeRunnableTests
testSafeRunnableRunsSuccessfully,testSafeRunnableHandlesExceptionCommits