[Tarantool-patches] [PATCH] box: introduce on_election triggers
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Mon Jul 26 23:27:10 MSK 2021
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.
More information about the Tarantool-patches
mailing list