From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp57.i.mail.ru (smtp57.i.mail.ru [217.69.128.37]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 6C4854765E0 for ; Mon, 21 Dec 2020 10:36:21 +0300 (MSK) References: <51c5f48d04df7d43e4b773e8d9c2cdad76137dc1.1607879643.git.v.shpilevoy@tarantool.org> <1f433ed9-b8c8-f67a-fb7c-3da7cc504110@tarantool.org> <51c59e82-8351-8383-552b-d0c6056a324c@tarantool.org> <36a79bd1-6349-cc6c-a90c-78b473a8f5f2@tarantool.org> From: Serge Petrenko Message-ID: <7469b83f-dde4-e0b6-3509-8fc009a8e906@tarantool.org> Date: Mon, 21 Dec 2020 10:36:19 +0300 MIME-Version: 1.0 In-Reply-To: <36a79bd1-6349-cc6c-a90c-78b473a8f5f2@tarantool.org> Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: en-GB Subject: Re: [Tarantool-patches] [PATCH 4/8] test: introduce raft unit tests List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy , tarantool-patches@dev.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