From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 3FEE82BB93 for ; Wed, 17 Apr 2019 15:56:41 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id GR28I4Co3XUk for ; Wed, 17 Apr 2019 15:56:41 -0400 (EDT) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id ACADD2BAAC for ; Wed, 17 Apr 2019 15:56:40 -0400 (EDT) From: Vladislav Shpilevoy Subject: [tarantool-patches] [PATCH 3/4] swim: fix a bug with invalidation of round msg in fly Date: Wed, 17 Apr 2019 22:56:36 +0300 Message-Id: In-Reply-To: References: In-Reply-To: References: Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-Help: List-Unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-Subscribe: List-Owner: List-post: List-Archive: To: tarantool-patches@freelists.org Cc: kostja@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)