public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v2 2/2] Fix a bug of timeline-tracking while sending a historic timeline 4+ messages / 4 participants [nested] [flat]
* [PATCH v2 2/2] Fix a bug of timeline-tracking while sending a historic timeline @ 2020-12-09 08:22 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Kyotaro Horiguchi @ 2020-12-09 08:22 UTC (permalink / raw) Since PG13, we should track a timeline switch while sending a historic timeline running physical replication, but WalSndSegmentOpen fails to do that. It is a thinko of 709d003fbd. --- src/backend/replication/walsender.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c index fe0d368a35..8545c6c423 100644 --- a/src/backend/replication/walsender.c +++ b/src/backend/replication/walsender.c @@ -2491,7 +2491,7 @@ WalSndSegmentOpen(XLogReaderState *state, XLogSegNo nextSegNo, XLogSegNo endSegNo; XLByteToSeg(sendTimeLineValidUpto, endSegNo, state->segcxt.ws_segsize); - if (state->seg.ws_segno == endSegNo) + if (nextSegNo == endSegNo) *tli_p = sendTimeLineNextTLI; } -- 2.27.0 ----Next_Part(Mon_Jan__4_12_06_10_2021_267)---- ^ permalink raw reply [nested|flat] 4+ messages in thread
* Set arbitrary GUC options during initdb @ 2023-01-25 21:25 Tom Lane <[email protected]> 2023-01-25 22:03 ` Re: Set arbitrary GUC options during initdb Andres Freund <[email protected]> 2023-01-27 15:29 ` Re: Set arbitrary GUC options during initdb Robert Haas <[email protected]> 0 siblings, 2 replies; 4+ messages in thread From: Tom Lane @ 2023-01-25 21:25 UTC (permalink / raw) To: [email protected] The attached patch responds to the discussion at [1] about how we ought to offer a way to set any server GUC from the initdb command line. Currently, if for some reason the server won't start with default parameters, the only way to get through initdb is to change the installed version of postgresql.conf.sample. And even that is just a kluge, because the initial probes to choose max_connections and shared_buffers will all fail, causing initdb to choose rock-bottom-minimum values of those settings. You can fix that up after the fact if you notice it, but you might not. So this invents an initdb switch "-c NAME=VALUE" just like the one that the server itself has long had. The specified settings are applied on the command line of the initial probe calls (which happen before we've made any config files), and then they are added to postgresql.auto.conf, which causes them to take effect for the bootstrap backend runs as well as subsequent postmaster starts. I also invented "--set NAME=VALUE", mainly because just about every other initdb switch has a long form. The server itself doesn't have that spelling, so I'm not wedded to that part. Thoughts? regards, tom lane [1] https://www.postgresql.org/message-id/flat/17757-dbdfc1f1c954a6db%40postgresql.org Attachments: [text/x-diff] set-arbitrary-options-during-initdb-1.patch (10.7K, ../../[email protected]/2-set-arbitrary-options-during-initdb-1.patch) download | inline diff: diff --git a/doc/src/sgml/ref/initdb.sgml b/doc/src/sgml/ref/initdb.sgml index 5b2bdac101..904cee7858 100644 --- a/doc/src/sgml/ref/initdb.sgml +++ b/doc/src/sgml/ref/initdb.sgml @@ -433,6 +433,23 @@ PostgreSQL documentation Other, less commonly used, options are also available: <variablelist> + <varlistentry id="app-initdb-option-set"> + <term><option>-c <replaceable>name</replaceable>=<replaceable>value</replaceable></option></term> + <term><option>--set <replaceable>name</replaceable>=<replaceable>value</replaceable></option></term> + <listitem> + <para> + Forcibly set the server parameter <replaceable>name</replaceable> + to <replaceable>value</replaceable> during <command>initdb</command>, + and also set up that assignment in the + generated <filename>postgresql.auto.conf</filename> file, as though + it had been issued via <command>ALTER SYSTEM SET</command>. + This option can be used more than once to set several parameters. + It is primarily useful when the environment is such that the server + will not start at all using the default parameters. + </para> + </listitem> + </varlistentry> + <varlistentry id="app-initdb-option-debug"> <term><option>-d</option></term> <term><option>--debug</option></term> diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index 7a58c33ace..8daad6a1f6 100644 --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c @@ -82,6 +82,13 @@ /* Ideally this would be in a .h file, but it hardly seems worth the trouble */ extern const char *select_default_timezone(const char *share_path); +/* simple list of strings */ +typedef struct _stringlist +{ + char *str; + struct _stringlist *next; +} _stringlist; + static const char *const auth_methods_host[] = { "trust", "reject", "scram-sha-256", "md5", "password", "ident", "radius", #ifdef ENABLE_GSS @@ -142,6 +149,8 @@ static char *pwfilename = NULL; static char *superuser_password = NULL; static const char *authmethodhost = NULL; static const char *authmethodlocal = NULL; +static _stringlist *extra_guc_names = NULL; +static _stringlist *extra_guc_values = NULL; static bool debug = false; static bool noclean = false; static bool noinstructions = false; @@ -253,6 +262,7 @@ static void check_input(char *path); static void write_version_file(const char *extrapath); static void set_null_conf(void); static void test_config_settings(void); +static bool test_specific_config_settings(int test_conns, int test_buffs); static void setup_config(void); static void bootstrap_template1(void); static void setup_auth(FILE *cmdfd); @@ -359,6 +369,27 @@ escape_quotes_bki(const char *src) return result; } +/* + * Add an item at the end of a stringlist. + */ +static void +add_stringlist_item(_stringlist **listhead, const char *str) +{ + _stringlist *newentry = pg_malloc(sizeof(_stringlist)); + _stringlist *oldentry; + + newentry->str = pg_strdup(str); + newentry->next = NULL; + if (*listhead == NULL) + *listhead = newentry; + else + { + for (oldentry = *listhead; oldentry->next; oldentry = oldentry->next) + /* skip */ ; + oldentry->next = newentry; + } +} + /* * make a copy of the array of lines, with token replaced by replacement * the first time it occurs on each line. @@ -873,11 +904,9 @@ test_config_settings(void) 400, 300, 200, 100, 50 }; - char cmd[MAXPGPATH]; const int connslen = sizeof(trial_conns) / sizeof(int); const int bufslen = sizeof(trial_bufs) / sizeof(int); int i, - status, test_conns, test_buffs, ok_buffers = 0; @@ -903,19 +932,7 @@ test_config_settings(void) test_conns = trial_conns[i]; test_buffs = MIN_BUFS_FOR_CONNS(test_conns); - snprintf(cmd, sizeof(cmd), - "\"%s\" --check %s %s " - "-c max_connections=%d " - "-c shared_buffers=%d " - "-c dynamic_shared_memory_type=%s " - "< \"%s\" > \"%s\" 2>&1", - backend_exec, boot_options, extra_options, - test_conns, test_buffs, - dynamic_shared_memory_type, - DEVNULL, DEVNULL); - fflush(NULL); - status = system(cmd); - if (status == 0) + if (test_specific_config_settings(test_conns, test_buffs)) { ok_buffers = test_buffs; break; @@ -940,19 +957,7 @@ test_config_settings(void) break; } - snprintf(cmd, sizeof(cmd), - "\"%s\" --check %s %s " - "-c max_connections=%d " - "-c shared_buffers=%d " - "-c dynamic_shared_memory_type=%s " - "< \"%s\" > \"%s\" 2>&1", - backend_exec, boot_options, extra_options, - n_connections, test_buffs, - dynamic_shared_memory_type, - DEVNULL, DEVNULL); - fflush(NULL); - status = system(cmd); - if (status == 0) + if (test_specific_config_settings(n_connections, test_buffs)) break; } n_buffers = test_buffs; @@ -968,6 +973,48 @@ test_config_settings(void) printf("%s\n", default_timezone ? default_timezone : "GMT"); } +/* + * Test a specific combination of configuration settings. + */ +static bool +test_specific_config_settings(int test_conns, int test_buffs) +{ + PQExpBuffer cmd = createPQExpBuffer(); + _stringlist *gnames, + *gvalues; + int status; + + /* Set up the test postmaster invocation */ + printfPQExpBuffer(cmd, + "\"%s\" --check %s %s " + "-c max_connections=%d " + "-c shared_buffers=%d " + "-c dynamic_shared_memory_type=%s", + backend_exec, boot_options, extra_options, + test_conns, test_buffs, + dynamic_shared_memory_type); + + /* Add any user-given setting overrides */ + for (gnames = extra_guc_names, gvalues = extra_guc_values; + gnames != NULL; /* assume lists have the same length */ + gnames = gnames->next, gvalues = gvalues->next) + { + appendPQExpBuffer(cmd, " -c %s=", gnames->str); + appendShellString(cmd, gvalues->str); + } + + appendPQExpBuffer(cmd, + " < \"%s\" > \"%s\" 2>&1", + DEVNULL, DEVNULL); + + fflush(NULL); + status = system(cmd->data); + + destroyPQExpBuffer(cmd); + + return (status == 0); +} + /* * Calculate the default wal_size with a "pretty" unit. */ @@ -994,7 +1041,10 @@ setup_config(void) char **conflines; char repltok[MAXPGPATH]; char path[MAXPGPATH]; - char *autoconflines[3]; + char **autoconflines; + int numautoconflines; + _stringlist *gnames, + *gvalues; fputs(_("creating configuration files ... "), stdout); fflush(stdout); @@ -1152,15 +1202,32 @@ setup_config(void) if (chmod(path, pg_file_create_mode) != 0) pg_fatal("could not change permissions of \"%s\": %m", path); + free(conflines); + + /* - * create the automatic configuration file to store the configuration - * parameters set by ALTER SYSTEM command. The parameters present in this - * file will override the value of parameters that exists before parse of - * this file. + * Create the automatic configuration file that stores the configuration + * parameters set by the ALTER SYSTEM command. We also place any + * parameter values set with initdb's -c option into this file. */ + numautoconflines = 2; /* fixed header lines */ + for (gnames = extra_guc_names; gnames != NULL; gnames = gnames->next) + numautoconflines++; + autoconflines = (char **) pg_malloc((numautoconflines + 1) * sizeof(char *)); + autoconflines[0] = pg_strdup("# Do not edit this file manually!\n"); autoconflines[1] = pg_strdup("# It will be overwritten by the ALTER SYSTEM command.\n"); - autoconflines[2] = NULL; + numautoconflines = 2; + for (gnames = extra_guc_names, gvalues = extra_guc_values; + gnames != NULL; /* assume lists have the same length */ + gnames = gnames->next, gvalues = gvalues->next) + { + autoconflines[numautoconflines++] = + psprintf("%s = '%s'\n", + gnames->str, + escape_quotes(gvalues->str)); + } + autoconflines[numautoconflines] = NULL; sprintf(path, "%s/postgresql.auto.conf", pg_data); @@ -1168,7 +1235,7 @@ setup_config(void) if (chmod(path, pg_file_create_mode) != 0) pg_fatal("could not change permissions of \"%s\": %m", path); - free(conflines); + free(autoconflines); /* pg_hba.conf */ @@ -2124,6 +2191,7 @@ usage(const char *progname) printf(_(" -X, --waldir=WALDIR location for the write-ahead log directory\n")); printf(_(" --wal-segsize=SIZE size of WAL segments, in megabytes\n")); printf(_("\nLess commonly used options:\n")); + printf(_(" -c, --set NAME=VALUE override default setting for server parameter\n")); printf(_(" -d, --debug generate lots of debugging output\n")); printf(_(" --discard-caches set debug_discard_caches=1\n")); printf(_(" -L DIRECTORY where to find the input files\n")); @@ -2759,6 +2827,7 @@ main(int argc, char *argv[]) {"nosync", no_argument, NULL, 'N'}, /* for backwards compatibility */ {"no-sync", no_argument, NULL, 'N'}, {"no-instructions", no_argument, NULL, 13}, + {"set", required_argument, NULL, 'c'}, {"sync-only", no_argument, NULL, 'S'}, {"waldir", required_argument, NULL, 'X'}, {"wal-segsize", required_argument, NULL, 12}, @@ -2808,7 +2877,8 @@ main(int argc, char *argv[]) /* process command-line options */ - while ((c = getopt_long(argc, argv, "A:dD:E:gkL:nNsST:U:WX:", long_options, &option_index)) != -1) + while ((c = getopt_long(argc, argv, "A:c:dD:E:gkL:nNsST:U:WX:", + long_options, &option_index)) != -1) { switch (c) { @@ -2831,6 +2901,24 @@ main(int argc, char *argv[]) case 11: authmethodhost = pg_strdup(optarg); break; + case 'c': + { + char *buf = pg_strdup(optarg); + char *equals = strchr(buf, '='); + + if (!equals) + { + pg_log_error("-c %s requires a value", buf); + pg_log_error_hint("Try \"%s --help\" for more information.", + progname); + exit(1); + } + *equals++ = '\0'; /* terminate variable name */ + add_stringlist_item(&extra_guc_names, buf); + add_stringlist_item(&extra_guc_values, equals); + pfree(buf); + } + break; case 'D': pg_data = pg_strdup(optarg); break; diff --git a/src/bin/initdb/t/001_initdb.pl b/src/bin/initdb/t/001_initdb.pl index 772769acab..1e23ac8d0c 100644 --- a/src/bin/initdb/t/001_initdb.pl +++ b/src/bin/initdb/t/001_initdb.pl @@ -48,7 +48,13 @@ mkdir $datadir; local (%ENV) = %ENV; delete $ENV{TZ}; - command_ok([ 'initdb', '-N', '-T', 'german', '-X', $xlogdir, $datadir ], + # while we are here, also exercise -T and -c options + command_ok( + [ + 'initdb', '-N', '-T', 'german', '-c', + 'default_text_search_config=german', + '-X', $xlogdir, $datadir + ], 'successful creation'); # Permissions on PGDATA should be default @@ -147,4 +153,7 @@ command_fails( ], 'fails for invalid option combination'); +command_fails([ 'initdb', '--no-sync', '--set', 'foo=bar', "$tempdir/dataX" ], + 'fails for invalid --set option'); + done_testing(); ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Set arbitrary GUC options during initdb 2023-01-25 21:25 Set arbitrary GUC options during initdb Tom Lane <[email protected]> @ 2023-01-25 22:03 ` Andres Freund <[email protected]> 1 sibling, 0 replies; 4+ messages in thread From: Andres Freund @ 2023-01-25 22:03 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: [email protected] Hi, On 2023-01-25 16:25:19 -0500, Tom Lane wrote: > The attached patch responds to the discussion at [1] about how > we ought to offer a way to set any server GUC from the initdb > command line. Are you thinking of backpatching this, to offer the people affected by the issue in [1] a way out? > So this invents an initdb switch "-c NAME=VALUE" just like the > one that the server itself has long had. I still am +1 on the idea. I've actually wanted this for development purposes a couple times... > The specified settings are applied on the command line of the initial probe > calls (which happen before we've made any config files), and then they are > added to postgresql.auto.conf, which causes them to take effect for the > bootstrap backend runs as well as subsequent postmaster starts. I think this means that if you set e.g. max_connections as an initdb parameter, the probes won't do much. Probably fine? Perhaps worth memorializing the priority of the -c options in a test? E.g. setting shared_buffers = 20MB or so and then testing that that's the value when starting the server? > I also invented "--set NAME=VALUE", mainly because just about > every other initdb switch has a long form. The server itself > doesn't have that spelling, so I'm not wedded to that part. Fine with me, but also fine to leave out. Greetings, Andres Freund ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Set arbitrary GUC options during initdb 2023-01-25 21:25 Set arbitrary GUC options during initdb Tom Lane <[email protected]> @ 2023-01-27 15:29 ` Robert Haas <[email protected]> 1 sibling, 0 replies; 4+ messages in thread From: Robert Haas @ 2023-01-27 15:29 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: [email protected] On Wed, Jan 25, 2023 at 4:26 PM Tom Lane <[email protected]> wrote: > So this invents an initdb switch "-c NAME=VALUE" just like the > one that the server itself has long had. HUGE +1 from me. This will, I think, be extremely convenient in many situations. > The specified settings > are applied on the command line of the initial probe calls > (which happen before we've made any config files), and then they > are added to postgresql.auto.conf, which causes them to take > effect for the bootstrap backend runs as well as subsequent > postmaster starts. I agree with others that it would seem more natural to edit them in postgresql.conf itself, but I also think it doesn't matter nearly as much as getting the feature in some form. -- Robert Haas EDB: http://www.enterprisedb.com ^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2023-01-27 15:29 UTC | newest] Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-12-09 08:22 [PATCH v2 2/2] Fix a bug of timeline-tracking while sending a historic timeline Kyotaro Horiguchi <[email protected]> 2023-01-25 21:25 Set arbitrary GUC options during initdb Tom Lane <[email protected]> 2023-01-25 22:03 ` Re: Set arbitrary GUC options during initdb Andres Freund <[email protected]> 2023-01-27 15:29 ` Re: Set arbitrary GUC options during initdb Robert Haas <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox