From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (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 C30094696C3 for ; Fri, 10 Apr 2020 02:08:22 +0300 (MSK) References: From: Vladislav Shpilevoy Message-ID: Date: Fri, 10 Apr 2020 01:08:20 +0200 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH v6 1/3] replication: omit 0-th vclock component in replication responses List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Serge Petrenko Cc: tarantool-patches@dev.tarantool.org Hi! Thanks for the patch! > diff --git a/src/box/xrow.c b/src/box/xrow.c > index be026a43c..21a68220a 100644 > --- a/src/box/xrow.c > +++ b/src/box/xrow.c > @@ -51,7 +51,7 @@ static_assert(IPROTO_DATA < 0x7f && IPROTO_METADATA < 0x7f && > static inline uint32_t > mp_sizeof_vclock(const struct vclock *vclock) > { > - uint32_t size = vclock_size(vclock); > + uint32_t size = vclock_size_ignore0(vclock); This doesn't look right to silently ignore 0 component in a function, which says nothing about that: mp_sizeof_vclock(). Not in the function name, nor in a comment. Maybe worth renaming it to mp_sizeof_vclock_ignore0(). Along with mp_encode_vclock(). I am picky to the names, because we still actually have places, where 0 component *is* needed (we have, right?). And we need to say explicitly, which places ignore 0, which don't. Also seems mp_decode_vclock() does not ignore 0. Is it possible that we will get a connection from tarantool, which still does not ignore 0 component, and will send it to us? For example, it was upgraded recently on the latest branch, right before we pushed this patchset. > return mp_sizeof_map(size) + size * (mp_sizeof_uint(UINT32_MAX) + > mp_sizeof_uint(UINT64_MAX)); > }