[tarantool-patches] [PATCH 3/4] swim: fix a bug with invalidation of round msg in fly

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

SWIM works in rounds, each split in steps. On step SWIM sends a
round message. The message is not changed between steps of one
round usually, and is cached so as not to rebuild it without
necessity. But when something changes in one member's attributes,
or in the member table, the message is invalidated to be rebuilt
on a next step. Invalidation resets the cached packet.

But it leads to a bug, when a round message is already scheduled
to be sent, however is not actually sent and invalidated in fly.
Such a message on a next EV_WRITE event will be sent as an empty
packet, which obviously makes no sense.

On the other hand it would be harmful to cancel the invalidated
packet if it is in fly, because during frequent changes the
instance will not send anything.

There are no a test case, because empty packets does not break
anything, but still they are useless. And, as it is said above,
such invalidation would prevent sending any round messages when
there are lots of updates.

Follow up for cf0ddeb889ad798c38542a6f0883bc53ea4b3388
(swim: keep encoded round message cached)
---
 src/lib/swim/swim.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/src/lib/swim/swim.c b/src/lib/swim/swim.c
index 40fa3fb21..902b7111f 100644
--- a/src/lib/swim/swim.c
+++ b/src/lib/swim/swim.c
@@ -366,6 +366,11 @@ struct swim {
 	 * be triggered by any update of any member.
 	 */
 	struct swim_task round_step_task;
+	/**
+	 * True if a packet in the round step task is still valid
+	 * and can be resent on a next round step.
+	 */
+	bool is_round_packet_valid;
 	/**
 	 * Preallocated buffer to store shuffled members here at
 	 * the beginning of each round.
@@ -416,11 +421,18 @@ struct swim {
 	struct rlist dissemination_queue;
 };
 
-/** Reset cached round message on any change of any member. */
+/**
+ * Mark cached round message invalid on any change of any member.
+ * It triggers postponed rebuilding of the message. The round
+ * packet can not be rebuilt right now because 1) invalidation can
+ * occur several times in row when multiple member attributes are
+ * updated, or more than one member are added, 2) the message can
+ * be in fly right now in the output queue inside the scheduler.
+ */
 static inline void
 swim_cached_round_msg_invalidate(struct swim *swim)
 {
-	swim_packet_create(&swim->round_step_task.packet);
+	swim->is_round_packet_valid = false;
 }
 
 /** Put the member into a list of ACK waiters. */
@@ -847,7 +859,7 @@ swim_encode_dissemination(struct swim *swim, struct swim_packet *packet)
 static void
 swim_encode_round_msg(struct swim *swim)
 {
-	if (swim_packet_body_size(&swim->round_step_task.packet) > 0)
+	if (swim->is_round_packet_valid)
 		return;
 	struct swim_packet *packet = &swim->round_step_task.packet;
 	swim_packet_create(packet);
@@ -861,6 +873,7 @@ swim_encode_round_msg(struct swim *swim)
 
 	assert(mp_sizeof_map(map_size) == 1 && map_size >= 2);
 	mp_encode_map(header, map_size);
+	swim->is_round_packet_valid = true;
 }
 
 /**
@@ -964,6 +977,7 @@ swim_send_fd_msg(struct swim *swim, struct swim_task *task,
 	/*
 	 * Reset packet allocator in case if task is being reused.
 	 */
+	assert(! swim_task_is_scheduled(task));
 	swim_packet_create(&task->packet);
 	char *header = swim_packet_alloc(&task->packet, 1);
 	int map_size = swim_encode_src_uuid(swim, &task->packet);
-- 
2.17.2 (Apple Git-113)