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 99D2C6EC58; Mon, 21 Jun 2021 18:09:46 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 99D2C6EC58 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1624288186; bh=uVwk099xPJQjuJwrcPRnRkOxmZ6mqY6ffjDFH6U+zFI=; 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=GGfQW7BHLJd3CJbZyq1yCBKLa6Wp6oVZewSg8UtisLTc5nUijs12jHi0u+kHZutos pnLeUAM9tBCQ85ORS0oHclEvfPlVD4EAsfP7mWyZxLHqkPC30Pgd0gK5SuuQ+akJkT jtSr2JIskOFwibLE7XmREdix2U1Vw71OME46qQFs= Received: from smtp36.i.mail.ru (smtp36.i.mail.ru [94.100.177.96]) (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 DDE016EC58 for ; Mon, 21 Jun 2021 18:09:43 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org DDE016EC58 Received: by smtp36.i.mail.ru with esmtpa (envelope-from ) id 1lvLYV-00019L-5q; Mon, 21 Jun 2021 18:09:43 +0300 To: Cyrill Gorcunov Cc: v.shpilevoy@tarantool.org, tarantool-patches@dev.tarantool.org References: Message-ID: Date: Mon, 21 Jun 2021 18:09:42 +0300 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 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: 78E4E2B564C1792B X-77F55803: 4F1203BC0FB41BD91C2C07775F13263A48BAE59A842244EB7433BA0B4B73D52900894C459B0CD1B9DB91D3F7102B0F692E0EA71AC72F77A6D3A0E7E64A993AA78483CDA5FC7E7E3C X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7A140E7B1D51EB231EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006375121C8F3070B83748638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D876FCB65522862714A2D5033F6732753C117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC974A882099E279BDA471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD18E5D25F19253116ADD2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B6BF3059D42242344A089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: 0D63561A33F958A541BCCA9A9323D6F33CC41B2E04BD465C3681A7D20FB24BECD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75448CF9D3A7B2C848410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D3498910055B812BD9C8103B76ABB75CEC744BBB7CA56FBA37519678BD091A70EBA6B55B84D8530E69C1D7E09C32AA3244C2FF7C79437A688F72F2BC53910D36AC7435BF7150578642FFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj8x+Gb+jwA+QicJuGchrYHQ== X-Mailru-Sender: 3B9A0136629DC9125D61937A2360A446E5CC36347D00BEC51D27F2E68031D2AD79B38F0480470646424AE0EB1F3D1D21E2978F233C3FAE6EE63DB1732555E4A8EE80603BA4A5B0BC112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v2 2/8] txn_limbo: fix promote term filtering 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" 18.06.2021 13:31, Cyrill Gorcunov пишет: > On Fri, Jun 18, 2021 at 12:07:36AM +0300, Serge Petrenko wrote: >> txn_limbo_process() used to filter out promote requests whose term was >> equal to the greatest term seen. This wasn't correct for PROMOTE entries >> with term 1. >> >> Such entries appear after box.ctl.promote() is issued on an instance >> with disabled elections. Every PROMOTE entry from such an instance has >> term 1, but should still be applied. Fix this in the patch. >> >> Also, when an outdated PROMOTE entry with term smaller than already >> applied from some replica arrived, it wasn't filtered at all. Such a >> situation shouldn't be possible, but fix it as well. >> >> Part-of #6034 >> --- >> src/box/txn_limbo.c | 22 +++++++++++++--------- >> 1 file changed, 13 insertions(+), 9 deletions(-) >> >> diff --git a/src/box/txn_limbo.c b/src/box/txn_limbo.c >> index 51dc2a186..a5d1df00c 100644 >> --- a/src/box/txn_limbo.c >> +++ b/src/box/txn_limbo.c >> @@ -665,17 +665,21 @@ txn_limbo_process(struct txn_limbo *limbo, const struct synchro_request *req) >> { >> uint64_t term = req->term; >> uint32_t origin = req->origin_id; >> - if (txn_limbo_replica_term(limbo, origin) < term) { >> + if (txn_limbo_replica_term(limbo, origin) < term) >> vclock_follow(&limbo->promote_term_map, origin, term); >> - if (term > limbo->promote_greatest_term) { >> - limbo->promote_greatest_term = term; >> - } else if (req->type == IPROTO_PROMOTE) { >> - /* >> - * PROMOTE for outdated term. Ignore. >> - */ >> - return; >> - } >> + >> + if (term > limbo->promote_greatest_term) { >> + limbo->promote_greatest_term = term; >> + } else if (req->type == IPROTO_PROMOTE && >> + limbo->promote_greatest_term > 1) { >> + /* PROMOTE for outdated term. Ignore. */ >> + say_info("RAFT: ignoring PROMOTE request from instance " >> + "id %"PRIu32" for term %"PRIu64". Greatest term seen " >> + "before (%"PRIu64") is bigger.", origin, term, >> + limbo->promote_greatest_term); >> + return; >> } Thanks for the review! > Serge, here is a moment I don't understand. The promote_term_map > and promote_greatest_term both are zero initially, then we start > accepting requests from the net and previously we've been updating > promote_term_map and promote_greatest_term in same context, iow > promote_term_map is filtering incoming traffic and update > promote_greatest_term with max from promote_term_map, thus > we can denote it as > > if (term > promote_term_map[i]) { > promote_term_map[i] = term; > promote_greatest_term = max(promote_greatest_term, term); > } > > thus promote_greatest_term always = max({promote_term_map[i]}), so I don't > understart why say > > uint64_t term = req->term; > uint32_t origin = req->origin_id; > > if (txn_limbo_replica_term(limbo, origin) < term) { > vclock_follow(&limbo->promote_term_map, origin, term); > if (term > limbo->promote_greatest_term) > limbo->promote_greatest_term = term; > } > > if (iproto_type_is_promote_request(req->type) && > limbo->promote_greatest_term > 1) { > /* PROMOTE for outdated term. Ignore. */ > say_info("RAFT: ignoring %s request from instance " > "id %"PRIu32" for term %"PRIu64". Greatest term seen " > "before (%"PRIu64") is bigger.", > iproto_type_name(req->type), origin, term, > limbo->promote_greatest_term); > return; > } > > won't do the trick? Do I miss something obvious here? It will, thanks for the suggestion! I applied your changes: ========================== diff --git a/src/box/txn_limbo.c b/src/box/txn_limbo.c index e91782b40..16181b8a0 100644 --- a/src/box/txn_limbo.c +++ b/src/box/txn_limbo.c @@ -665,11 +665,10 @@ txn_limbo_process(struct txn_limbo *limbo, const struct synchro_request *req)  {         uint64_t term = req->term;         uint32_t origin = req->origin_id; -       if (txn_limbo_replica_term(limbo, origin) < term) +       if (txn_limbo_replica_term(limbo, origin) < term) {                 vclock_follow(&limbo->promote_term_map, origin, term); - -       if (term > limbo->promote_greatest_term) { -               limbo->promote_greatest_term = term; +               if (term > limbo->promote_greatest_term) +                       limbo->promote_greatest_term = term;         } else if (req->type == IPROTO_PROMOTE &&                    limbo->promote_greatest_term > 1) {                 /* PROMOTE for outdated term. Ignore. */ ========================== -- Serge Petrenko