Re: Support for N synchronous standby servers - take 2

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
Cc: sawada(dot)mshk(at)gmail(dot)com, Masao Fujii <masao(dot)fujii(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, thom(at)linux(dot)com, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, memissemerson(at)gmail(dot)com, Josh Berkus <josh(at)agliodbs(dot)com>, amit(dot)kapila16(at)gmail(dot)com, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Support for N synchronous standby servers - take 2
Date: 2016-02-10 09:36:43
Message-ID: CAB7nPqTHmuuDdKWmoaY1ZAi-gRnT_HRdHGyiqpNfFFr15qc5uA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Feb 10, 2016 at 5:34 PM, Kyotaro HORIGUCHI
<horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>
> Hello,
>
> At Wed, 10 Feb 2016 15:22:44 +0900, Michael Paquier <michael(dot)paquier(at)gmail(dot)com> wrote in <CAB7nPqRk4ZjoQfs4rmF6Di1zp=b4eA=hk0L4GFzUj47GwhgM7g(at)mail(dot)gmail(dot)com>
> > On Wed, Feb 10, 2016 at 3:13 PM, Michael Paquier
> > <michael(dot)paquier(at)gmail(dot)com> wrote:
> > > On Wed, Feb 10, 2016 at 11:25 AM, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
> > > wrote:
> > > I am personally fine with () and [] as you mention, we could even consider
> > > {}, each one of them has a different meaning mathematically..
> > >
> > > I am not entered into a detailed review yet (waiting for the docs), but the
> > > patch looks brittle. I have been able to crash the server just by querying
> > > pg_stat_replication:
> > > * thread #1: tid = 0x0000, 0x0000000105eb36c2
> > > postgres`pg_stat_get_wal_senders(fcinfo=0x00007fff5a156290) + 498 at
> > > walsender.c:2783, stop reason = signal SIGSTOP
> > > * frame #0: 0x0000000105eb36c2
> > > postgres`pg_stat_get_wal_senders(fcinfo=0x00007fff5a156290) + 498 at
> > > walsender.c:2783
> > > frame #1: 0x0000000105d4277d
> > > postgres`ExecMakeTableFunctionResult(funcexpr=0x00007fea128f3838,
> > > econtext=0x00007fea128f1b58, argContext=0x00007fea128c8ea8,
> > > expectedDesc=0x00007fea128f4710, randomAccess='\0') + 1005 at
> > > execQual.c:2211
> > > frame #2: 0x0000000105d70c24
> > > postgres`FunctionNext(node=0x00007fea128f2f78) + 180 at
> > > nodeFunctionscan.c:95
> > > * thread #1: tid = 0x0000, 0x0000000105eb36c2
> > > postgres`pg_stat_get_wal_senders(fcinfo=0x00007fff5a156290) + 498 at
> > > walsender.c:2783, stop reason = signal SIGSTOP
> > > frame #0: 0x0000000105eb36c2
> > > postgres`pg_stat_get_wal_senders(fcinfo=0x00007fff5a156290) + 498 at
> > > walsender.c:2783
> > > 2780 /*
> > > 2781 * Get the currently active synchronous standby.
> > > 2782 */
> > > -> 2783 sync_standbys = (int *) palloc(sizeof(int) *
> > > SyncRepStandbyNames->wait_num);
> > > 2784 LWLockAcquire(SyncRepLock, LW_SHARED);
> > > 2785 num_sync =
> > > SyncRepGetSyncStandbysPriority(SyncRepStandbyNames, sync_standbys);
> > > 2786 LWLockRelease(SyncRepLock);
> > > (lldb) p SyncRepStandbyNames
> > > (SyncGroupNode *) $0 = 0x0000000000000000
> > >
> > > +sync_node_group:
> > > + sync_list { $$ = create_group_node(1, $1);
> > > }
> > > + | sync_element_ast { $$ = create_group_node(1,
> > > $1);}
> > > + | INT '[' sync_list ']' { $$ = create_group_node($1,
> > > $3);}
> > > + | INT '[' sync_element_ast ']' { $$ = create_group_node($1,
> > > $3); }
> > > We may want to be careful with the use of '[' in application_name. I am not
> > > much thrilled with forbidding the use of []() in application_name, so we may
> > > want to recommend user to use a backslash when using s_s_names when a group
> > > is defined.
>
> Mmmm. I found that application_name can contain
> commas. Furthermore, there seems to be no limitation for
> character in the name.
>
> postgres=# set application_name='ho,ge';
> postgres=# select application_name from pg_stat_activity;
> application_name
> ------------------
> ho,ge
>
> check_application_name() allows all characters in the range
> between 32 to 126 in ascii. All other characters are replaced
> with '?'.

Actually I was thinking about that a couple of hours ago. If the
application_name of a node has a comma, it cannot become a sync
replica, no? Wouldn't we need a special handling in s_s_names like
'\,' make a comma part of an application name? Or just ban commas from
the list of supported characters in the application name?
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2016-02-10 09:54:39 Re: Relation extension scalability
Previous Message Oleg Bartunov 2016-02-10 09:28:22 old bug in full text parser