Fix: don't wait for the load thread before serving client commands

Since the session loading thread uses the same communication than
the external clients, it should not be included in the set of
threads that must be launched before the sessiond starts to serve
client commands.

Since the "load session" thread is guaranteed to be the last
essential thread to be initialized, it can explicitly signal
the parents that the sessiond is ready once it is done auto-loading
session configurations.

This commit also adds a lengthy comment explaining the initialization
of the session daemon.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

diff --git a/src/bin/lttng-sessiond/load-session-thread.c b/src/bin/lttng-sessiond/load-session-thread.c
index b95764d6693a76cd902187955362979ccb3c651c..a6c393c9f994a912230d46f5c7f926ee84d040f6 100644 (file)
--- a/src/bin/lttng-sessiond/load-session-thread.c
+++ b/src/bin/lttng-sessiond/load-session-thread.c
@@ -103,6 +103,6 @@ void *thread_load_session(void *data)
        }
 
 end:
-       sessiond_notify_ready();
+       sessiond_signal_parents();
        return NULL;
 }

diff --git a/src/bin/lttng-sessiond/lttng-sessiond.h b/src/bin/lttng-sessiond/lttng-sessiond.h
index 65ab37d59ec4ec5026cb72dc5bea18a7d5004dd0..bcc3b4d97c3c3536f5f2d2318418f7fbcc570c85 100644 (file)
--- a/src/bin/lttng-sessiond/lttng-sessiond.h
+++ b/src/bin/lttng-sessiond/lttng-sessiond.h
@@ -124,5 +124,6 @@ extern int is_root;
 int sessiond_check_thread_quit_pipe(int fd, uint32_t events);
 int sessiond_set_thread_pollset(struct lttng_poll_event *events, size_t size);
 void sessiond_notify_ready(void);
+void sessiond_signal_parents(void);
 
 #endif /* _LTT_SESSIOND_H */

diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c
index cd2f07b60e8f0e6cce6556dab502a161cf351157..d5759b39180d1d789d7171ef6b5c68202ac03c01 100644 (file)
--- a/src/bin/lttng-sessiond/main.c
+++ b/src/bin/lttng-sessiond/main.c
@@ -309,12 +309,38 @@ struct load_session_thread_data *load_info;
 struct lttng_ht *agent_apps_ht_by_sock = NULL;
 
 /*
- * Whether sessiond is ready for commands/health check requests.
- * NR_LTTNG_SESSIOND_READY must match the number of calls to
- * sessiond_notify_ready().
+ * The initialization of the session daemon is done in multiple phases.
+ *
+ * While all threads are launched near-simultaneously, only some of them
+ * are needed to ensure the session daemon can start to respond to client
+ * requests.
+ *
+ * There are two important guarantees that we wish to offer with respect
+ * to the initialisation of the session daemon:
+ *   - When the daemonize/background launcher process exits, the sessiond
+ *     is fully able to respond to client requests,
+ *   - Auto-loaded sessions are visible to clients.
+ *
+ * In order to achieve this, a number of support threads have to be launched
+ * to allow the "client" thread to function properly. Moreover, since the
+ * "load session" thread needs the client thread, we must provide a way
+ * for the "load session" thread to know that the "client" thread is up
+ * and running.
+ *
+ * Hence, the support threads decrement the lttng_sessiond_ready counter
+ * while the "client" threads waits for it to reach 0. Once the "client" thread
+ * unblocks, it posts the message_thread_ready semaphore which allows the
+ * "load session" thread to progress.
+ *
+ * This implies that the "load session" thread is the last to be initialized
+ * and will explicitly call sessiond_signal_parents(), which signals the parents
+ * that the session daemon is fully initialized.
+ *
+ * The two (2) support threads are:
+ *  - agent_thread
+ *  - health_thread
  */
-#define NR_LTTNG_SESSIOND_READY                4
-int lttng_sessiond_ready = NR_LTTNG_SESSIOND_READY;
+int lttng_sessiond_ready = 2;
 
 int sessiond_check_thread_quit_pipe(int fd, uint32_t events)
 {
@@ -323,28 +349,36 @@ int sessiond_check_thread_quit_pipe(int fd, uint32_t events)
 
 /* Notify parents that we are ready for cmd and health check */
 LTTNG_HIDDEN
-void sessiond_notify_ready(void)
+void sessiond_signal_parents(void)
 {
-       if (uatomic_sub_return(&lttng_sessiond_ready, 1) == 0) {
-               /*
-                * Notify parent pid that we are ready to accept command
-                * for client side.  This ppid is the one from the
-                * external process that spawned us.
-                */
-               if (opt_sig_parent) {
-                       kill(ppid, SIGUSR1);
-               }
+       /*
+        * Notify parent pid that we are ready to accept command
+        * for client side.  This ppid is the one from the
+        * external process that spawned us.
+        */
+       if (opt_sig_parent) {
+               kill(ppid, SIGUSR1);
+       }
 
-               /*
-                * Notify the parent of the fork() process that we are
-                * ready.
-                */
-               if (opt_daemon || opt_background) {
-                       kill(child_ppid, SIGUSR1);
-               }
+       /*
+        * Notify the parent of the fork() process that we are
+        * ready.
+        */
+       if (opt_daemon || opt_background) {
+               kill(child_ppid, SIGUSR1);
        }
 }
 
+LTTNG_HIDDEN
+void sessiond_notify_ready(void)
+{
+       /*
+        * The _return variant is used since the implied memory barriers are
+        * required.
+        */
+       (void) uatomic_sub_return(&lttng_sessiond_ready, 1);
+}
+
 static
 void setup_consumerd_path(void)
 {
@@ -4375,8 +4409,6 @@ static void *thread_manage_clients(void *data)
                goto error;
        }
 
-       sessiond_notify_ready();
-
        ret = sem_post(&load_info->message_thread_ready);
        if (ret) {
                PERROR("sem_post message_thread_ready");
@@ -4409,6 +4441,7 @@ static void *thread_manage_clients(void *data)
                if (ret > 0 || (ret < 0 && errno != EINTR)) {
                        goto exit;
                }
+               cmm_smp_rmb();
        }
 
        /* This testpoint is after we signal readiness to the parent. */