Fix: update next_net_seq_num after sending header

Increment the sequence number after we are sure that the relayd has
received correctly the data header. If an error occurs when sending the
header, the data won't be extracted from the buffers thus keeping this
sequence number untouched.

Furthermore, after sending the header, if the relayd dies, this value
won't matter much and if there is an error on the stream when reading
the trace data, the stream will be deleted thus closed on the relayd
making this value useless.

It's important to note that this sequence number is updated on the
relayd side if the full expected data packet was received. So,
incrementing the value after the transmission of the header is not
changing anything in terms of value coherency. The point is to have a
semantic of when read and used successfully (transmission to relayd),
let's update it.

In that code flow, the stream's lock is acquired so no need to
read/update it atomically. I've also added a comments to better
understand the purpose of this variable and how to use it.

Acked-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: David Goulet <dgoulet@efficios.com>

diff --git a/src/common/consumer.c b/src/common/consumer.c
index 935a03d235fee67a32f1f6bd14fa7c9bdf65845d..15fc9b09f800d6225be465b0d3c993e69a279f09 100644 (file)
--- a/src/common/consumer.c
+++ b/src/common/consumer.c
@@ -755,7 +755,7 @@ static int write_relayd_stream_header(struct lttng_consumer_stream *stream,
                 * this next value, 1 should always be substracted in order to compare
                 * the last seen sequence number on the relayd side to the last sent.
                 */
-               data_hdr.net_seq_num = htobe64(stream->next_net_seq_num++);
+               data_hdr.net_seq_num = htobe64(stream->next_net_seq_num);
                /* Other fields are zeroed previously */
 
                ret = relayd_send_data_hdr(&relayd->data_sock, &data_hdr,
@@ -764,6 +764,8 @@ static int write_relayd_stream_header(struct lttng_consumer_stream *stream,
                        goto error;
                }
 
+               ++stream->next_net_seq_num;
+
                /* Set to go on data socket */
                outfd = relayd->data_sock.fd;
        }

diff --git a/src/common/consumer.h b/src/common/consumer.h
index 193533e926e12ea9b392686b24ff5559fccd8efb..830514659772b0afa6bb5de1ad7be678e033dcfb 100644 (file)
--- a/src/common/consumer.h
+++ b/src/common/consumer.h
@@ -129,7 +129,19 @@ struct lttng_consumer_stream {
        unsigned int metadata_flag;
        /* Used when the stream is set for network streaming */
        uint64_t relayd_stream_id;
-       /* Next sequence number to use for trace packet */
+       /*
+        * When sending a stream packet to a relayd, this number is used to track
+        * the packet sent by the consumer and seen by the relayd. When sending the
+        * data header to the relayd, this number is sent and if the transmission
+        * was successful, it is incremented.
+        *
+        * Even if the full data is not fully transmitted it won't matter since
+        * only two possible error can happen after that where either the relayd
+        * died or a read error is detected on the stream making this value useless
+        * after that.
+        *
+        * This value SHOULD be read/updated atomically or with the lock acquired.
+        */
        uint64_t next_net_seq_num;
        /*
         * Lock to use the stream FDs since they are used between threads.