CFE-3581: Graceful restart option #4503

peckpeck · 2021-02-11T15:23:08Z

This PR is base on the systemd one #4499 because it uses the same autotools option for systemd.

peckpeck · 2021-02-11T15:23:21Z

See https://tracker.mender.io/browse/CFE-3581

amousset · 2021-02-12T11:50:43Z

misc/systemd/cf-serverd.socket

+Before=cf-serverd.service
+
+[Socket]
+ListenStream = 5309


Suggested change

ListenStream = 5309

ListenStream = 5308

amousset · 2021-02-17T15:55:24Z

cf-serverd/cf-serverd-functions.c

+        /* if we have a reload config requested but not yet processed
+         * it means we still have clients, let's do a graceful restart */
+        if(ReloadConfigRequested() && GRACEFUL != 0) {
+            Log(LOG_LEVEL_ERR, "Doing a Graceful restart");


Suggested change

Log(LOG_LEVEL_ERR, "Doing a Graceful restart");

Log(LOG_LEVEL_INFO, "Doing a graceful restart");

amousset · 2021-02-17T16:53:44Z

cf-serverd/cf-serverd-functions.c

+ * If there is no systemd make sure alternative init restarts us. */
+static void GracefulStop()
+{
+    Log(LOG_LEVEL_ERR, "Stopping gracefully");


probably not LOG_LEVEL_ERR, we should have one info log and the rest in verbose or debug.

peckpeck · 2021-02-23T17:56:14Z

done

peckpeck · 2021-03-10T09:59:30Z

remove a 2s sleep on recent systemd

peckpeck · 2021-03-10T15:02:05Z

Rebased now that #4499 is merged

vpodzime · 2021-03-11T10:04:30Z

Please rebase this again when #4536 is merged.

peckpeck · 2021-03-15T13:40:08Z

Done, rebase was not needed

vpodzime

Please squash the commits and add an explanation of what is being done, how and why to the commit message. Also please go through the code and make sure it follows the coding style and guidelines from CONTRIBUTING.md

peckpeck · 2021-03-16T15:29:36Z

Better ?

vpodzime

Thanks! Just a few nitpicks and one question.

vpodzime · 2021-03-17T09:33:41Z

cf-serverd/cf-serverd-functions.c

+            if (optarg == NULL)
+            {
+                GRACEFUL = 60;
+            } else


else should be on a separate line

vpodzime · 2021-03-17T09:35:25Z

cf-serverd/cf-serverd-functions.c

@@ -540,14 +557,99 @@ static void PrepareServer(int sd)
    WritePID("cf-serverd.pid"); /* Arranges for cleanup() to tidy it away */
 }

+/* Graceful Stop
+ * This runs a new main process that will die and that init can restart (as systemd can do).
+ * This this can prevent systemd from kill us it there is a problem.


killing us if there is a problem

vpodzime · 2021-03-17T09:35:37Z

cf-serverd/cf-serverd-functions.c

+/* Graceful Stop
+ * This runs a new main process that will die and that init can restart (as systemd can do).
+ * This this can prevent systemd from kill us it there is a problem.
+ * But this makes it possible to finish kandling connections while systemd tries to restart us.


vpodzime · 2021-03-17T09:38:26Z

cf-serverd/cf-serverd-functions.c

+        return;
+    }
+#ifdef HAVE_SD_NOTIFY_BARRIER
+    int anything = 1;


This could be just an unsigned char.

vpodzime · 2021-03-17T09:42:03Z

cf-serverd/cf-serverd-functions.c

-    }
+        /* if we have a reload config requested but not yet processed
+         * it means we still have clients, let's do a graceful restart */
+        if(ReloadConfigRequested() && GRACEFUL != 0)


Missing space after if

vpodzime · 2021-03-17T09:42:29Z

cf-serverd/cf-serverd-functions.c

-    YieldCurrentLock(thislock);
+    int threads_left;
+
+    if(ReloadConfigRequested() && GRACEFUL != 0)


cf-serverd/cf-serverd-functions.c

olehermanse · 2021-03-18T12:01:38Z

cf-serverd/cf-serverd-functions.c

+#if HAVE_SYSTEMD_SD_DAEMON_H
+/* Graceful Stop
+ * This runs a new main process that will die and that init can restart (as systemd can do).
+ * This this can prevent systemd from kill us if there is a problem.


Suggested change

* This this can prevent systemd from kill us if there is a problem.

* This can prevent systemd from killing us if there is a problem.

vpodzime

Nice! Thanks! Please change the commit message title to Add a graceful detach option that can be used with systemd. Also please change the path in the commit message to /var/cfengine/bin/cf-serverd.

peckpeck · 2021-03-19T08:27:51Z

indeed :)

vpodzime · 2021-03-22T10:03:23Z

ExecStart=/var/cfengine/bin/cf-serverd --graceful-detach=600 --no-fork
Should be 60

When cf-serverd receives a SIGHUP, instead of reloading it detaches itself from init (systemd) and continues to process existing connections. At the same time systemd detects that the process has died and restarts a new one to accept new connections. It works by double forking (detach from parent) and by signaling to systemd that the new process is now the main process. And then the new process suddenly dies, to make systemd restart it. This way you can implement a graceful restart if you have a unit with following options : ExecStart=/var/cfengine/bin/cf-serverd --graceful-detach=60 --no-fork ExecReload=/bin/kill -HUP $MAINPID Restart=always NotifyAccess=main KillMode=process Ticket: CFE-3581

peckpeck · 2021-03-24T11:23:57Z

done

vpodzime · 2021-03-24T14:41:05Z

@cf-bottom jenkins, please

vpodzime

Looks great to me.

cf-bottom · 2021-03-24T14:43:56Z

Sure, I triggered a build:

Jenkins: https://ci.cfengine.com/job/pr-pipeline/6254/

Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-6254/

vpodzime · 2021-03-25T09:52:49Z

Clouds... Retry:

vpodzime · 2021-03-25T15:18:15Z

Clouds... Retry:

Only non-related failures. 👍

amousset reviewed Feb 12, 2021

View reviewed changes

amousset reviewed Feb 17, 2021

View reviewed changes

peckpeck force-pushed the graceful_restart_option branch from 812afbe to d7759ec Compare March 10, 2021 09:58

peckpeck force-pushed the graceful_restart_option branch from d7759ec to f03caf3 Compare March 10, 2021 15:01

peckpeck mentioned this pull request Mar 10, 2021

Add systemd socket activation option #4499

Merged

olehermanse changed the title ~~Graceful restart option~~ CFE-3581: Graceful restart option Mar 11, 2021

vpodzime self-requested a review March 11, 2021 10:04

vpodzime requested changes Mar 16, 2021

View reviewed changes

peckpeck force-pushed the graceful_restart_option branch from f03caf3 to f9a3a39 Compare March 16, 2021 15:26

vpodzime reviewed Mar 17, 2021

View reviewed changes

peckpeck force-pushed the graceful_restart_option branch from f9a3a39 to 199b7db Compare March 17, 2021 10:13

vpodzime self-requested a review March 18, 2021 08:35

peckpeck force-pushed the graceful_restart_option branch from 199b7db to 87e91b1 Compare March 18, 2021 10:36

olehermanse reviewed Mar 18, 2021

View reviewed changes

vpodzime reviewed Mar 19, 2021

View reviewed changes

peckpeck force-pushed the graceful_restart_option branch from 87e91b1 to e9586fe Compare March 19, 2021 08:27

peckpeck force-pushed the graceful_restart_option branch from e9586fe to 9ccc646 Compare March 22, 2021 10:29

vpodzime approved these changes Mar 24, 2021

View reviewed changes

vpodzime merged commit 9914e9c into cfengine:master Mar 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CFE-3581: Graceful restart option #4503

CFE-3581: Graceful restart option #4503

peckpeck commented Feb 11, 2021

peckpeck commented Feb 11, 2021

amousset Feb 12, 2021

amousset Feb 17, 2021 •

edited

Loading

amousset Feb 17, 2021

peckpeck commented Feb 23, 2021

peckpeck commented Mar 10, 2021 •

edited

Loading

peckpeck commented Mar 10, 2021

vpodzime commented Mar 11, 2021

peckpeck commented Mar 15, 2021

vpodzime left a comment

peckpeck commented Mar 16, 2021

vpodzime left a comment

vpodzime Mar 17, 2021

vpodzime Mar 17, 2021

vpodzime Mar 17, 2021

vpodzime Mar 17, 2021

vpodzime Mar 17, 2021

vpodzime Mar 17, 2021

olehermanse Mar 18, 2021

vpodzime left a comment

peckpeck commented Mar 19, 2021

vpodzime commented Mar 22, 2021

peckpeck commented Mar 24, 2021

vpodzime commented Mar 24, 2021

vpodzime left a comment

cf-bottom commented Mar 24, 2021

vpodzime commented Mar 25, 2021

vpodzime commented Mar 25, 2021

	Log(LOG_LEVEL_ERR, "Doing a Graceful restart");
	Log(LOG_LEVEL_INFO, "Doing a graceful restart");

	* This this can prevent systemd from kill us if there is a problem.
	* This can prevent systemd from killing us if there is a problem.

CFE-3581: Graceful restart option #4503

CFE-3581: Graceful restart option #4503

Conversation

peckpeck commented Feb 11, 2021

peckpeck commented Feb 11, 2021

Choose a reason for hiding this comment

amousset Feb 17, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peckpeck commented Feb 23, 2021

peckpeck commented Mar 10, 2021 • edited Loading

peckpeck commented Mar 10, 2021

vpodzime commented Mar 11, 2021

peckpeck commented Mar 15, 2021

vpodzime left a comment

Choose a reason for hiding this comment

peckpeck commented Mar 16, 2021

vpodzime left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vpodzime left a comment

Choose a reason for hiding this comment

peckpeck commented Mar 19, 2021

vpodzime commented Mar 22, 2021

peckpeck commented Mar 24, 2021

vpodzime commented Mar 24, 2021

vpodzime left a comment

Choose a reason for hiding this comment

cf-bottom commented Mar 24, 2021

vpodzime commented Mar 25, 2021

vpodzime commented Mar 25, 2021

amousset Feb 17, 2021 •

edited

Loading

peckpeck commented Mar 10, 2021 •

edited

Loading