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 7DEEF6EC40; Tue, 10 Aug 2021 15:31:07 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 7DEEF6EC40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1628598667; bh=3fWPdkSi2FiCU5inbGq4mcNaw0w0OZMC6hG1ZQy0Q4s=; h=To:References:Date:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=s8RDf3Lzi0UrjtyG83OySwJK2PfSRp2Dl1/d8iS5nL1lJT256VKnUGuHMsaj8Hosk QuwhHQvXfko4JrJcTW7VZd/sQGWULuyYPJc/P0Npt6SN9vp5iJt2aIT2Y95myffl4p awASeiyrJhuNCW4LClBopmi9VYDw3jQKSxTSdvBw= Received: from smtp35.i.mail.ru (smtp35.i.mail.ru [94.100.177.95]) (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 A6D986EC40 for ; Tue, 10 Aug 2021 15:31:05 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org A6D986EC40 Received: by smtp35.i.mail.ru with esmtpa (envelope-from ) id 1mDQuO-00061R-RW; Tue, 10 Aug 2021 15:31:05 +0300 To: Cyrill Gorcunov References: <20210804190752.488147-1-gorcunov@gmail.com> <20210804190752.488147-4-gorcunov@gmail.com> <7e881b83-478c-6bc0-615d-ade811da8471@tarantool.org> Message-ID: <2f3274e6-80c4-1d07-b739-c8d758447fe1@tarantool.org> Date: Tue, 10 Aug 2021 15:31:04 +0300 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.12.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-4EC0790: 10 X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD92087353F0EC44DD9ECFD080E047A606F6525B29142351271182A05F53808504010C9BF7D9529602E0AD2ECC241FFF8FEA42C45C2D55B363E154C137D24FF110B X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE75DF2B1F23425CAE5EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063769AE3176B9E099158638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D863BBEF5F9896FF3A7BA23711E7B6B6C8117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCECADA55FE5B58BB7A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F44604297287769387670735201E561CDFBCA1751F618001F51B5FD3F9D2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EE4B6963042765DA4B2D242C3BD2E3F4C64AD6D5ED66289B52698AB9A7B718F8C46E0066C2D8992A16725E5C173C3A84C30836ED9F10B4AA34BA3038C0950A5D36B5C8C57E37DE458B0BC6067A898B09E46D1867E19FE14079C09775C1D3CA48CF3D321E7403792E342EB15956EA79C166A417C69337E82CC275ECD9A6C639B01B78DA827A17800CE7E1BCFB2C0BE3F189731C566533BA786AA5CC5B56E945C8DA X-C1DE0DAB: 0D63561A33F958A585BBF7C2F9D8619ADC7A4C053632E663E469B2C123B43AA6D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA753177526CD55AFC11410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34D4E96E2A5B1100E0E9627F8ACBE47D90F7F1A1631156851B8DE827DB62E2EB0CE69EA6D1CCA0BC231D7E09C32AA3244CF9BB3D75C81F736EF0B2400CF76FE5D4E646F07CC2D4F3D8FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojGhQhWEp1aB8Wxe/0qGaaCQ== X-Mailru-Sender: 6C3E74F07C41AE94D32402E5012278FA807AA4937789805B4B8F17C77FB58F78CC7E417CA509C94007784C02288277CA03E0582D3806FB6A5317862B1921BA260ED6CFD6382C13A6112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v10 3/4] limbo: filter incoming synchro requests 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: Vladislav Shpilevoy via Tarantool-patches Reply-To: Vladislav Shpilevoy Cc: tml Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" >>>>> +/** >>>>> + * Common chain for any incoming packet. >>>>> + */ >>>>> +static int >>>>> +filter_in(struct txn_limbo *limbo, const struct synchro_request *req) >>>>> +{ >>>>> + (void)limbo; >>>> >>>> 6. So you have the filtering enabled dynamically in the limbo, but >>>> you do not use the limbo here? Why? Maybe at least add an assertion >>>> that the filter is enabled? >>> >>> All chains are having same interface it is just happen that for common >>> filter I don't need to use limbo. I could add some operations here >>> but not sure if it worth it. As far as I see leave unused args is >>> pretty fine in our code base. >> >> You didn't answer the second question: >> >> Maybe at least add an assertion that the filter is enabled? > > I did > > | I could add some operations here but not sure if it worth it. > > Letme state it clear then - I could add this assert() if you insist > but I think we aready spread too many assertions all over the code, > and if it is possible I would be glad not to add new ones. After all > either we should add this assert() to each filter chain or not add > at all, otherwise there will be kind of code imbalance. What is wrong with the assertions that you don't like adding them? You add panics quite often, and they cost some perf. But asserts just help to catch bugs and cost nothing in Release build. >>>>> +/** >>>>> + * Filter CONFIRM and ROLLBACK packets. >>>>> + */ >>>>> +static int >>>>> +filter_confirm_rollback(struct txn_limbo *limbo, >>>>> + const struct synchro_request *req) >>>>> +{ >>>>> + /* >>>>> + * When limbo is empty we have nothing to >>>>> + * confirm/commit and if this request comes >>>>> + * in it means the split brain has happened. >>>>> + */ >>>>> + if (!txn_limbo_is_empty(limbo)) >>>>> + return 0; >>>> >>>> 9. What if rollback is for LSN > limbo's last LSN? It >>>> also means nothing to do. The same for confirm LSN < limbo's >>>> first LSN. >>> >>> static void >>> txn_limbo_read_rollback(struct txn_limbo *limbo, int64_t lsn) >>> { >>> --> assert(limbo->owner_id != REPLICA_ID_NIL || txn_limbo_is_empty(limbo)); >>> >>> txn_limbo_read_confirm(struct txn_limbo *limbo, int64_t lsn) >>> { >>> --> assert(limbo->owner_id != REPLICA_ID_NIL || txn_limbo_is_empty(limbo)); >>> >>> Currently we're allowed to process empty limbo if only owner is not nil, >>> I think I should add this case here. >> >> My question is not about the owner ID. I asked what if rollback/confirm >> try to cover a range not present in the limbo while it is not empty. If >> it is not empty, it has an owner obviously. But it does not matter. >> What if it has an owner, has transactions, but you got ROLLBACK/CONFIRM >> for data out of the LSN range present in the limbo? > > Since the terms are matching I think such scenarion should be fine, right? > IOW, some old replica has been stopped for some reason and been living out > of quorum for some time thus such requests should be considered as OK to > pass and when filter accepts them the will reach txn_limbo_read_confirm > or txn_limbo_read_rollback where they will be simply ignored as far as I > unrestand. IOW, such requests are valid, no? If a replica is outdated, it should not matter. It will receive the needed data in order anyway. Like if the data was just sent. Hence, it seems irrelevant whether it is outdated. And still looks the same as the thing you are trying to filter out (when the limbo is empty = confirm/rollback do not cover anything too).