Re: [Tarantool-patches] [PATCH 2/2] box: introduce a cfg handle to become syncro leader

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Here is also a general problem - having this as box.cfg option
means, that the selected leader should stay selected regardless
of what happens in the cluster. In particular, it should reject
any attempts to add an entry into the limbo, not originated from
this instance.

Currently this is not guaranteed, see comment below.

> diff --git a/src/box/box.cc b/src/box/box.cc
> index ca24b98ca..087710383 100644
> --- a/src/box/box.cc
> +++ b/src/box/box.cc
> @@ -78,6 +78,7 @@
>  #include "sequence.h"
>  #include "sql_stmt_cache.h"
>  #include "msgpack.h"
> +#include "trivia/util.h"
>  
>  static char status[64] = "unknown";
>  
> @@ -945,6 +946,84 @@ box_set_replication_anon(void)
>  
>  }
>  
> +void
> +box_set_replication_synchro_leader(void)
> +{
> +	bool is_leader = cfg_geti("replication_synchro_leader");
> +	/*
> +	 * For now no actions required when an instance stops
> +	 * being a leader. We should probably wait until txn_limbo
> +	 * becomes empty.
> +	 */
> +	if (!is_leader)
> +		return;
> +	uint32_t former_leader_id = txn_limbo.instance_id;
> +	if (former_leader_id == REPLICA_ID_NIL ||
> +	    former_leader_id == instance_id) {

When limbo is empty, it will change its instance id to whatever
entry will be added next. So it can happen, that I gave replication_synchro_leader
to 2 instances, and if they will create transactions one at a
time, this will work. But looks wrong.

Perhaps it would be better to add a box.ctl function to do this
'limbo cleanup'? Without persisting any leader role in a config.
Until we have a better understanding how leader-read_only-master
roles coexist.