Re: [Tarantool-patches] [PATCH 4/8] test: introduce raft unit tests

[Serge Petrenko <sergepetrenko@tarantool.org>]

20.12.2020 20:28, Vladislav Shpilevoy пишет:
> Thanks for the review!
>
>>>>> diff --git a/test/unit/raft.c b/test/unit/raft.c
>>>>> new file mode 100644
>>>>> index 000000000..dfb5f8e43
>>>>> --- /dev/null
>>>>> +++ b/test/unit/raft.c
>>>>> +
>>>>> +static void
>>>>> +raft_test_vote_skip(void)
>>>>> +{
>>>>> +    raft_start_test(33);
>>>>> +    struct raft_node node;
>>>>> +    raft_node_create(&node);
>>>>> +
>>>>> +    /* Everything is skipped if the term is outdated. */
>>>> 1. Let's also test a case when vote response has greater term than the candidate itself.
>>> Good idea. Below is the diff (without .result file diff, to be
>>> short).
>>>
>>> ====================
>>> @@ -346,7 +346,7 @@ raft_test_vote(void)
>>>    static void
>>>    raft_test_vote_skip(void)
>>>    {
>>> -    raft_start_test(33);
>>> +    raft_start_test(37);
>>>        struct raft_node node;
>>>        raft_node_create(&node);
>>>    @@ -507,6 +507,35 @@ raft_test_vote_skip(void)
>>>          raft_node_cfg_is_candidate(&node, true);
>>>    +    /*
>>> +     * Vote response with a bigger term must be skipped, but it will bump
>>> +     * the term.
>>> +     */
>>> +
>>> +    /* Re-create the node so as not to write the vclock each time. */
>>> +    raft_node_destroy(&node);
>>> +    raft_node_create(&node);
>>> +
>>> +    raft_run_next_event();
>>> +    is(node.raft.state, RAFT_STATE_CANDIDATE, "became candidate");
>>> +    is(node.raft.term, 2, "term is bumped");
>>> +
>>> +    is(raft_node_send_vote_response(&node,
>>> +        3 /* Term. */,
>>> +        1 /* Vote. */,
>>> +        2 /* Source. */
>>> +    ), 0, "message is accepted");
>>> +
>>> +    ok(raft_node_check_full_state(&node,
>>> +        RAFT_STATE_CANDIDATE /* State. */,
>>> +        0 /* Leader. */,
>>> +        3 /* Term. */,
>>> +        1 /* Vote. */,
>>> +        3 /* Volatile term. */,
>>> +        1 /* Volatile vote. */,
>>> +        "{0: 3}" /* Vclock. */
>>> +    ), "term is bumped and became candidate");
>>> +
>>>        raft_node_destroy(&node);
>>>        raft_finish_test();
>>>    }
>>> ====================
>>
>> I'd also set the quorum to 2 to make sure the node doesn't become leader in the old term
>>
>> and that it doesn't count votes from a bigger term, but maybe that's an overkill for this test.
>>
>> So, up to you.
> Sounds good, added:
>
> ====================
> @@ -525,6 +525,11 @@ raft_test_vote_skip(void)
>   	/* Re-create the node so as not to write the vclock each time. */
>   	raft_node_destroy(&node);
>   	raft_node_create(&node);
> +	/*
> +	 * Set quorum to 2 to ensure the node does not count the bigger-term
> +	 * vote and doesn't become a leader.
> +	 */
> +	raft_node_cfg_election_quorum(&node, 2);
>   
>   	raft_run_next_event();

Thanks!

LGTM.

-- 
Serge Petrenko