Re: [Tarantool-patches] [PATCH 1/3] xrow: encode timestamp into vclock message

[Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi! Thanks for the patch!

On 21.01.2021 18:17, Cyrill Gorcunov via Tarantool-patches wrote:
> The vclock message sent by "applierw" fiber to the replication
> master node operates not only for tracking status of applied data
> but also as a heartbeat packets (after 1.7.7).
> 
> The timestamp field of the xrow_header left zero here we can reuse
> it to track downstream node state by putting realtime value here.
> This won't break any existing instances without the patch because
> the field is unused anywhere yet.
> 
> Need to mention that xrow_encode_vclock used for other stages
> such as joining and encoding realtime value here won't hurt
> as well.

"Won't hurt" is not a good justification for adding a new field
to all vclocks we send everywhere. As well as it is not good to
add libev dependency to xrow internals. Better avoid adding something
"just in case". We need tm in applier heartbeats - lets add them to
there and only there.

You need tm in a single place - in applier_writer_f(), is it
correct? Just add a new function: xrow_encode_vclock_timed()
or something like this. It would take a timestamp. Like
xrow_encode_timestamp() does.

The old xrow_encode_vclock() would call xrow_encode_vclock_timed()
with 0 time, so there won't even be any code duplication.

> Part-of #5447
> 
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> ---
>  src/box/xrow.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/box/xrow.c b/src/box/xrow.c
> index bc06738ad..1a077fafd 100644
> --- a/src/box/xrow.c
> +++ b/src/box/xrow.c
> @@ -1648,6 +1648,7 @@ xrow_encode_vclock(struct xrow_header *row, const struct vclock *vclock)
>  	row->body[0].iov_base = buf;
>  	row->body[0].iov_len = (data - buf);
>  	row->bodycnt = 1;
> +	row->tm = ev_now(loop());
>  	row->type = IPROTO_OK;
>  	return 0;
>  }
>