Re: [Tarantool-patches] [PATCH] replication: send only confirmed data during final join

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

Hi! Thanks for the patch!

See 3 comments below.

> diff --git a/src/box/applier.cc b/src/box/applier.cc
> index 8d7ce5d99..2af7693a0 100644
> --- a/src/box/applier.cc
> +++ b/src/box/applier.cc
> @@ -526,7 +528,8 @@ applier_wait_register(struct applier *applier, uint64_t row_count)
>  	while (true) {
>  		coio_read_xrow(coio, ibuf, &row);
>  		applier->last_row_time = ev_monotonic_now(loop());
> -		if (iproto_type_is_dml(row.type)) {
> +		if (iproto_type_is_dml(row.type) ||
> +		    row.type == IPROTO_CONFIRM) {
>  			vclock_follow_xrow(&replicaset.vclock, &row);
>  			if (apply_final_join_row(&row) != 0)

1. apply_final_join_row() internally simply ignores synchro rows. Including
the rollbacks. You might want to handle synchro rows separately from
apply_final_join_row(). Just follow vclock and increase row_count.
apply_final_join_row() would assert that the row is DML then.

2. What if we meet ROLLBACK during final join rows retrieval? It could be
stored in xlogs created during initial join. It might even be followed for
CONFIRMs of newer synchro transactions. Or might now, it does not matter.
Won't ROLLBACK among final join rows result into unknown request type again?

>  				diag_raise();
> diff --git a/src/box/relay.cc b/src/box/relay.cc
> index 186f77ed4..5699b8a63 100644
> --- a/src/box/relay.cc
> +++ b/src/box/relay.cc
> @@ -381,6 +381,10 @@ relay_final_join(int fd, uint64_t sync, struct vclock *start_vclock,
>  		relay_delete(relay);
>  	});
>  
> +	/* \sa relay_initial_join(). */

3. Probably better stick with @sa, as we usually use @ instead of \ in
the new code.

> +	if (txn_limbo_wait_confirm(&txn_limbo) != 0)
> +		diag_raise();
> +
>  	relay->r = recovery_new(wal_dir(), false, start_vclock);
>  	vclock_copy(&relay->stop_vclock, stop_vclock);