From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 785F06F87A; Fri, 16 Apr 2021 18:40:06 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 785F06F87A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1618587606; bh=DQ4LnkHY31g5emqphy1fqibSf93wzRV3lOgxCi1ekAg=; h=To:Cc:References:Date:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=KTCo4ZMGCsJUSdPpd0k+NOiAIB4j/NfdIuahInK2INog4WaPYUnlikuu0HD9nDWDY p5qHY3qS5MMQBFhZunVEE0DYd3n9LCmtqfDLLpN+h7XlnHD38nVfWdg+M6Ejh1789W T3zmnwP0kS/XRb9MScd8rNwqDeWsTneJKubhVlHA= Received: from smtp58.i.mail.ru (smtp58.i.mail.ru [217.69.128.38]) (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 7DEC96F87A for ; Fri, 16 Apr 2021 18:40:05 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 7DEC96F87A Received: by smtp58.i.mail.ru with esmtpa (envelope-from ) id 1lXQZg-0008K7-M8; Fri, 16 Apr 2021 18:40:05 +0300 To: Vladislav Shpilevoy , gorcunov@gmail.com Cc: tarantool-patches@dev.tarantool.org References: Message-ID: <815ca075-7d25-a584-b75a-9a4a28e6cdaf@tarantool.org> Date: Fri, 16 Apr 2021 18:40:04 +0300 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.9.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-GB X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD92FFCB8E6708E7480D608FE24BC85426BB1B55F651FED8C70182A05F5380850404B1AD5D0CEAAB537FFB448B72DBEE9FF1F24591E967D1FC9E5271E342EA40AC8 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE78FED028BAF25EB9BEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637B103A303C52566448638F802B75D45FF914D58D5BE9E6BC1A93B80C6DEB9DEE97C6FB206A91F05B2402DC3BFC7ED30FB7ED656BD0206BB57BDFBBEFFF4125B51D2E47CDBA5A96583C09775C1D3CA48CFE478A468B35FE767117882F4460429724CE54428C33FAD30A8DF7F3B2552694AC26CFBAC0749D213D2E47CDBA5A9658378DA827A17800CE78A0F7C24A37A3D769FA2833FD35BB23DF004C90652538430302FCEF25BFAB3454AD6D5ED66289B5278DA827A17800CE7921071A8AEF1278CD32BA5DBAC0009BE395957E7521B51C20BC6067A898B09E4090A508E0FED6299176DF2183F8FC7C0044B9F2C9FBC7C0DCD04E86FAF290E2D7E9C4E3C761E06A71DD303D21008E298D5E8D9A59859A8B6B372FE9A2E580EFC725E5C173C3A84C3CE9959E2676FD87735872C767BF85DA2F004C90652538430E4A6367B16DE6309 X-C1DE0DAB: 0D63561A33F958A511BF0B58041059A45B1924881D5B2DAA6EB4D534B4A6677DD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7502E6951B79FF9A3F410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34C1E32F4AD4B2486B217E51C814C9202DDDC22C1A8F39B862BB19A2EA107B22848109748E83B528171D7E09C32AA3244CA769597D7049242B9F6CC91EB57437023C6EB905E3A8056BFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj3S6P1v0GIqRzj2tJaw2bPg== X-Mailru-Sender: 583F1D7ACE8F49BDD2846D59FC20E9F8E07881E9C51BF8B21A6AD08D0FD726A4ACCF5FD9644812B1424AE0EB1F3D1D21E2978F233C3FAE6EE63DB1732555E4A8EE80603BA4A5B0BC112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v3 08/10] Support manual elections in `box.ctl.clear_synchro_queue()` X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Serge Petrenko via Tarantool-patches Reply-To: Serge Petrenko Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" 16.04.2021 18:38, Serge Petrenko via Tarantool-patches пишет: > > > 16.04.2021 02:30, Vladislav Shpilevoy пишет: >>> diff --git a/src/box/raft.h b/src/box/raft.h >>> index 15f4e80d9..8fce423e1 100644 >>> --- a/src/box/raft.h >>> +++ b/src/box/raft.h >>> @@ -97,6 +97,9 @@ box_raft_checkpoint_remote(struct raft_request *req); >>>   int >>>   box_raft_process(struct raft_request *req, uint32_t source); >>>   +void >>> +box_raft_wait_leader_found(); >>> + >>>   void >>>   box_raft_init(void); >>>   diff --git a/src/lib/raft/raft.c b/src/lib/raft/raft.c >>> index e9ce8cade..7b77e05ea 100644 >>> --- a/src/lib/raft/raft.c >>> +++ b/src/lib/raft/raft.c >>> @@ -846,7 +846,7 @@ raft_cfg_is_enabled(struct raft *raft, bool >>> is_enabled) >>>   } >>>     void >>> -raft_cfg_is_candidate(struct raft *raft, bool is_candidate) >>> +raft_cfg_is_candidate(struct raft *raft, bool is_candidate, bool >>> demote) >> 5. I know it might lead to some code duplication, but probably >> better move that to other functions. For example, >> >>     raft_cfg_is_temporary_candidate() >> >> or something like that. Otherwise it appears surprisingly hard >> to follow these 2 flags together. Although I might be wrong and >> it would look worse. Did you try? >> >> Or another option: >> >>     raft_cfg_is_candidate(box_raft(), true, false); >>     raft_cfg_is_candidate(box_raft(), false, false); >> >> turns into >> >>     raft_start_candidate(box_raft()) >>     raft_stop_candidate(box_raft()) >> >> Also it would be good to have unit tests for the changes in raft.h >> and raft.c. > > This variant sounds good. I'll implement in in a new commit. The commit I was talking about: ===================================================== commit 79940c7b20a4acefaa5984550fee2872a58fef0c Author: Serge Petrenko Date:   Fri Apr 16 18:22:28 2021 +0300     raft: introduce raft_start/stop_candidate     Extract raft_start_candidate and raft_stop_candidate functions from     raft_cfg_is_candidate.     These functions will be used in manual elections.     Prerequisite #3055 diff --git a/src/lib/raft/raft.c b/src/lib/raft/raft.c index e9ce8cade..8deb06eb5 100644 --- a/src/lib/raft/raft.c +++ b/src/lib/raft/raft.c @@ -848,38 +848,59 @@ raft_cfg_is_enabled(struct raft *raft, bool is_enabled)  void  raft_cfg_is_candidate(struct raft *raft, bool is_candidate)  { -    bool old_is_candidate = raft->is_candidate;      raft->is_cfg_candidate = is_candidate; -    raft->is_candidate = is_candidate && raft->is_enabled; -    if (raft->is_candidate == old_is_candidate) -        return; +    is_candidate = is_candidate && raft->is_enabled; +    if (is_candidate) +        raft_start_candidate(raft); +    else +        raft_stop_candidate(raft, true); +} -    if (raft->is_candidate) { -        assert(raft->state == RAFT_STATE_FOLLOWER); -        if (raft->is_write_in_progress) { -            /* -             * If there is an on-going WAL write, it means there was -             * some node who sent newer data to this node. So it is -             * probably a better candidate. Anyway can't do anything -             * until the new state is fully persisted. -             */ -        } else if (raft->leader != 0) { -            raft_sm_wait_leader_dead(raft); -        } else { -            raft_sm_wait_leader_found(raft); -        } +void +raft_start_candidate(struct raft *raft) +{ +    if (raft->is_candidate) +        return; +    raft->is_candidate = true; +    assert(raft->state == RAFT_STATE_FOLLOWER); +    if (raft->is_write_in_progress) { +        /* +         * If there is an on-going WAL write, it means there was +         * some node who sent newer data to this node. So it is +         * probably a better candidate. Anyway can't do anything +         * until the new state is fully persisted. +         */ +    } else if (raft->leader != 0) { +        raft_sm_wait_leader_dead(raft);      } else { -        if (raft->state != RAFT_STATE_LEADER) { -            /* Do not wait for anything while being a voter. */ -            raft_ev_timer_stop(raft_loop(), &raft->timer); -        } -        if (raft->state != RAFT_STATE_FOLLOWER) { -            if (raft->state == RAFT_STATE_LEADER) -                raft->leader = 0; -            raft->state = RAFT_STATE_FOLLOWER; -            /* State is visible and changed - broadcast. */ -            raft_schedule_broadcast(raft); +        raft_sm_wait_leader_found(raft); +    } +} + +void +raft_stop_candidate(struct raft *raft, bool demote) +{ +    if (!raft->is_candidate) +        return; +    raft->is_candidate = false; +    if (raft->state != RAFT_STATE_LEADER) { +        /* Do not wait for anything while being a voter. */ +        raft_ev_timer_stop(raft_loop(), &raft->timer); +    } +    if (raft->state != RAFT_STATE_FOLLOWER) { +        if (raft->state == RAFT_STATE_LEADER) { +            if (!demote) { +                /* +                 * Remain leader until someone +                 * triggers new elections. +                 */ +                return; +            } +            raft->leader = 0;          } +        raft->state = RAFT_STATE_FOLLOWER; +        /* State is visible and changed - broadcast. */ +        raft_schedule_broadcast(raft);      }  } diff --git a/src/lib/raft/raft.h b/src/lib/raft/raft.h index a5f7e08d9..69dec63c6 100644 --- a/src/lib/raft/raft.h +++ b/src/lib/raft/raft.h @@ -327,6 +327,19 @@ raft_cfg_is_enabled(struct raft *raft, bool is_enabled);  void  raft_cfg_is_candidate(struct raft *raft, bool is_candidate); +/** + * Make the instance a candidate. + */ +void +raft_start_candidate(struct raft *raft); + +/** + * Make the instance stop taking part in new elections. + * @param demote whether to stop being a leader immediately or not. + */ +void +raft_stop_candidate(struct  raft *raft, bool demote); +  /** Configure Raft leader election timeout. */  void  raft_cfg_election_timeout(struct raft *raft, double timeout); diff --git a/test/unit/raft.c b/test/unit/raft.c index 0306cefcd..575886932 100644 --- a/test/unit/raft.c +++ b/test/unit/raft.c @@ -1296,15 +1296,43 @@ raft_test_term_filter(void)      ok(!raft_is_node_outdated(&node.raft, 3), "node doesn't become "                            "outdated"); -      raft_node_destroy(&node);      raft_finish_test();  } +static void +raft_test_start_stop_candidate(void) +{ +    raft_start_test(4); +    struct raft_node node; +    raft_node_create(&node); + +    raft_node_cfg_is_candidate(&node, false); +    raft_node_cfg_election_quorum(&node, 1); + +    raft_start_candidate(&node.raft); +    raft_run_next_event(); +    is(node.raft.state, RAFT_STATE_LEADER, "became leader after " +                           "start_candidate"); +    raft_stop_candidate(&node.raft, false); +    raft_run_for(node.cfg_death_timeout); +    is(node.raft.state, RAFT_STATE_LEADER, "remain leader after " +                           "stop_candidate"); + +    is(raft_node_send_vote_request(&node, +        3 /* Term. */, +        "{}" /* Vclock. */, +        2 /* Source. */ +    ), 0, "vote request from 2"); +    is(node.raft.state, RAFT_STATE_FOLLOWER, "demote once new election " +                         "starts"); +    raft_finish_test(); +} +  static int  main_f(va_list ap)  { -    raft_start_test(14); +    raft_start_test(15);      (void) ap;      fakeev_init(); @@ -1323,6 +1351,7 @@ main_f(va_list ap)      raft_test_enable_disable();      raft_test_too_long_wal_write();      raft_test_term_filter(); +    raft_test_start_stop_candidate();      fakeev_free(); diff --git a/test/unit/raft.result b/test/unit/raft.result index ecb962e42..bb799936b 100644 --- a/test/unit/raft.result +++ b/test/unit/raft.result @@ -1,5 +1,5 @@      *** main_f *** -1..14 +1..15      *** raft_test_leader_election ***      1..24      ok 1 - 1 pending message at start @@ -233,4 +233,12 @@ ok 13 - subtests      ok 9 - node doesn't become outdated  ok 14 - subtests      *** raft_test_term_filter: done *** +    *** raft_test_start_stop_candidate *** +    1..4 +    ok 1 - became leader after start_candidate +    ok 2 - remain leader after stop_candidate +    ok 3 - vote request from 2 +    ok 4 - demote once new election starts +ok 15 - subtests +    *** raft_test_start_stop_candidate: done ***      *** main_f: done *** -- Serge Petrenko