Re: [Tarantool-patches] [PATCH] box: introduce on_election triggers

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

Hi! Thanks for the patch!

See 3 comments below.

> diff --git a/src/box/raft.c b/src/box/raft.c
> index 7f787c0c5..c9c3ba818 100644
> --- a/src/box/raft.c
> +++ b/src/box/raft.c
> @@ -52,6 +52,8 @@ enum election_mode box_election_mode = ELECTION_MODE_INVALID;
>   */
>  static struct trigger box_raft_on_update;
>  
> +struct rlist box_raft_on_state = RLIST_HEAD_INITIALIZER(box_raft_on_state);

1. I propose to call it 'box_raft_on_update'. We usually use 'on_<action>'
naming pattern.

> +
>  /**
>   * Worker fiber does all the asynchronous work, which may need yields and can be
>   * long. These are WAL writes, network broadcasts. That allows not to block the
> @@ -274,6 +276,7 @@ box_raft_broadcast(struct raft *raft, const struct raft_msg *msg)
>  	assert(raft == box_raft());
>  	struct raft_request req;
>  	box_raft_msg_to_request(msg, &req);
> +	trigger_run(&box_raft_on_state, NULL);

2. Maybe call it after the state is pushed to the relays? So as it
would be sent earlier in case the triggers are going to yield.

>  	replicaset_foreach(replica)
>  		relay_push_raft(replica->relay, &req);
>  }
> diff --git a/test/replication/gh-5819-election-triggers.result b/test/replication/gh-5819-election-triggers.result
> new file mode 100644
> index 000000000..546aac8a2
> --- /dev/null
> +++ b/test/replication/gh-5819-election-triggers.result

3. Hm, I think we are supposed to use gh-#### file naming only for
bugfixes.
https://github.com/tarantool/tarantool/wiki/Code-review-procedure#testing

This is a feature, hence can have a normal name, or be in an existing
test file which is also not bound to a single ticket.