Tarantool development patches archive
 help / color / mirror / Atom feed
From: Serge Petrenko <sergepetrenko@tarantool.org>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>, gorcunov@gmail.com
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH] raft: make sure the leader stays ro till it clears the limbo
Date: Mon, 30 Nov 2020 12:40:28 +0300	[thread overview]
Message-ID: <459c1422-cb8d-45df-742f-4f720e8ec9e7@tarantool.org> (raw)
In-Reply-To: <22c104d3-bb7b-fd4d-ebd0-730133e8ded9@tarantool.org>


28.11.2020 01:10, Vladislav Shpilevoy пишет:
>>> 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.
> Indeed. But I have no idea if we will ever support it, and if
> yes - when. It is possible in theory, but we never tried to
> elaborate so far. I wouldn't expect it happening in the next
> year or so.
Ok, I see.
>> 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.
> It also bothers me, that now box.ctl.wait_rw() actually does not wait
> for rw, strictly speaking. So it is probably even a bug, not a feature.
> A user can get wait_rw() true, but still won't be able to write.
> Value of such helper becomes zero with synchronous replication, in
> essence.


Ok.


>
>> 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.
> Perhaps. If nothing will change in raft and limbo significantly.
>
>> 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.
> We discuss it, but there is no really a plan. No customer request, and
> no ticket AFAIR. I assume some big customer must ask for this for us to
> even start planning.
>
>> 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`
> Why? You will need to patch the limbo as well, not just remove
> box_update_ro_summary from after box_clear_synchro_queue. We will
> need to add a method like txn_limbo_is_ro() (similar to raft_is_ro()),
> use it in box_update_ro_summary(), and call the latter from some
> places in the limbo.


Never mind, I don't know what I was thinking at the time of writing that.


>
> I vote for the option 2. Mostly because I am afraid it is rather a bug.
> wait_rw just does not guarantee anything now.


Done. Please see "[PATCH v2] box: make instace ro while limbo is not empty"

-- 
Serge Petrenko

      reply	other threads:[~2020-11-30  9:40 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-24 13:18 Serge Petrenko
2020-11-24 14:14 ` Cyrill Gorcunov
2020-11-25  8:48   ` Serge Petrenko
2020-11-26 21:01 ` Vladislav Shpilevoy
2020-11-27 13:00   ` Serge Petrenko
2020-11-27 22:10     ` Vladislav Shpilevoy
2020-11-30  9:40       ` Serge Petrenko [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=459c1422-cb8d-45df-742f-4f720e8ec9e7@tarantool.org \
    --to=sergepetrenko@tarantool.org \
    --cc=gorcunov@gmail.com \
    --cc=tarantool-patches@dev.tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH] raft: make sure the leader stays ro till it clears the limbo' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox