From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp39.i.mail.ru (smtp39.i.mail.ru [94.100.177.99]) (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 E9DD845C304 for ; Sun, 20 Dec 2020 20:28:59 +0300 (MSK) References: <51c5f48d04df7d43e4b773e8d9c2cdad76137dc1.1607879643.git.v.shpilevoy@tarantool.org> <1f433ed9-b8c8-f67a-fb7c-3da7cc504110@tarantool.org> <51c59e82-8351-8383-552b-d0c6056a324c@tarantool.org> From: Vladislav Shpilevoy Message-ID: <36a79bd1-6349-cc6c-a90c-78b473a8f5f2@tarantool.org> Date: Sun, 20 Dec 2020 18:28:56 +0100 MIME-Version: 1.0 In-Reply-To: <51c59e82-8351-8383-552b-d0c6056a324c@tarantool.org> Content-Type: text/plain; charset="utf-8" Content-Language: en-US Content-Transfer-Encoding: 8bit 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: Serge Petrenko , tarantool-patches@dev.tarantool.org 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();