From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp56.i.mail.ru (smtp56.i.mail.ru [217.69.128.36]) (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 CAA5E45C304 for ; Fri, 27 Nov 2020 16:00:08 +0300 (MSK) References: <20201124131820.32981-1-sergepetrenko@tarantool.org> From: Serge Petrenko Message-ID: Date: Fri, 27 Nov 2020 16:00:07 +0300 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: 8bit Content-Language: en-GB Subject: Re: [Tarantool-patches] [PATCH] raft: make sure the leader stays ro till it clears the limbo List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy , gorcunov@gmail.com Cc: tarantool-patches@dev.tarantool.org 27.11.2020 00:01, Vladislav Shpilevoy пишет: > Hi! Thanks for the patch! > > On 24.11.2020 14:18, Serge Petrenko wrote: >> When running a cluster with leader election, its useful to wait till the >> instance is writeable to determine that it has become a leader. However, >> sometimes the instance fails to write data right after transitioning to >> leader because its limbo still contains pending transactions from the >> old leader. Make sure the instance deals with pending transactions first >> and becomes writeable only once the limbo is empty. > I just realized one thing. We can add a function txn_limbo_is_ro(), > like we did with raft_is_ro(), account it in box_update_ro_summary(), > and call box_update_ro_summary() when we see that the limbo is emptied, > or when its ownership changes to a different instance. > > Probably would be simpler, and also we could make it work with manual > election! So users could call box.ctl.wait_rw() even without using raft! > > To show concrete error if somebody still tries to write, we could > patch box_check_writable() to show the reason why the instance is not > writable. We will do it anyway for raft, to tell the users the real > leader in case they are trying to write on a replica. In scope of > https://github.com/tarantool/tarantool/issues/5568. > > Your version of the patch also looks good. > > What do you think? Thanks for your answer! Your proposal looks good. One question though. What about multimaster synchro? Are we planning to support it one day? If yes, then limbo emptiness will mean nothing. So, there're two options: 1) we may leave this patch as is. Then one won't be    able to call wait_rw() with manual election. That's a pity, since    your proposal looks quite logical, especially from the user's point    of view. Having a single error for all these cases would be good. 2) Make limbo affect is_ro. Then everything's good for now, but we'll    have to rewrite it back once (and if) we decide to implement    multimaster synchro. Then the patch will look exactly like it does    now. I'm not sure whether we're planning to make multimaster synchro work, so I can't choose between these options and leave you to decide. Besides, looks like if we take option 2 the patch will differ in a single line: `box_update_ro_summary()` after `box_clear_synchro_queue` -- Serge Petrenko