Re: [ADMIN] pg_basebackup blocking all queries with horrible performance

On Mon, Jun 11, 2012 at 6:06 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
> On Tue, Jun 12, 2012 at 12:47 AM, Magnus Hagander <magnus@hagander.net> wrote:
>> On Mon, Jun 11, 2012 at 5:37 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
>>> On Mon, Jun 11, 2012 at 3:24 AM, Magnus Hagander <magnus@hagander.net> wrote:
>>>> On Sun, Jun 10, 2012 at 6:08 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
>>>>> On Sun, Jun 10, 2012 at 11:45 PM, Magnus Hagander <magnus@hagander.net> wrote:
>>>>>> On Sun, Jun 10, 2012 at 4:29 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
>>>>>>> On Sun, Jun 10, 2012 at 11:10 PM, Magnus Hagander <magnus@hagander.net> wrote:
>>>>>>>> On Sun, Jun 10, 2012 at 4:08 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
>>>>>>>>> On Sun, Jun 10, 2012 at 10:34 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
>>>>>>>>>> On Sun, Jun 10, 2012 at 9:25 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
>>>>>>>>>>> On Sun, Jun 10, 2012 at 7:43 PM, Magnus Hagander <magnus@hagander.net> wrote:
>>>>>>>>>>>> On Sat, Jun 9, 2012 at 2:51 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>>>>>>>>>>>> Fujii Masao <masao.fujii@gmail.com> writes:
>>>>>>>>>>>>>> This seems a bug. I think we should prevent pg_basebackup from
>>>>>>>>>>>>>> becoming synchronous standby. Thought?
>>>>>>>>>>>>>
>>>>>>>>>>>>> Absolutely.  If we have replication clients that are not actually
>>>>>>>>>>>>> capable of being standbys, there *must* be a way for the master
>>>>>>>>>>>>> to know that.
>>>>>>>>>>>>
>>>>>>>>>>>> I thought we fixed this already by sending InvalidXlogRecPtr as flush
>>>>>>>>>>>> location? And that this only applied in 9.2?
>>>>>>>>>>>>
>>>>>>>>>>>> Are you saying we picked pg_basebackup *in backup mode* (not log
>>>>>>>>>>>> streaming) as synchronous standby?
>>>>>>>>>>>
>>>>>>>>>>> Yes.
>>>>>>>>>>>
>>>>>>>>>>>> If so then yes, that is
>>>>>>>>>>>> *definitely* a bug that should be fixed. We should never select a
>>>>>>>>>>>> connection that's not even streaming log as standby!
>>>>>>>>>>>
>>>>>>>>>>> Agreed. Attached patch prevents pg_basebackup from becoming sync
>>>>>>>>>>> standby. Also this patch fixes another problem: currently only walsender
>>>>>>>>>>> which reaches STREAMING state can become sync walsender. OTOH,
>>>>>>>>>>> sync walsender thinks that walsender with higher priority will be sync one
>>>>>>>>>>> whether its state is STREAMING, and switches to potential sync walsender.
>>>>>>>>>>> So when the standby with higher priority connects to the master, we
>>>>>>>>>>> might have no sync standby until it reaches the STREAMING state.
>>>>>>>>>>> To fix this problem, the patch switches walsender's state from sync to
>>>>>>>>>>> potential *after* walsender with higher priority has reached the
>>>>>>>>>>> STREAMING state.
>>>>>>>>>>>
>>>>>>>>>>> We also should not select (1) background stream process forked from
>>>>>>>>>>> pg_basebackup and (2) pg_receivexlog as sync standby because they
>>>>>>>>>>> don't send back replication progress. To address this, I'm thinking to
>>>>>>>>>>> introduce new option "NOSYNC" in "START_REPLICATION" command
>>>>>>>>>>> as follows, and to change (1) and (2) so that they specify NOSYNC.
>>>>>>>>>>>
>>>>>>>>>>>    START_REPLICATION XXX/XXX [NOSYNC]
>>>>>>>>>>>
>>>>>>>>>>> If the standby specifies NOSYNC option, it's never assigned as sync
>>>>>>>>>>> standby even if its name is in synchronous_standby_names. Thought?
>>>>>>>>>>
>>>>>>>>>> The standby which always sends InvalidXLogRecPtr back should not
>>>>>>>>>> become sync one. So instead of NOSYNC option, by checking whether
>>>>>>>>>> InvalidXLogRecPtr is sent, we can avoid problematic sync standby.
>>>>>>>>>
>>>>>>>>> We should not do this because Magnus is proposing the patch
>>>>>>>>> (http://archives.postgresql.org/pgsql-hackers/2012-06/msg00348.php)
>>>>>>>>> which breaks the above assumption at all. So we should introduce
>>>>>>>>> something like NOSYNC option.
>>>>>>>>
>>>>>>>> Wouldn't the better choice there in that case be to give a switch to
>>>>>>>> pg_receivexlog if you *want* it to be able to become a sync replica,
>>>>>>>> and by default disallow it? And then keep the backend just treating
>>>>>>>> InvalidXlogRecPtr as don't-become-sync-replica.
>>>>>>>
>>>>>>> I don't object to making pg_receivexlog as sync standby at all. So at least
>>>>>>> for me, that switch is not necessary. What I'm worried about is the
>>>>>>> background stream process forked from pg_basebackup. I think that
>>>>>>> it should not run as sync standby but sending back its replication progress
>>>>>>> seems helpful because a user can see the progress from pg_stat_replication.
>>>>>>> So I'm thinking that something like NOSYNC option is required.
>>>>>>
>>>>>> On principle, no. By default, yes.
>>>>>>
>>>>>> How about:
>>>>>> pg_basebackup background: *never* sends flush location, and therefor
>>>>>> won't become sync replica
>>>>>> pg_receivexlog *optionally* sends flush location. by defualt own't
>>>>>> become sync replica, but can be made so with a switch
>>>>>
>>>>> Wouldn't a user who sees NULL in flush_location from pg_stat_replication
>>>>> misunderstand that pg_receivexlog (in default mode) and pg_basebackup
>>>>> background don't flush WAL files at all?
>>>>
>>>> That sounds like a "documentable issue".
>>>>
>>>> But maybe you're right, and we need the "never become sync" as a flag.
>>>
>>> You agreed to add something like NOSYNC option into START_REPLICATION command?
>>
>> I'm on the fence. I was hoping somebody else would chime in with an
>> opinion as well.
>
> +1

Nobody else with any opinion on this? :(

--
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/