[tarantool-patches] Re: [PATCH 1/1] swim: keep encoded round message cached

[Konstantin Osipov <kostja@tarantool.org>]

* Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [19/04/10 17:15]:
> During a SWIM round a message is being handed out consisting of
> at most 4 sections. Parts of the message change rarely along with
> a member attribute update, or with removal of a member. So it is
> possible to cache the message and send it during several round
> steps in a row. Or even do not rebuild it the whole round.
> 
> Part of #3234
> ---
> Branch: https://github.com/tarantool/tarantool/tree/gerold103/gh-3234-swim-cached-msg
> Issue: https://github.com/tarantool/tarantool/issues/3234
> 
>  src/lib/swim/swim.c    | 24 ++++++++++++++++++++----
>  src/lib/swim/swim_io.h |  7 +++++++
>  2 files changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/src/lib/swim/swim.c b/src/lib/swim/swim.c
> index c64b8df3a..8453295a5 100644
> --- a/src/lib/swim/swim.c
> +++ b/src/lib/swim/swim.c
> @@ -417,6 +417,13 @@ struct swim {
>  	struct rlist dissemination_queue;
>  };
>  
> +/** Reset cached round message on any change of any member. */
> +static inline void
> +cached_round_msg_invalidate(struct swim *swim)
> +{
> +	swim_packet_create(&swim->round_step_task.packet);
> +}

Are you going to add anything to this method? If not, please get
rid of it and inline swim_packet_create() to all relevant places,
with an appropriate comment.

>  swim_on_member_update(struct swim *swim, struct swim_member *member)
>  {
>  	member->unacknowledged_pings = 0;
> +	cached_round_msg_invalidate(swim);
>  	swim_register_event(swim, member);

Please move the call to cached_round_msg_invalidate() to
swim_register_event(). Actually the three members -
in_dissemination_queue, status_ttl and cached message - have
interdependent life cycle. There are two major events to account
for: 
    - whenever you reset status_ttl you need to invalidate the
      cache and ensure the member is registered in the
      dissemination queue.
    - whenever status_ttl drops to zero the member is deleted from
      the dissemination queue and the cached message can be
      deleted as well.

You have two relevant places for these two events:
swim_register_event and swim_decrease_event_ttl().

It should be sufficient to add cache invalidation to them, plus
initialize the cache in the constructor and destroy in the
destructor.


-- 
Konstantin Osipov, Moscow, Russia, +7 903 626 22 32
http://tarantool.io - www.twitter.com/kostja_osipov