On Wed, Feb 10, 2016 at 3:13 PM, Michael Paquier <michael.paqu...@gmail.com> wrote: > On Wed, Feb 10, 2016 at 11:25 AM, Masahiko Sawada <sawada.m...@gmail.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. > > +void > +yyerror(const char *message) > +{ > + ereport(ERROR, > + (errcode(ERRCODE_SYNTAX_ERROR), > + errmsg_internal("%s", message))); > +} > whitespace errors here.
+#define MAX_WALSENDER_NAME 8192 + typedef enum WalSndState { WALSNDSTATE_STARTUP = 0, @@ -62,6 +64,11 @@ typedef struct WalSnd * SyncRepLock. */ int sync_standby_priority; + + /* + * Corresponding standby's application_name. + */ + const char name[MAX_WALSENDER_NAME]; } WalSnd; NAMEDATALEN instead? -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers