diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index dce1daa73ae9ba4cb07569815018f8c0739de623..b2c5e4d5dd2fed658a3330bd770c1a03b15ae7a0 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -2983,7 +2983,7 @@ include_dir 'conf.d' </term> <listitem> <para> - Specifies a list of standby names that can support + Specifies a list of standby servers that can support <firstterm>synchronous replication</>, as described in <xref linkend="synchronous-replication">. There will be one or more active synchronous standbys; @@ -2991,7 +2991,7 @@ include_dir 'conf.d' these standby servers confirm receipt of their data. The synchronous standbys will be those whose names appear earlier in this list, and - that is both currently connected and streaming data in real-time + that are both currently connected and streaming data in real-time (as shown by a state of <literal>streaming</literal> in the <link linkend="monitoring-stats-views-table"> <literal>pg_stat_replication</></link> view). @@ -3002,7 +3002,7 @@ include_dir 'conf.d' Specifying more than one standby name can allow very high availability. </para> <para> - This parameter specifies a list of standby servers by using + This parameter specifies a list of standby servers using either of the following syntaxes: <synopsis> <replaceable class="parameter">num_sync</replaceable> ( <replaceable class="parameter">standby_name</replaceable> [, ...] ) @@ -3013,17 +3013,17 @@ include_dir 'conf.d' wait for replies from, and <replaceable class="parameter">standby_name</replaceable> is the name of a standby server. For example, a setting of - <literal>'3 (s1, s2, s3, s4)'</> makes transaction commits wait - until their WAL records are received by three higher priority standbys + <literal>3 (s1, s2, s3, s4)</> makes transaction commits wait + until their WAL records are received by three higher-priority standbys chosen from standby servers <literal>s1</>, <literal>s2</>, <literal>s3</> and <literal>s4</>. </para> <para> The second syntax was used before <productname>PostgreSQL</> version 9.6 and is still supported. It's the same as the first syntax - with <replaceable class="parameter">num_sync</replaceable>=1. - For example, both settings of <literal>'1 (s1, s2)'</> and - <literal>'s1, s2'</> have the same meaning; either <literal>s1</> + with <replaceable class="parameter">num_sync</replaceable> equal to 1. + For example, <literal>1 (s1, s2)</> and + <literal>s1, s2</> have the same meaning: either <literal>s1</> or <literal>s2</> is chosen as a synchronous standby. </para> <para> @@ -3039,11 +3039,12 @@ include_dir 'conf.d' </para> <note> <para> - The <replaceable class="parameter">standby_name</replaceable> - must be enclosed in double quotes if a comma (<literal>,</>), - a double quote (<literal>"</>), <!-- " font-lock sanity --> - a left parentheses (<literal>(</>), a right parentheses (<literal>)</>) - or a space is used in the name of a standby server. + Each <replaceable class="parameter">standby_name</replaceable> + should have the form of a valid SQL identifier, unless it + is <literal>*</>. You can use double-quoting if necessary. But note + that <replaceable class="parameter">standby_name</replaceable>s are + compared to standby application names case-insensitively, whether + double-quoted or not. </para> </note> <para> diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c index 3c9142eddca1407dc088ebe91d9357adb3162fcf..acdbf1e230c57a2281b372fc0bc379010477bbc6 100644 --- a/src/backend/replication/syncrep.c +++ b/src/backend/replication/syncrep.c @@ -78,7 +78,7 @@ char *SyncRepStandbyNames; static bool announce_next_takeover = true; -SyncRepConfigData *SyncRepConfig; +static SyncRepConfigData *SyncRepConfig = NULL; static int SyncRepWaitMode = SYNC_REP_NO_WAIT; static void SyncRepQueueInsert(int mode); @@ -361,11 +361,6 @@ SyncRepInitConfig(void) { int priority; - /* Update the config data of synchronous replication */ - SyncRepFreeConfig(SyncRepConfig); - SyncRepConfig = NULL; - SyncRepUpdateConfig(); - /* * Determine if we are a potential sync standby and remember the result * for handling replies from standby. @@ -509,7 +504,9 @@ SyncRepGetOldestSyncRecPtr(XLogRecPtr *writePtr, XLogRecPtr *flushPtr, * Quick exit if we are not managing a sync standby or there are not * enough synchronous standbys. */ - if (!(*am_sync) || list_length(sync_standbys) < SyncRepConfig->num_sync) + if (!(*am_sync) || + SyncRepConfig == NULL || + list_length(sync_standbys) < SyncRepConfig->num_sync) { list_free(sync_standbys); return false; @@ -568,14 +565,15 @@ SyncRepGetSyncStandbys(bool *am_sync) volatile WalSnd *walsnd; /* Use volatile pointer to prevent * code rearrangement */ + /* Set default result */ + if (am_sync != NULL) + *am_sync = false; + /* Quick exit if sync replication is not requested */ if (SyncRepConfig == NULL) return NIL; - if (am_sync != NULL) - *am_sync = false; - - lowest_priority = list_length(SyncRepConfig->members); + lowest_priority = SyncRepConfig->nmembers; next_highest_priority = lowest_priority + 1; /* @@ -730,9 +728,8 @@ SyncRepGetSyncStandbys(bool *am_sync) static int SyncRepGetStandbyPriority(void) { - List *members; - ListCell *l; - int priority = 0; + const char *standby_name; + int priority; bool found = false; /* @@ -742,22 +739,19 @@ SyncRepGetStandbyPriority(void) if (am_cascading_walsender) return 0; - if (!SyncStandbysDefined()) + if (!SyncStandbysDefined() || SyncRepConfig == NULL) return 0; - members = SyncRepConfig->members; - foreach(l, members) + standby_name = SyncRepConfig->member_names; + for (priority = 1; priority <= SyncRepConfig->nmembers; priority++) { - char *standby_name = (char *) lfirst(l); - - priority++; - if (pg_strcasecmp(standby_name, application_name) == 0 || - pg_strcasecmp(standby_name, "*") == 0) + strcmp(standby_name, "*") == 0) { found = true; break; } + standby_name += strlen(standby_name) + 1; } return (found ? priority : 0); @@ -867,50 +861,6 @@ SyncRepUpdateSyncStandbysDefined(void) } } -/* - * Parse synchronous_standby_names and update the config data - * of synchronous standbys. - */ -void -SyncRepUpdateConfig(void) -{ - int parse_rc; - - if (!SyncStandbysDefined()) - return; - - /* - * check_synchronous_standby_names() verifies the setting value of - * synchronous_standby_names before this function is called. So - * syncrep_yyparse() must not cause an error here. - */ - syncrep_scanner_init(SyncRepStandbyNames); - parse_rc = syncrep_yyparse(); - syncrep_scanner_finish(); - - if (parse_rc != 0) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg_internal("synchronous_standby_names parser returned %d", - parse_rc))); - - SyncRepConfig = syncrep_parse_result; - syncrep_parse_result = NULL; -} - -/* - * Free a previously-allocated config data of synchronous replication. - */ -void -SyncRepFreeConfig(SyncRepConfigData *config) -{ - if (!config) - return; - - list_free_deep(config->members); - pfree(config); -} - #ifdef USE_ASSERT_CHECKING static bool SyncRepQueueIsOrderedByLSN(int mode) @@ -955,78 +905,104 @@ SyncRepQueueIsOrderedByLSN(int mode) bool check_synchronous_standby_names(char **newval, void **extra, GucSource source) { - int parse_rc; - if (*newval != NULL && (*newval)[0] != '\0') { + int parse_rc; + SyncRepConfigData *pconf; + + /* Reset communication variables to ensure a fresh start */ + syncrep_parse_result = NULL; + syncrep_parse_error_msg = NULL; + + /* Parse the synchronous_standby_names string */ syncrep_scanner_init(*newval); parse_rc = syncrep_yyparse(); syncrep_scanner_finish(); - if (parse_rc != 0) + if (parse_rc != 0 || syncrep_parse_result == NULL) { GUC_check_errcode(ERRCODE_SYNTAX_ERROR); - GUC_check_errdetail("synchronous_standby_names parser returned %d", - parse_rc); + if (syncrep_parse_error_msg) + GUC_check_errdetail("%s", syncrep_parse_error_msg); + else + GUC_check_errdetail("synchronous_standby_names parser failed"); return false; } /* * Warn if num_sync exceeds the number of names of potential sync - * standbys. This setting doesn't make sense in most cases because - * it implies that enough number of sync standbys will not appear, - * which makes transaction commits wait for sync replication - * infinitely. + * standbys. This setting doesn't make sense in most cases because it + * implies that enough number of sync standbys will not appear, which + * makes transaction commits wait for sync replication infinitely. * * If there are more than one standbys having the same name and * priority, we can see enough sync standbys to complete transaction - * commits. However it's not recommended to run multiple standbys - * with the same priority because we cannot gain full control of - * the selection of sync standbys from them. + * commits. However it's not recommended to run multiple standbys with + * the same priority because we cannot gain full control of the + * selection of sync standbys from them. * * OTOH, that setting is OK if we understand the above problem - * regarding the selection of sync standbys and intentionally - * specify * to match all the standbys. + * regarding the selection of sync standbys and intentionally specify * + * to match all the standbys. */ - if (syncrep_parse_result->num_sync > - list_length(syncrep_parse_result->members)) + if (syncrep_parse_result->num_sync > syncrep_parse_result->nmembers) { - ListCell *l; - bool has_asterisk = false; + const char *standby_name; + int i; + bool has_asterisk = false; - foreach(l, syncrep_parse_result->members) + standby_name = syncrep_parse_result->member_names; + for (i = 1; i <= syncrep_parse_result->nmembers; i++) { - char *standby_name = (char *) lfirst(l); - - if (pg_strcasecmp(standby_name, "*") == 0) + if (strcmp(standby_name, "*") == 0) { has_asterisk = true; break; } + standby_name += strlen(standby_name) + 1; } /* - * Only the postmaster warns this inappropriate setting - * to avoid cluttering the log. + * Only the postmaster warns about this inappropriate setting to + * avoid cluttering the log. */ if (!has_asterisk && !IsUnderPostmaster) ereport(WARNING, - (errmsg("The configured number of synchronous standbys (%d) exceeds the number of names of potential synchronous ones (%d)", - syncrep_parse_result->num_sync, list_length(syncrep_parse_result->members)), + (errmsg("configured number of synchronous standbys (%d) exceeds the number of names of potential synchronous ones (%d)", + syncrep_parse_result->num_sync, + syncrep_parse_result->nmembers), errhint("Specify more names of potential synchronous standbys in synchronous_standby_names."))); } + /* GUC extra value must be malloc'd, not palloc'd */ + pconf = (SyncRepConfigData *) + malloc(syncrep_parse_result->config_size); + if (pconf == NULL) + return false; + memcpy(pconf, syncrep_parse_result, syncrep_parse_result->config_size); + + *extra = (void *) pconf; + /* - * syncrep_yyparse sets the global syncrep_parse_result as side effect. - * But this function is required to just check, so frees it - * after parsing the parameter. + * We need not explicitly clean up syncrep_parse_result. It, and any + * other cruft generated during parsing, will be freed when the + * current memory context is deleted. (This code is generally run in + * a short-lived context used for config file processing, so that will + * not be very long.) */ - SyncRepFreeConfig(syncrep_parse_result); } + else + *extra = NULL; return true; } +void +assign_synchronous_standby_names(const char *newval, void *extra) +{ + SyncRepConfig = (SyncRepConfigData *) extra; +} + void assign_synchronous_commit(int newval, void *extra) { diff --git a/src/backend/replication/syncrep_gram.y b/src/backend/replication/syncrep_gram.y index 380fedc17126d28463e8524e15f50e9e85852b3c..35c27760d1235e339718077419b2369715b0ee39 100644 --- a/src/backend/replication/syncrep_gram.y +++ b/src/backend/replication/syncrep_gram.y @@ -12,16 +12,16 @@ * *------------------------------------------------------------------------- */ - #include "postgres.h" #include "replication/syncrep.h" -#include "utils/formatting.h" -/* Result of the parsing is returned here */ -SyncRepConfigData *syncrep_parse_result; +/* Result of parsing is returned in one of these two variables */ +SyncRepConfigData *syncrep_parse_result; +char *syncrep_parse_error_msg; -static SyncRepConfigData *create_syncrep_config(char *num_sync, List *members); +static SyncRepConfigData *create_syncrep_config(const char *num_sync, + List *members); /* * Bison doesn't allocate anything that needs to live across parser calls, @@ -43,10 +43,10 @@ static SyncRepConfigData *create_syncrep_config(char *num_sync, List *members); { char *str; List *list; - SyncRepConfigData *config; + SyncRepConfigData *config; } -%token <str> NAME NUM +%token <str> NAME NUM JUNK %type <config> result standby_config %type <list> standby_list @@ -57,29 +57,57 @@ static SyncRepConfigData *create_syncrep_config(char *num_sync, List *members); %% result: standby_config { syncrep_parse_result = $1; } -; + ; + standby_config: standby_list { $$ = create_syncrep_config("1", $1); } - | NUM '(' standby_list ')' { $$ = create_syncrep_config($1, $3); } -; + | NUM '(' standby_list ')' { $$ = create_syncrep_config($1, $3); } + ; + standby_list: - standby_name { $$ = list_make1($1);} - | standby_list ',' standby_name { $$ = lappend($1, $3);} -; + standby_name { $$ = list_make1($1); } + | standby_list ',' standby_name { $$ = lappend($1, $3); } + ; + standby_name: - NAME { $$ = $1; } - | NUM { $$ = $1; } -; + NAME { $$ = $1; } + | NUM { $$ = $1; } + ; %% + static SyncRepConfigData * -create_syncrep_config(char *num_sync, List *members) +create_syncrep_config(const char *num_sync, List *members) { - SyncRepConfigData *config = - (SyncRepConfigData *) palloc(sizeof(SyncRepConfigData)); + SyncRepConfigData *config; + int size; + ListCell *lc; + char *ptr; + /* Compute space needed for flat representation */ + size = offsetof(SyncRepConfigData, member_names); + foreach(lc, members) + { + char *standby_name = (char *) lfirst(lc); + + size += strlen(standby_name) + 1; + } + + /* And transform the data into flat representation */ + config = (SyncRepConfigData *) palloc(size); + + config->config_size = size; config->num_sync = atoi(num_sync); - config->members = members; + config->nmembers = list_length(members); + ptr = config->member_names; + foreach(lc, members) + { + char *standby_name = (char *) lfirst(lc); + + strcpy(ptr, standby_name); + ptr += strlen(standby_name) + 1; + } + return config; } diff --git a/src/backend/replication/syncrep_scanner.l b/src/backend/replication/syncrep_scanner.l index 968265e3bbb2c46075c5b441e1501bffe9844ca8..d20662ed03874a5b2f6965b32205ce297791cd6e 100644 --- a/src/backend/replication/syncrep_scanner.l +++ b/src/backend/replication/syncrep_scanner.l @@ -15,31 +15,16 @@ */ #include "postgres.h" -#include "miscadmin.h" #include "lib/stringinfo.h" -/* - * flex emits a yy_fatal_error() function that it calls in response to - * critical errors like malloc failure, file I/O errors, and detection of - * internal inconsistency. That function prints a message and calls exit(). - * Mutate it to instead call ereport(FATAL), which terminates this process. - * - * The process that causes this fatal error should be terminated. - * Otherwise it has to abandon the new setting value of - * synchronous_standby_names and keep running with the previous one - * while the other processes switch to the new one. - * This inconsistency of the setting that each process is based on - * can cause a serious problem. Though it's basically not good idea to - * use FATAL here because it can take down the postmaster, - * we should do that in order to avoid such an inconsistency. - */ +/* Avoid exit() on fatal scanner errors (a bit ugly -- see yy_fatal_error) */ #undef fprintf -#define fprintf(file, fmt, msg) syncrep_flex_fatal(fmt, msg) +#define fprintf(file, fmt, msg) fprintf_to_ereport(fmt, msg) static void -syncrep_flex_fatal(const char *fmt, const char *msg) +fprintf_to_ereport(const char *fmt, const char *msg) { - ereport(FATAL, (errmsg_internal("%s", msg))); + ereport(ERROR, (errmsg_internal("%s", msg))); } /* Handles to the buffer that the lexer uses internally */ @@ -51,8 +36,9 @@ static StringInfoData xdbuf; %option 8bit %option never-interactive -%option nounput +%option nodefault %option noinput +%option nounput %option noyywrap %option warn %option prefix="syncrep_yy" @@ -62,57 +48,79 @@ static StringInfoData xdbuf; */ %x xd -space [ \t\n\r\f\v] +space [ \t\n\r\f\v] -undquoted_start [^ ,\(\)\"] -undquoted_cont [^ ,\(\)] -undquoted_name {undquoted_start}{undquoted_cont}* -dquoted_name [^\"]+ +digit [0-9] +ident_start [A-Za-z\200-\377_] +ident_cont [A-Za-z\200-\377_0-9\$] +identifier {ident_start}{ident_cont}* -/* Double-quoted string */ -dquote \" -xdstart {dquote} +dquote \" +xdstart {dquote} +xdstop {dquote} xddouble {dquote}{dquote} -xdstop {dquote} -xdinside {dquoted_name} +xdinside [^"]+ %% -{space}+ { /* ignore */ } +{space}+ { /* ignore */ } + {xdstart} { initStringInfo(&xdbuf); BEGIN(xd); } <xd>{xddouble} { - appendStringInfoChar(&xdbuf, '\"'); + appendStringInfoChar(&xdbuf, '"'); } <xd>{xdinside} { appendStringInfoString(&xdbuf, yytext); } <xd>{xdstop} { - yylval.str = pstrdup(xdbuf.data); - pfree(xdbuf.data); + yylval.str = xdbuf.data; + xdbuf.data = NULL; BEGIN(INITIAL); return NAME; } -"," { return ','; } -"(" { return '('; } -")" { return ')'; } -[1-9][0-9]* { +<xd><<EOF>> { + yyerror("unterminated quoted identifier"); + return JUNK; + } + +{identifier} { yylval.str = pstrdup(yytext); - return NUM; + return NAME; } -{undquoted_name} { + +{digit}+ { yylval.str = pstrdup(yytext); + return NUM; + } + +"*" { + yylval.str = "*"; return NAME; } + +"," { return ','; } +"(" { return '('; } +")" { return ')'; } + +. { return JUNK; } %% + +/* Needs to be here for access to yytext */ void -yyerror(const char *message) +syncrep_yyerror(const char *message) { - ereport(IsUnderPostmaster ? DEBUG2 : LOG, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("%s at or near \"%s\"", message, yytext))); + /* report only the first error in a parse operation */ + if (syncrep_parse_error_msg) + return; + if (yytext[0]) + syncrep_parse_error_msg = psprintf("%s at or near \"%s\"", + message, yytext); + else + syncrep_parse_error_msg = psprintf("%s at end of input", + message); } void @@ -134,6 +142,9 @@ syncrep_scanner_init(const char *str) memcpy(scanbuf, str, slen); scanbuf[slen] = scanbuf[slen + 1] = YY_END_OF_BUFFER_CHAR; scanbufhandle = yy_scan_buffer(scanbuf, slen + 2); + + /* Make sure we start in proper state */ + BEGIN(INITIAL); } void diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c index 81d3d285c900e643f256d027e0a6ab47204702f3..20d23d5a26a16fe1506c5c9826773fabc70324d4 100644 --- a/src/backend/replication/walsender.c +++ b/src/backend/replication/walsender.c @@ -2780,23 +2780,12 @@ pg_stat_get_wal_senders(PG_FUNCTION_ARGS) MemoryContextSwitchTo(oldcontext); /* - * Allocate and update the config data of synchronous replication, - * and then get the currently active synchronous standbys. + * Get the currently active synchronous standbys. */ - SyncRepUpdateConfig(); LWLockAcquire(SyncRepLock, LW_SHARED); sync_standbys = SyncRepGetSyncStandbys(NULL); LWLockRelease(SyncRepLock); - /* - * Free the previously-allocated config data because a backend - * no longer needs it. The next call of this function needs to - * allocate and update the config data newly because the setting - * of sync replication might be changed between the calls. - */ - SyncRepFreeConfig(SyncRepConfig); - SyncRepConfig = NULL; - for (i = 0; i < max_wal_senders; i++) { WalSnd *walsnd = &WalSndCtl->walsnds[i]; diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 26ca06cfc2578e45b0a748cb4130d7afca0e4aaf..752f823a164f2cc968c17b1ff15ce301e6a2c920 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -3484,7 +3484,7 @@ static struct config_string ConfigureNamesString[] = }, &SyncRepStandbyNames, "", - check_synchronous_standby_names, NULL, NULL + check_synchronous_standby_names, assign_synchronous_standby_names, NULL }, { diff --git a/src/include/replication/syncrep.h b/src/include/replication/syncrep.h index 14b56649da179bb4242d106eae316f980fbc2489..e4e0e2737143e8f475bfe47432f9d0b338c47715 100644 --- a/src/include/replication/syncrep.h +++ b/src/include/replication/syncrep.h @@ -20,7 +20,7 @@ (max_wal_senders > 0 && synchronous_commit > SYNCHRONOUS_COMMIT_LOCAL_FLUSH) /* SyncRepWaitMode */ -#define SYNC_REP_NO_WAIT -1 +#define SYNC_REP_NO_WAIT (-1) #define SYNC_REP_WAIT_WRITE 0 #define SYNC_REP_WAIT_FLUSH 1 #define SYNC_REP_WAIT_APPLY 2 @@ -34,15 +34,24 @@ /* * Struct for the configuration of synchronous replication. + * + * Note: this must be a flat representation that can be held in a single + * chunk of malloc'd memory, so that it can be stored as the "extra" data + * for the synchronous_standby_names GUC. */ typedef struct SyncRepConfigData { - int num_sync; /* number of sync standbys that we need to wait for */ - List *members; /* list of names of potential sync standbys */ + int config_size; /* total size of this struct, in bytes */ + int num_sync; /* number of sync standbys that we need to + * wait for */ + int nmembers; /* number of members in the following list */ + /* member_names contains nmembers consecutive nul-terminated C strings */ + char member_names[FLEXIBLE_ARRAY_MEMBER]; } SyncRepConfigData; +/* communication variables for parsing synchronous_standby_names GUC */ extern SyncRepConfigData *syncrep_parse_result; -extern SyncRepConfigData *SyncRepConfig; +extern char *syncrep_parse_error_msg; /* user-settable parameters for synchronous replication */ extern char *SyncRepStandbyNames; @@ -59,21 +68,21 @@ extern void SyncRepReleaseWaiters(void); /* called by wal sender and user backend */ extern List *SyncRepGetSyncStandbys(bool *am_sync); -extern void SyncRepUpdateConfig(void); -extern void SyncRepFreeConfig(SyncRepConfigData *config); /* called by checkpointer */ extern void SyncRepUpdateSyncStandbysDefined(void); +/* GUC infrastructure */ extern bool check_synchronous_standby_names(char **newval, void **extra, GucSource source); +extern void assign_synchronous_standby_names(const char *newval, void *extra); extern void assign_synchronous_commit(int newval, void *extra); /* * Internal functions for parsing synchronous_standby_names grammar, * in syncrep_gram.y and syncrep_scanner.l */ -extern int syncrep_yyparse(void); -extern int syncrep_yylex(void); +extern int syncrep_yyparse(void); +extern int syncrep_yylex(void); extern void syncrep_yyerror(const char *str); extern void syncrep_scanner_init(const char *query_string); extern void syncrep_scanner_finish(void);