Fix: use biggest subbuffer size for snapshot max-size

Instead of using the output max size divided by the total amount of
streams in the session, we find the largest subbuffer in the session's
channels. Using that value, we multiply it by the total amount of
streams which gives us the lower limit of the snapshot size. That is
enough to make sure that we can take the snapshot or not.

Once done, the max stream size possible used for the snapshot record is
the largest subbuffer size in the session. This is to make sure that
every channel can extract the same amount of data which ensure fairness
between each channel in the session.

Fixes #783

Acked-by: Julien Desfossez <jdesfossez@efficios.com>
Signed-off-by: David Goulet <dgoulet@efficios.com>

diff --git a/doc/man/lttng.1 b/doc/man/lttng.1
index 3c7bbb44bd84bee3fa9ad58f6182832b6a5a5955..d4fba11878a3fb095b0b5c14f9310bd721bba0d2 100644 (file)
--- a/doc/man/lttng.1
+++ b/doc/man/lttng.1
@@ -880,6 +880,10 @@ Name of the snapshot's output.
 Maximum size in bytes of the snapshot. The maxium size does not include the
 metadata file. Human readable format is accepted: {+k,+M,+G}. For instance,
 \-\-max-size 5M
+
+The minimum size of a snapshot is computed by multiplying the total amount of
+streams in the session by the largest subbuffer size. This is to ensure
+fairness between channels when extracting data.
 .TP
 .BR "\-C, \-\-ctrl-url URL"
 Set control path URL. (Must use -D also)

