scratch build was tested and fix confirmed. I think we should also reschedule all existing tevent timers, we don't have an infrastructure to do it though.
On Wed, 2016-08-24 at 11:29 +0200, Pavel Březina wrote:
From 09e5ad3448694d9609bd2a9e89e8a05ef82f66bc Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= pbrezina@redhat.com Date: Mon, 22 Aug 2016 13:15:04 +0200 Subject: [PATCH] watchdog: cope with time shift Resolves: https://fedorahosted.org/sssd/ticket/3154
May I ask you to be a bit more verbose in the commit message, please?
src/util/util_watchdog.c | 41
+++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+) diff --git a/src/util/util_watchdog.c b/src/util/util_watchdog.c index
5032fddba1b94b3fc7e560162c392dfa57d699cf..178ff92d5c6b950d0086204012354 74499ba595d 100644
--- a/src/util/util_watchdog.c +++ b/src/util/util_watchdog.c @@ -29,8 +29,39 @@ struct watchdog_ctx { struct timeval interval; struct tevent_timer *te; volatile int ticks;
+ /* To detect time shift. */ + struct tevent_context *ev; + int input_interval; + time_t timestamp; } watchdog_ctx; +bool watchdog_detect_timeshift(void)
This function may be static, as it's not used anywhere out of this file.
+{ + time_t prev_time; + time_t cur_time; + errno_t ret;
+ prev_time = watchdog_ctx.timestamp; + cur_time = watchdog_ctx.timestamp = time(NULL); + if (cur_time < prev_time) { + /* Time shift detected. We need to restart watchdog. */ + DEBUG(SSSDBG_IMPORTANT_INFO, "Time shift detected, " + "restarting watchdog!\n"); + teardown_watchdog(); + ret = setup_watchdog(watchdog_ctx.ev,
watchdog_ctx.input_interval);
+ if (ret != EOK) { + DEBUG(SSSDBG_FATAL_FAILURE, "Unable to restart watchdog
"
+ "[%d]: %s\n", ret, sss_strerror(ret)); + orderly_shutdown(1); + }
+ return true; + }
+ return false; +}
/* the watchdog is purposefully *not* handled by the tevent * signal handler as it is meant to check if the daemon is * still processing the event queue itself. A stuck process @@ -38,6 +69,12 @@ struct watchdog_ctx { * signals either */ static void watchdog_handler(int sig) { + /* Do not count ticks if time shift was detected + * since watchdog was restarted. */ + if (watchdog_detect_timeshift()) { + return; + }
/* if 3 ticks passed by kills itself */ if (__sync_add_and_fetch(&watchdog_ctx.ticks, 1) > 3) { @@ -101,6 +138,10 @@ int setup_watchdog(struct tevent_context *ev,
int interval)
watchdog_ctx.interval.tv_sec = interval; watchdog_ctx.interval.tv_usec = 0; + watchdog_ctx.ev = ev; + watchdog_ctx.input_interval = interval; + watchdog_ctx.timestamp = time(NULL);
/* Start the timer */ /* we give 1 second head start to the watchdog event */ its.it_value.tv_sec = interval + 1; -- 2.1.0
Patch looks good to me apart from the minor comments.
Best Regards, -- Fabiano Fidêncio
On 08/24/2016 11:41 AM, Fabiano Fidêncio wrote:
On Wed, 2016-08-24 at 11:29 +0200, Pavel Březina wrote:
From 09e5ad3448694d9609bd2a9e89e8a05ef82f66bc Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= pbrezina@redhat.com Date: Mon, 22 Aug 2016 13:15:04 +0200 Subject: [PATCH] watchdog: cope with time shift
May I ask you to be a bit more verbose in the commit message, please?
Sure.
src/util/util_watchdog.c | 41
+++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/src/util/util_watchdog.c b/src/util/util_watchdog.c index
5032fddba1b94b3fc7e560162c392dfa57d699cf..178ff92d5c6b950d0086204012354 74499ba595d 100644
--- a/src/util/util_watchdog.c +++ b/src/util/util_watchdog.c @@ -29,8 +29,39 @@ struct watchdog_ctx { struct timeval interval; struct tevent_timer *te; volatile int ticks;
- /* To detect time shift. */
- struct tevent_context *ev;
- int input_interval;
- time_t timestamp; } watchdog_ctx;
+bool watchdog_detect_timeshift(void)
This function may be static, as it's not used anywhere out of this file.
Yep.
Patch looks good to me apart from the minor comments.
Thank you, new patch is attached.
On Wed, Aug 24, 2016 at 11:55 AM, Pavel Březina pbrezina@redhat.com wrote:
On 08/24/2016 11:41 AM, Fabiano Fidêncio wrote:
On Wed, 2016-08-24 at 11:29 +0200, Pavel Březina wrote:
From 09e5ad3448694d9609bd2a9e89e8a05ef82f66bc Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= pbrezina@redhat.com Date: Mon, 22 Aug 2016 13:15:04 +0200 Subject: [PATCH] watchdog: cope with time shift
May I ask you to be a bit more verbose in the commit message, please?
Sure.
src/util/util_watchdog.c | 41
+++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/src/util/util_watchdog.c b/src/util/util_watchdog.c index
5032fddba1b94b3fc7e560162c392dfa57d699cf..178ff92d5c6b950d0086204012354 74499ba595d 100644
--- a/src/util/util_watchdog.c +++ b/src/util/util_watchdog.c @@ -29,8 +29,39 @@ struct watchdog_ctx { struct timeval interval; struct tevent_timer *te; volatile int ticks;
- /* To detect time shift. */
- struct tevent_context *ev;
- int input_interval;
- time_t timestamp; } watchdog_ctx;
+bool watchdog_detect_timeshift(void)
This function may be static, as it's not used anywhere out of this file.
Yep.
Patch looks good to me apart from the minor comments.
Thank you, new patch is attached.
I didn't run any tests (neither CI or make check), but I assume you will before pushing the patch.
Acked-by: Fabiano Fidêncio fidencio@redhat.com
Best Regards, -- Fabiano Fidêncio
On (24/08/16 11:58), Fabiano Fidêncio wrote:
On Wed, Aug 24, 2016 at 11:55 AM, Pavel Březina pbrezina@redhat.com wrote:
On 08/24/2016 11:41 AM, Fabiano Fidêncio wrote:
On Wed, 2016-08-24 at 11:29 +0200, Pavel Březina wrote:
From 09e5ad3448694d9609bd2a9e89e8a05ef82f66bc Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= pbrezina@redhat.com Date: Mon, 22 Aug 2016 13:15:04 +0200 Subject: [PATCH] watchdog: cope with time shift
May I ask you to be a bit more verbose in the commit message, please?
Sure.
src/util/util_watchdog.c | 41
+++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/src/util/util_watchdog.c b/src/util/util_watchdog.c index
5032fddba1b94b3fc7e560162c392dfa57d699cf..178ff92d5c6b950d0086204012354 74499ba595d 100644
--- a/src/util/util_watchdog.c +++ b/src/util/util_watchdog.c @@ -29,8 +29,39 @@ struct watchdog_ctx { struct timeval interval; struct tevent_timer *te; volatile int ticks;
- /* To detect time shift. */
- struct tevent_context *ev;
- int input_interval;
- time_t timestamp; } watchdog_ctx;
+bool watchdog_detect_timeshift(void)
This function may be static, as it's not used anywhere out of this file.
Yep.
Patch looks good to me apart from the minor comments.
Thank you, new patch is attached.
I didn't run any tests (neither CI or make check), but I assume you will before pushing the patch.
Acked-by: Fabiano Fidêncio fidencio@redhat.com
ACK++
and it seems that it fixed some failure in downstream offline tests. Because they change time here and back many times.
http://sssd-ci.duckdns.org/logs/job/52/37/summary.html
LS
On (25/08/16 15:54), Lukas Slebodnik wrote:
On (24/08/16 11:58), Fabiano Fidêncio wrote:
On Wed, Aug 24, 2016 at 11:55 AM, Pavel Březina pbrezina@redhat.com wrote:
On 08/24/2016 11:41 AM, Fabiano Fidêncio wrote:
On Wed, 2016-08-24 at 11:29 +0200, Pavel Březina wrote:
From 09e5ad3448694d9609bd2a9e89e8a05ef82f66bc Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= pbrezina@redhat.com Date: Mon, 22 Aug 2016 13:15:04 +0200 Subject: [PATCH] watchdog: cope with time shift
May I ask you to be a bit more verbose in the commit message, please?
Sure.
src/util/util_watchdog.c | 41
+++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/src/util/util_watchdog.c b/src/util/util_watchdog.c index
5032fddba1b94b3fc7e560162c392dfa57d699cf..178ff92d5c6b950d0086204012354 74499ba595d 100644
--- a/src/util/util_watchdog.c +++ b/src/util/util_watchdog.c @@ -29,8 +29,39 @@ struct watchdog_ctx { struct timeval interval; struct tevent_timer *te; volatile int ticks;
- /* To detect time shift. */
- struct tevent_context *ev;
- int input_interval;
- time_t timestamp; } watchdog_ctx;
+bool watchdog_detect_timeshift(void)
This function may be static, as it's not used anywhere out of this file.
Yep.
Patch looks good to me apart from the minor comments.
Thank you, new patch is attached.
I didn't run any tests (neither CI or make check), but I assume you will before pushing the patch.
Acked-by: Fabiano Fidêncio fidencio@redhat.com
ACK++
and it seems that it fixed some failure in downstream offline tests. Because they change time here and back many times.
master: * b8ceaeb80cffb00c26390913ea959b77f7e848b9
LS
On 08/25/2016 04:03 PM, Lukas Slebodnik wrote:
On (25/08/16 15:54), Lukas Slebodnik wrote:
On (24/08/16 11:58), Fabiano Fidêncio wrote:
On Wed, Aug 24, 2016 at 11:55 AM, Pavel Březina pbrezina@redhat.com wrote:
On 08/24/2016 11:41 AM, Fabiano Fidêncio wrote:
On Wed, 2016-08-24 at 11:29 +0200, Pavel Březina wrote:
From 09e5ad3448694d9609bd2a9e89e8a05ef82f66bc Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= pbrezina@redhat.com Date: Mon, 22 Aug 2016 13:15:04 +0200 Subject: [PATCH] watchdog: cope with time shift
May I ask you to be a bit more verbose in the commit message, please?
Sure.
src/util/util_watchdog.c | 41
+++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
diff --git a/src/util/util_watchdog.c b/src/util/util_watchdog.c index
5032fddba1b94b3fc7e560162c392dfa57d699cf..178ff92d5c6b950d0086204012354 74499ba595d 100644
--- a/src/util/util_watchdog.c +++ b/src/util/util_watchdog.c @@ -29,8 +29,39 @@ struct watchdog_ctx { struct timeval interval; struct tevent_timer *te; volatile int ticks;
- /* To detect time shift. */
- struct tevent_context *ev;
- int input_interval;
- time_t timestamp; } watchdog_ctx;
+bool watchdog_detect_timeshift(void)
This function may be static, as it's not used anywhere out of this file.
Yep.
Patch looks good to me apart from the minor comments.
Thank you, new patch is attached.
I didn't run any tests (neither CI or make check), but I assume you will before pushing the patch.
Acked-by: Fabiano Fidêncio fidencio@redhat.com
ACK++
and it seems that it fixed some failure in downstream offline tests. Because they change time here and back many times.
master:
- b8ceaeb80cffb00c26390913ea959b77f7e848b9
This patch should be pushed to all versions with watchdog present.
On (26/08/16 10:08), Pavel Březina wrote:
On 08/25/2016 04:03 PM, Lukas Slebodnik wrote:
On (25/08/16 15:54), Lukas Slebodnik wrote:
On (24/08/16 11:58), Fabiano Fidêncio wrote:
On Wed, Aug 24, 2016 at 11:55 AM, Pavel Březina pbrezina@redhat.com wrote:
On 08/24/2016 11:41 AM, Fabiano Fidêncio wrote:
On Wed, 2016-08-24 at 11:29 +0200, Pavel Březina wrote:
> From 09e5ad3448694d9609bd2a9e89e8a05ef82f66bc Mon Sep 17 00:00:00
2001 > > From: =?UTF-8?q?Pavel=20B=C5=99ezina?= pbrezina@redhat.com > Date: Mon, 22 Aug 2016 13:15:04 +0200 > Subject: [PATCH] watchdog: cope with time shift > > Resolves: > https://fedorahosted.org/sssd/ticket/3154
May I ask you to be a bit more verbose in the commit message, please?
Sure.
> --- > src/util/util_watchdog.c | 41
+++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 41 insertions(+) > > diff --git a/src/util/util_watchdog.c b/src/util/util_watchdog.c > index
5032fddba1b94b3fc7e560162c392dfa57d699cf..178ff92d5c6b950d0086204012354 74499ba595d 100644 > > --- a/src/util/util_watchdog.c > +++ b/src/util/util_watchdog.c > @@ -29,8 +29,39 @@ struct watchdog_ctx { > struct timeval interval; > struct tevent_timer *te; > volatile int ticks; > + > + /* To detect time shift. */ > + struct tevent_context *ev; > + int input_interval; > + time_t timestamp; > } watchdog_ctx; > > +bool watchdog_detect_timeshift(void)
This function may be static, as it's not used anywhere out of this file.
Yep.
Patch looks good to me apart from the minor comments.
Thank you, new patch is attached.
I didn't run any tests (neither CI or make check), but I assume you will before pushing the patch.
Acked-by: Fabiano Fidêncio fidencio@redhat.com
ACK++
and it seems that it fixed some failure in downstream offline tests. Because they change time here and back many times.
master:
- b8ceaeb80cffb00c26390913ea959b77f7e848b9
This patch should be pushed to all versions with watchdog present.
It was :-)
sh$ git log origin/sssd-1-13 src/util/util_watchdog.c sh$ echo $? 0
Watchdog is only in master and we do not have separate branch for 1.14
LS
sssd-devel@lists.fedorahosted.org