-
Notifications
You must be signed in to change notification settings - Fork 299
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
watchdog: improve the behavior how watchdog is handled #727
base: master
Are you sure you want to change the base?
Conversation
* dlt_daemon_process_user_messages * dlt_daemon_send_ringbuffer_to_client * dlt_daemon_client_send_all_multiple * dlt_daemon_process_systemd_timer * dlt_daemon_handle_event * dlt_daemon_socket_sendreliable These methods can take longer than the systemd watchdog timeout allowed, depending on the number of clients, which leads that the systemd watchdog kills dlt-daemon. Add a timeout to these methods so that they may only run for as long as the watchdog timer is set. In addition, set the default WatchdogTimeout to an appropriate value in case the env variable is not set. Signed-off-by: Ahmet Findikci <ahmet.findikci@mercedes-benz.com>
|
||
if (watchdogTimeoutSeconds == 0) { | ||
dlt_log(LOG_WARNING, "Watchdog timeout is too small, need at least 1s, setting 1s timeout\n"); | ||
watchdogTimeoutSeconds = 1; |
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 set the default value is 30 for this case?
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.
we could, but this scenario only applies if WATCHDOG_USEC was deliberately set to 0, the default 30 is only used if the WATCHDOG_USEC is not set at all.
I have no opinion about it
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.
@lti9hc so do you want 30 or is 1 good for you?
return false; | ||
} | ||
if (sd_notify(0, "WATCHDOG=1") < 0) | ||
dlt_vlog(LOG_WARNING, "%s: Could not reset systemd watchdog\n", __func__); |
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.
I think we should add return false in case could not reset the systemd watchdog
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.
yup, I can add that
These methods can take longer than the systemd watchdog timeout allowed, depending on the number of clients, which leads that the systemd watchdog kills dlt-daemon. Add a timeout to these methods so that they may only run for as long as the watchdog timer is set. In addition, set the default WatchdogTimeout to an appropriate value in case the env variable is not set.
The program was tested solely for our own use cases, which might differ from yours.
Licensed under Mozilla Public License Version 2.0
Ahmet Findikci ahmet.findikci@mercedes-benz.com, Mercedes-Benz Tech Innovation GmbH, imprint