diff --git a/include/lttng/lttng-error.h b/include/lttng/lttng-error.h
index f4f3a8344d79085d1e1f50a03637c60dfd98df52..e18fb00e8bec8ceb6b64e4777b603ec6f9305afb 100644 (file)
--- a/include/lttng/lttng-error.h
+++ b/include/lttng/lttng-error.h
@@ -110,7 +110,7 @@ enum lttng_error_code {
        LTTNG_ERR_LOAD_INVALID_CONFIG    = 87,  /* Invalid session configuration */
        LTTNG_ERR_LOAD_IO_FAIL           = 88,  /* IO error while reading a session configuration */
        LTTNG_ERR_LOAD_SESSION_NOENT     = 89,  /* Session file not found */
-       /* 90 */
+       LTTNG_ERR_MAX_SIZE_INVALID       = 90,  /* Snapshot max size is invalid. */
        /* 91 */
        /* 92 */
        /* 93 */

diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c
index fff4b917c230e322e36df7caa0ae8a7ab8619bdc..044e9eefec7e14eab9eb1046411395d1ccdebdff 100644 (file)
--- a/src/bin/lttng-sessiond/cmd.c
+++ b/src/bin/lttng-sessiond/cmd.c
@@ -2832,7 +2832,7 @@ error:
  */
 static int record_kernel_snapshot(struct ltt_kernel_session *ksess,
                struct snapshot_output *output, struct ltt_session *session,
-               int wait, int nb_streams)
+               int wait, uint64_t max_stream_size)
 {
        int ret;
 
@@ -2863,7 +2863,7 @@ static int record_kernel_snapshot(struct ltt_kernel_session *ksess,
                goto error_snapshot;
        }
 
-       ret = kernel_snapshot_record(ksess, output, wait, nb_streams);
+       ret = kernel_snapshot_record(ksess, output, wait, max_stream_size);
        if (ret != LTTNG_OK) {
                goto error_snapshot;
        }
@@ -2884,7 +2884,7 @@ error:
  */
 static int record_ust_snapshot(struct ltt_ust_session *usess,
                struct snapshot_output *output, struct ltt_session *session,
-               int wait, int nb_streams)
+               int wait, uint64_t max_stream_size)
 {
        int ret;
 
@@ -2915,7 +2915,7 @@ static int record_ust_snapshot(struct ltt_ust_session *usess,
                goto error_snapshot;
        }
 
-       ret = ust_app_snapshot_record(usess, output, wait, nb_streams);
+       ret = ust_app_snapshot_record(usess, output, wait, max_stream_size);
        if (ret < 0) {
                switch (-ret) {
                case EINVAL:
@@ -2940,11 +2940,51 @@ error:
        return ret;
 }
 
+/*
+ * Return the biggest subbuffer size of all channels in the given session.
+ */
+static uint64_t get_session_max_subbuf_size(struct ltt_session *session)
+{
+       uint64_t max_size = 0;
+
+       assert(session);
+
+       if (session->kernel_session) {
+               struct ltt_kernel_channel *chan;
+               struct ltt_kernel_session *ksess = session->kernel_session;
+
+               /*
+                * For each channel, add to the max size the size of each subbuffer
+                * multiplied by their sized.
+                */
+               cds_list_for_each_entry(chan, &ksess->channel_list.head, list) {
+                       if (chan->channel->attr.subbuf_size > max_size) {
+                               max_size = chan->channel->attr.subbuf_size;
+                       }
+               }
+       }
+
+       if (session->ust_session) {
+               struct lttng_ht_iter iter;
+               struct ltt_ust_channel *uchan;
+               struct ltt_ust_session *usess = session->ust_session;
+
+               cds_lfht_for_each_entry(usess->domain_global.channels->ht, &iter.iter,
+                               uchan, node.node) {
+                       if (uchan->attr.subbuf_size > max_size) {
+                               max_size = uchan->attr.subbuf_size;
+                       }
+               }
+       }
+
+       return max_size;
+}
+
 /*
  * Returns the total number of streams for a session or a negative value
  * on error.
  */
-static unsigned int get_total_nb_stream(struct ltt_session *session)
+static unsigned int get_session_nb_streams(struct ltt_session *session)
 {
        unsigned int total_streams = 0;
 
@@ -2978,6 +3018,7 @@ int cmd_snapshot_record(struct ltt_session *session,
        unsigned int use_tmp_output = 0;
        struct snapshot_output tmp_output;
        unsigned int nb_streams, snapshot_success = 0;
+       uint64_t session_max_size = 0, max_stream_size = 0;
 
        assert(session);
 
@@ -3017,17 +3058,43 @@ int cmd_snapshot_record(struct ltt_session *session,
        }
 
        /*
-        * Get the total number of stream of that session which is used by the
-        * maximum size of the snapshot feature.
+        * Get the session maximum size for a snapshot meaning it will compute the
+        * size of all streams from all domain.
+        */
+       max_stream_size = get_session_max_subbuf_size(session);
+
+       nb_streams = get_session_nb_streams(session);
+       if (nb_streams) {
+               /*
+                * The maximum size of the snapshot is the number of streams multiplied
+                * by the biggest subbuf size of all channels in a session which is the
+                * maximum stream size available for each stream. The session max size
+                * is now checked against the snapshot max size value given by the user
+                * and if lower, an error is returned.
+                */
+               session_max_size = max_stream_size * nb_streams;
+       }
+
+       DBG3("Snapshot max size is %" PRIu64 " for max stream size of %" PRIu64,
+                       session_max_size, max_stream_size);
+
+       /*
+        * If we use a temporary output, check right away if the max size fits else
+        * for each output the max size will be checked.
         */
-       nb_streams = get_total_nb_stream(session);
+       if (use_tmp_output &&
+                       (tmp_output.max_size != 0 &&
+                       tmp_output.max_size < session_max_size)) {
+               ret = LTTNG_ERR_MAX_SIZE_INVALID;
+               goto error;
+       }
 
        if (session->kernel_session) {
                struct ltt_kernel_session *ksess = session->kernel_session;
 
                if (use_tmp_output) {
                        ret = record_kernel_snapshot(ksess, &tmp_output, session,
-                                       wait, nb_streams);
+                                       wait, max_stream_size);
                        if (ret != LTTNG_OK) {
                                goto error;
                        }
@@ -3051,6 +3118,13 @@ int cmd_snapshot_record(struct ltt_session *session,
                                        tmp_output.max_size = output->max_size;
                                }
 
+                               if (tmp_output.max_size != 0 &&
+                                               tmp_output.max_size < session_max_size) {
+                                       rcu_read_unlock();
+                                       ret = LTTNG_ERR_MAX_SIZE_INVALID;
+                                       goto error;
+                               }
+
                                /* Use temporary name. */
                                if (*output->name != '\0') {
                                        strncpy(tmp_output.name, output->name,
@@ -3060,7 +3134,7 @@ int cmd_snapshot_record(struct ltt_session *session,
                                tmp_output.nb_snapshot = session->snapshot.nb_snapshot;
 
                                ret = record_kernel_snapshot(ksess, &tmp_output,
-                                               session, wait, nb_streams);
+                                               session, wait, max_stream_size);
                                if (ret != LTTNG_OK) {
                                        rcu_read_unlock();
                                        goto error;
@@ -3076,7 +3150,7 @@ int cmd_snapshot_record(struct ltt_session *session,
 
                if (use_tmp_output) {
                        ret = record_ust_snapshot(usess, &tmp_output, session,
-                                       wait, nb_streams);
+                                       wait, max_stream_size);
                        if (ret != LTTNG_OK) {
                                goto error;
                        }
@@ -3100,6 +3174,13 @@ int cmd_snapshot_record(struct ltt_session *session,
                                        tmp_output.max_size = output->max_size;
                                }
 
+                               if (tmp_output.max_size != 0 &&
+                                               tmp_output.max_size < session_max_size) {
+                                       rcu_read_unlock();
+                                       ret = LTTNG_ERR_MAX_SIZE_INVALID;
+                                       goto error;
+                               }
+
                                /* Use temporary name. */
                                if (*output->name != '\0') {
                                        strncpy(tmp_output.name, output->name,
@@ -3109,7 +3190,7 @@ int cmd_snapshot_record(struct ltt_session *session,
                                tmp_output.nb_snapshot = session->snapshot.nb_snapshot;
 
                                ret = record_ust_snapshot(usess, &tmp_output, session,
-                                               wait, nb_streams);
+                                               wait, max_stream_size);
                                if (ret != LTTNG_OK) {
                                        rcu_read_unlock();
                                        goto error;

diff --git a/src/bin/lttng-sessiond/kernel.c b/src/bin/lttng-sessiond/kernel.c
index b997a4aeab6b7ac0aac7208fa82bacd6e964ce28..1fae30f0ad7372bdd2f439d10b1044181048f127 100644 (file)
--- a/src/bin/lttng-sessiond/kernel.c
+++ b/src/bin/lttng-sessiond/kernel.c
@@ -823,13 +823,12 @@ void kernel_destroy_channel(struct ltt_kernel_channel *kchan)
  * Return 0 on success or else return a LTTNG_ERR code.
  */
 int kernel_snapshot_record(struct ltt_kernel_session *ksess,
-               struct snapshot_output *output, int wait, unsigned int nb_streams)
+               struct snapshot_output *output, int wait, uint64_t max_size_per_stream)
 {
        int err, ret, saved_metadata_fd;
        struct consumer_socket *socket;
        struct lttng_ht_iter iter;
        struct ltt_kernel_metadata *saved_metadata;
-       uint64_t max_size_per_stream = 0;
 
        assert(ksess);
        assert(ksess->consumer);
@@ -855,10 +854,6 @@ int kernel_snapshot_record(struct ltt_kernel_session *ksess,
                goto error_open_stream;
        }
 
-       if (output->max_size > 0 && nb_streams > 0) {
-               max_size_per_stream = output->max_size / nb_streams;
-       }
-
        /* Send metadata to consumer and snapshot everything. */
        cds_lfht_for_each_entry(ksess->consumer->socks->ht, &iter.iter,
                        socket, node.node) {
@@ -885,17 +880,6 @@ int kernel_snapshot_record(struct ltt_kernel_session *ksess,
 
                /* For each channel, ask the consumer to snapshot it. */
                cds_list_for_each_entry(chan, &ksess->channel_list.head, list) {
-                       if (max_size_per_stream &&
-                                       chan->channel->attr.subbuf_size > max_size_per_stream) {
-                               ret = LTTNG_ERR_INVALID;
-                               DBG3("Kernel snapshot record maximum stream size %" PRIu64
-                                               " is smaller than subbuffer size of %" PRIu64,
-                                               max_size_per_stream, chan->channel->attr.subbuf_size);
-                               (void) kernel_consumer_destroy_metadata(socket,
-                                               ksess->metadata);
-                               goto error_consumer;
-                       }
-
                        pthread_mutex_lock(socket->lock);
                        ret = consumer_snapshot_channel(socket, chan->fd, output, 0,
                                        ksess->uid, ksess->gid,

diff --git a/src/bin/lttng-sessiond/kernel.h b/src/bin/lttng-sessiond/kernel.h
index e7947801257557befe55b32de144c81c2d2c415c..681301fc2f2c73ed971be54e920dcd8ade5adb32 100644 (file)
--- a/src/bin/lttng-sessiond/kernel.h
+++ b/src/bin/lttng-sessiond/kernel.h
@@ -55,7 +55,7 @@ int kernel_validate_version(int tracer_fd);
 void kernel_destroy_session(struct ltt_kernel_session *ksess);
 void kernel_destroy_channel(struct ltt_kernel_channel *kchan);
 int kernel_snapshot_record(struct ltt_kernel_session *ksess,
-               struct snapshot_output *output, int wait, unsigned int nb_streams);
+               struct snapshot_output *output, int wait, uint64_t max_stream_size);
 
 int init_kernel_workarounds(void);
 

diff --git a/src/bin/lttng-sessiond/ust-app.c b/src/bin/lttng-sessiond/ust-app.c
index fc744fc027cd376c0004431c6303136914d41254..4a7fadaf5231d4f3c84d33d7956f68f11dd998d5 100644 (file)
--- a/src/bin/lttng-sessiond/ust-app.c
+++ b/src/bin/lttng-sessiond/ust-app.c
@@ -4911,28 +4911,19 @@ void ust_app_destroy(struct ust_app *app)
  * Return 0 on success or else a negative value.
  */
 int ust_app_snapshot_record(struct ltt_ust_session *usess,
-               struct snapshot_output *output, int wait, unsigned int nb_streams)
+               struct snapshot_output *output, int wait, uint64_t max_stream_size)
 {
        int ret = 0;
        unsigned int snapshot_done = 0;
        struct lttng_ht_iter iter;
        struct ust_app *app;
        char pathname[PATH_MAX];
-       uint64_t max_stream_size = 0;
 
        assert(usess);
        assert(output);
 
        rcu_read_lock();
 
-       /*
-        * Compute the maximum size of a single stream if a max size is asked by
-        * the caller.
-        */
-       if (output->max_size > 0 && nb_streams > 0) {
-               max_stream_size = output->max_size / nb_streams;
-       }
-
        switch (usess->buffer_type) {
        case LTTNG_BUFFER_PER_UID:
        {
@@ -4962,30 +4953,16 @@ int ust_app_snapshot_record(struct ltt_ust_session *usess,
                        /* Add the UST default trace dir to path. */
                        cds_lfht_for_each_entry(reg->registry->channels->ht, &iter.iter,
                                        reg_chan, node.node) {
-
-                               /*
-                                * Make sure the maximum stream size is not lower than the
-                                * subbuffer size or else it's an error since we won't be able to
-                                * snapshot anything.
-                                */
-                               if (max_stream_size &&
-                                               reg_chan->subbuf_size > max_stream_size) {
-                                       ret = -EINVAL;
-                                       DBG3("UST app snapshot record maximum stream size %" PRIu64
-                                                       " is smaller than subbuffer size of %zu",
-                                                       max_stream_size, reg_chan->subbuf_size);
-                                       goto error;
-                               }
-                               ret = consumer_snapshot_channel(socket, reg_chan->consumer_key, output, 0,
-                                               usess->uid, usess->gid, pathname, wait,
+                               ret = consumer_snapshot_channel(socket, reg_chan->consumer_key,
+                                               output, 0, usess->uid, usess->gid, pathname, wait,
                                                max_stream_size);
                                if (ret < 0) {
                                        goto error;
                                }
                        }
-                       ret = consumer_snapshot_channel(socket, reg->registry->reg.ust->metadata_key, output,
-                                       1, usess->uid, usess->gid, pathname, wait,
-                                       max_stream_size);
+                       ret = consumer_snapshot_channel(socket,
+                                       reg->registry->reg.ust->metadata_key, output, 1,
+                                       usess->uid, usess->gid, pathname, wait, max_stream_size);
                        if (ret < 0) {
                                goto error;
                        }
@@ -5027,22 +5004,8 @@ int ust_app_snapshot_record(struct ltt_ust_session *usess,
 
                        cds_lfht_for_each_entry(ua_sess->channels->ht, &chan_iter.iter,
                                        ua_chan, node.node) {
-                               /*
-                                * Make sure the maximum stream size is not lower than the
-                                * subbuffer size or else it's an error since we won't be able to
-                                * snapshot anything.
-                                */
-                               if (max_stream_size &&
-                                               ua_chan->attr.subbuf_size > max_stream_size) {
-                                       ret = -EINVAL;
-                                       DBG3("UST app snapshot record maximum stream size %" PRIu64
-                                                       " is smaller than subbuffer size of %" PRIu64,
-                                                       max_stream_size, ua_chan->attr.subbuf_size);
-                                       goto error;
-                               }
-
-                               ret = consumer_snapshot_channel(socket, ua_chan->key, output, 0,
-                                               ua_sess->euid, ua_sess->egid, pathname, wait,
+                               ret = consumer_snapshot_channel(socket, ua_chan->key, output,
+                                               0, ua_sess->euid, ua_sess->egid, pathname, wait,
                                                max_stream_size);
                                if (ret < 0) {
                                        goto error;

diff --git a/src/bin/lttng-sessiond/ust-app.h b/src/bin/lttng-sessiond/ust-app.h
index 2fd3b56d936f764e46c31c3ba0e19ba53480dbb2..f83e857cfae0f36b17e8fe4159a6e538cbbf9af9 100644 (file)
--- a/src/bin/lttng-sessiond/ust-app.h
+++ b/src/bin/lttng-sessiond/ust-app.h
@@ -325,7 +325,7 @@ ssize_t ust_app_push_metadata(struct ust_registry_session *registry,
                struct consumer_socket *socket, int send_zero_data);
 void ust_app_destroy(struct ust_app *app);
 int ust_app_snapshot_record(struct ltt_ust_session *usess,
-               struct snapshot_output *output, int wait, unsigned int nb_streams);
+               struct snapshot_output *output, int wait, uint64_t max_stream_size);
 unsigned int ust_app_get_nb_stream(struct ltt_ust_session *usess);
 struct ust_app *ust_app_find_by_sock(int sock);
 
@@ -510,7 +510,7 @@ void ust_app_destroy(struct ust_app *app)
 }
 static inline
 int ust_app_snapshot_record(struct ltt_ust_session *usess,
-               struct snapshot_output *output, int wait, unsigned int nb_stream)
+               struct snapshot_output *output, int wait, uint64_t max_stream_size)
 {
        return 0;
 }

diff --git a/src/bin/lttng/commands/snapshot.c b/src/bin/lttng/commands/snapshot.c
index 94deb0cfa2eb0f06acd61cd68daf5efc9ac51874..2d250afc37cfb01df040159936dcbef24444f90d 100644 (file)
--- a/src/bin/lttng/commands/snapshot.c
+++ b/src/bin/lttng/commands/snapshot.c
@@ -372,6 +372,11 @@ static int record(const char *url)
 
        ret = lttng_snapshot_record(current_session_name, output, 0);
        if (ret < 0) {
+               if (ret == -LTTNG_ERR_MAX_SIZE_INVALID) {
+                       ERR("The minimum size of a snapshot is computed by multiplying "
+                                       "the total amount of streams with the largest subbuffer "
+                                       "in the session.");
+               }
                goto error;
        }
 

diff --git a/src/common/error.c b/src/common/error.c
index 5abb7453aaf4bbb0a46a59d94fb9b38baad9b801..26dc0679833b694edd87af2dfefcdf4ae9a3b500 100644 (file)
--- a/src/common/error.c
+++ b/src/common/error.c
@@ -160,6 +160,7 @@ static const char *error_string_array[] = {
        [ ERROR_INDEX(LTTNG_ERR_LOAD_INVALID_CONFIG) ] = "Invalid session configuration",
        [ ERROR_INDEX(LTTNG_ERR_LOAD_IO_FAIL) ] = "IO error while reading a session configuration",
        [ ERROR_INDEX(LTTNG_ERR_LOAD_SESSION_NOENT) ] = "Session file not found",
+       [ ERROR_INDEX(LTTNG_ERR_MAX_SIZE_INVALID) ] = "Snapshot max size is invalid",
 
        /* Last element */
        [ ERROR_INDEX(LTTNG_ERR_NR) ] = "Unknown error code"