Re: [Tarantool-patches] [PATCH 4/7] txn_limbo: persist the latest effective promote in snapshot

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

Danke schön für der Patch!

See 3 comments below.

> diff --git a/src/box/memtx_engine.c b/src/box/memtx_engine.c
> index 6c4982b9f..3713d39d0 100644
> --- a/src/box/memtx_engine.c
> +++ b/src/box/memtx_engine.c
> @@ -225,6 +226,22 @@ memtx_engine_recover_raft(const struct xrow_header *row)
>  	return 0;
>  }
>  
> +static int
> +memtx_engine_recover_promote(const struct xrow_header *row)

1. Maybe call it recover_synchro instead of promote? Even in the
body PROMOTE is not mentioned anyhow except for the assertion.
What if we ever add more members to the synchro state?

> +{
> +	assert(row->type == IPROTO_PROMOTE);
> +	struct synchro_request req;
> +	if (xrow_decode_synchro(row, &req) != 0)
> +		return -1;
> +	/*
> +	 * Origin id cannot be deduced from row.replica_id in a checkpoint,
> +	 * because all it's rows have a zero replica_id.
> +	 */
> +	req.origin_id = req.replica_id;
> +	txn_limbo_process(&txn_limbo, &req);
> +	return 0;
> +}
> @@ -655,6 +676,17 @@ finish:
>  	return rc;
>  }
>  
> +static int
> +checkpoint_write_promote(struct xlog *l, const struct synchro_request *req)
> +{
> +	struct xrow_header row;
> +	char body[XROW_SYNCHRO_BODY_LEN_MAX];
> +	xrow_encode_synchro(&row, body, req);
> +	if (checkpoint_write_row(l, &row) != 0)
> +		return -1;
> +	return 0;

2. Could also make 'return checkpoint_write_row(...);'
without branching + that might activate the tail call
optimization.

> diff --git a/src/box/txn_limbo.h b/src/box/txn_limbo.h
> index e409ac657..84a19bb40 100644
> --- a/src/box/txn_limbo.h
> +++ b/src/box/txn_limbo.h
> @@ -311,6 +311,12 @@ txn_limbo_process(struct txn_limbo *limbo, const struct synchro_request *req);
>  int
>  txn_limbo_wait_confirm(struct txn_limbo *limbo);
>  
> +/**
> + * Persist limbo state to a given synchro request.
> + */
> +void
> +txn_limbo_checkpoint(struct txn_limbo *limbo, struct synchro_request *req);

3. Lets make the limbo a const pointer. It does not change, does it?