From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <v.shpilevoy@tarantool.org>
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 dev.tarantool.org (Postfix) with ESMTPS id E8A36430D56
 for <tarantool-patches@dev.tarantool.org>;
 Tue, 29 Oct 2019 00:04:16 +0300 (MSK)
References: <20191028122506.4833-1-i.kosarev@tarantool.org>
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Message-ID: <9c427c1a-9c6f-263b-8cac-99458ccdc8fd@tarantool.org>
Date: Mon, 28 Oct 2019 22:09:45 +0100
MIME-Version: 1.0
In-Reply-To: <20191028122506.4833-1-i.kosarev@tarantool.org>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Subject: Re: [Tarantool-patches] [PATCH v2] replication: fix join vclock
 obtainment in relay_initial_join
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Ilya Kosarev <i.kosarev@tarantool.org>, Konstantin Osipov <kostja.osipov@gmail.com>
Cc: tarantool-patches@dev.tarantool.org

Hi! Thanks for the fixes!

> diff --git a/src/box/relay.cc b/src/box/relay.cc
> index e849fcf4f..d16cd1a56 100644
> --- a/src/box/relay.cc
> +++ b/src/box/relay.cc
> @@ -306,13 +306,14 @@ relay_initial_join(int fd, uint64_t sync, struct vclock *vclock)
>  	});
>  
>  	/*
> -	 * Sync WAL to make sure that all changes visible from
> -	 * the frozen read view are successfully committed.
> +	 * Make sure that current database state is flushed to
> +	 * the WAL and obtain corresponding vclock.
>  	 */
> -	if (wal_sync() != 0)
> +	struct wal_checkpoint checkpoint;
> +	if (wal_begin_checkpoint(&checkpoint) != 0)
>  		diag_raise();
>  
> -	vclock_copy(vclock, &replicaset.vclock);
> +	vclock_copy(vclock, &checkpoint.vclock);

It looks wrong, that we do begin_checkpoint() but omit
commit_checkpoint(). I would call commit too. Probably now
begin() does not allocate any resources, but it may start
in future, and we will have a leak here.