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 EC4756EC58; Wed, 26 May 2021 11:25:31 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org EC4756EC58 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1622017532; bh=5xKn7km3OxHoBnbP7p9RJXYHjeDytxjfYe8qcpJgD4I=; 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=VIIk0zCYOFLY75fN54ZThDBoM1PET+7asVDEHNoU1c/R05c+pXD8drudG5babPkPM McgVOFf1BTvZZXfOZTO+fZmM6ZtrH3kjDFYHaNaCjvEOJ9hNReQd1iw1GUZePueeeI +VsGUAd6uw3NGdslzdsv7DIVKMEOSTTfI69+Uneg= Received: from smtp40.i.mail.ru (smtp40.i.mail.ru [94.100.177.100]) (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 2AEB36EC58 for ; Wed, 26 May 2021 11:25:31 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 2AEB36EC58 Received: by smtp40.i.mail.ru with esmtpa (envelope-from ) id 1llor4-0007Mp-DJ; Wed, 26 May 2021 11:25:30 +0300 To: Cyrill Gorcunov Cc: v.shpilevoy@tarantool.org, tarantool-patches@dev.tarantool.org References: <8011f87bb9b5e1f53f5bee3124f3a8e9dbe1917c.1621935783.git.sergepetrenko@tarantool.org> Message-ID: Date: Wed, 26 May 2021 11:25:29 +0300 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.10.2 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: 4F1203BC0FB41BD9F4D89F53055293B7B476AE2F0FE38070A482055F3B604B06182A05F53808504099486A20DC648D6C507D604319CD4684A852A0E9323F5BB1A8231679B0B1C38F X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7956F10FFCC7409BAEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006375E347E1311495C1B8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8765E51D0FB7F4D8AED523D24B63A8C2E117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCF1175FABE1C0F9B6A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD182CC0D3CB04F14752D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B682BBBAF5DF00056E089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: 0D63561A33F958A52CA0BA418E66E73524EC16B8229C0972AAE34E8AE0CBEC7CD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75E3127721F5A72C97410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D346AD04FDA812EEFF18216DF688E29D1896847EDF9ECD225EF6E3246BB011B66377D0D4118EC6DCD9D1D7E09C32AA3244CA7BB591721F888BE540C2FD2C21DBB4DB4DF56057A86259FFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojiWl9Rlzdedm/zObKZEdrDQ== X-Mailru-Sender: 3B9A0136629DC9125D61937A2360A446B30367083DCD50ED5E7E78D1F6391FB14CE31D7B12C19F07424AE0EB1F3D1D21E2978F233C3FAE6EE63DB1732555E4A8EE80603BA4A5B0BC112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v2 2/2] box: fix an assertion failure in box.ctl.promote() 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" 26.05.2021 09:14, Cyrill Gorcunov пишет: > On Tue, May 25, 2021 at 01:39:29PM +0300, Serge Petrenko wrote: Hi! Thanks for the review! >> box.ctl.promote() used to assume that the last synchronous entry is >> already written to WAL by the time it's called. This is not the case >> when promote is executed on the limbo owner. The last synchronous entry >> might still be en route to WAL. > Typo "en" -> "in". No, "en route" means в пути/по пути. > >> @@ -1618,14 +1618,29 @@ box_promote(void) >> txn_limbo.owner_id); >> return -1; >> } >> + if (txn_limbo_is_empty(&txn_limbo)) { >> + wait_lsn = txn_limbo.confirmed_lsn; >> + goto promote; >> + } >> } >> >> - /* >> - * promote() is a no-op on the limbo owner, so all the rows >> - * in the limbo must've come through the applier meaning they already >> - * have an lsn assigned, even if their WAL write hasn't finished yet. >> - */ >> - wait_lsn = txn_limbo_last_synchro_entry(&txn_limbo)->lsn; >> + struct txn_limbo_entry *last_entry; >> + last_entry = txn_limbo_last_synchro_entry(&txn_limbo); >> + /* Wait for the last entries WAL write. */ >> + if (last_entry->lsn < 0) { >> + if (wal_sync(NULL) < 0) >> + return -1; >> + if (txn_limbo_is_empty(&txn_limbo)) { >> + wait_lsn = txn_limbo.confirmed_lsn; >> + goto promote; >> + } >> + if (last_entry != txn_limbo_last_synchro_entry(&txn_limbo)) { > This is a bit dangerous. We cache a pointer and then go to fiber_yield, > which switches context, at this moment the pointer become dangling one > and we simply can't be sure if it _were_ reused. IOW, Serge are we > 100% sure that the same pointer with same address but with new data > won't appear here as last entry in limbo? I agree this solution is not perfect. An alternative would be to do the following: 1) Check that the limbo owner hasn't changed 2) Check that the last entry has positive lsn (e.g. it's not a new entry which     wasn't yet written to WAL). And that this lsn is equal to the lsn of our entry. But what if our entry was confirmed and destroyed during wal_sync()? We can't compare other entries lsn with this ones. > >> + diag_set(ClientError, ER_QUORUM_WAIT, quorum, >> + "new synchronous transactions appeared"); >> + return -1; >> + } >> + } >> + wait_lsn = last_entry->lsn; >> assert(wait_lsn > 0); -- Serge Petrenko