Re: [Tarantool-patches] [PATCH v2 09/11] raft: introduce state machine

[Serge Petrenko <sergepetrenko@tarantool.org>]

23.09.2020 23:31, Vladislav Shpilevoy пишет:
>>> diff --git a/src/box/raft.c b/src/box/raft.c
>>> index f712887a1..ce90ed533 100644
>>> --- a/src/box/raft.c
>>> +++ b/src/box/raft.c
>>> @@ -177,16 +178,9 @@ raft_election_quorum(void)
>>> -            /* Can't vote when vclock is unknown. */
>>> -            if (req->vclock == NULL) {
>>> -                say_info("RAFT: vote request is skipped - "
>>> -                     "missing candidate vclock.");
>>> -                break;
>>> -            }
>>> -            /* Can't vote for too old or incomparable nodes. */
>>> -            if (!raft_can_vote_for(req->vclock)) {
>>> +            if (req->state != RAFT_STATE_CANDIDATE) {
>>>                    say_info("RAFT: vote request is skipped - "
>>> -                     "the vclock is not acceptable = %s",
>>> -                     vclock_to_string(req->vclock));
>>> +                     "this is a notification about a vote "
>>> +                     "for a third node, not a request");
>>>                    break;
>>>                }
>>> -            /*
>>> -             * Check if somebody is asking to vote for a third
>>> -             * node - nope. Make votes only when asked directly by
>>> -             * the new candidate. However that restriction may be
>>> -             * relaxed in future, if can be proven to be safe.
>>> -             */
>>> -            if (req->vote != source) {
>>> +            if (raft.volatile_vote != 0) {
>>>                    say_info("RAFT: vote request is skipped - "
>>> -                     "indirect votes are not allowed");
>>> +                     "already voted in this term");
>>>                    break;
>>>                }
>>> -            if (raft.leader != 0) {
>>> +            /* Vclock is not NULL, validated above. */
>>> +            if (!raft_can_vote_for(req->vclock)) {
>>>                    say_info("RAFT: vote request is skipped - the "
>>> -                     "leader is already known - %u",
>>> -                     raft.leader);
>>> +                     "vclock is not acceptable");
>>>                    break;
>>
>> Are you sure we want these messages on `info` level? Info is the default log level and
>>
>> RAFT spams the log with quite a lot of messages. Maybe `verbose` level would be better?
> Raft messages are supposed to be sent super rare. Only at an instance start, and when
> the leader fails. They are sent often only in the tests, because we tweak Raft config
> values a lot and start/kill instances. So I would keep these logs. They could help to
> recover what happened in a live cluster if something goes wrong with Raft.
>
> I propose to lower them to verbose or even remove in case someone will complain.

Ok, I agree

-- 
Serge Petrenko