Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1pK5fT-0008E8-At for pgsql-bugs@arkaria.postgresql.org; Mon, 23 Jan 2023 22:51:59 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1pK5fS-00009t-7P for pgsql-bugs@arkaria.postgresql.org; Mon, 23 Jan 2023 22:51:58 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1pK5fR-00009f-Tr for pgsql-bugs@lists.postgresql.org; Mon, 23 Jan 2023 22:51:57 +0000 Received: from sss.pgh.pa.us ([66.207.139.130]) by magus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1pK5fP-0006BE-2L for pgsql-bugs@lists.postgresql.org; Mon, 23 Jan 2023 22:51:57 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.15.2/8.15.2) with ESMTP id 30NMpkqG2139744; Mon, 23 Jan 2023 17:51:46 -0500 From: Tom Lane To: Andres Freund cc: "Sisson, David" , Tomas Vondra , "pgsql-bugs@lists.postgresql.org" Subject: Re: BUG #17757: Not honoring huge_pages setting during initdb causes DB crash in Kubernetes In-reply-to: <20230123195504.uvlcd3aytn3jg744@awork3.anarazel.de> References: <17757-dbdfc1f1c954a6db@postgresql.org> <4ce40288-5b88-f9fb-ea11-fc932c61472b@enterprisedb.com> <1113115.1674343827@sss.pgh.pa.us> <127328f2-4e01-e9ec-c9b1-76aef967343f@enterprisedb.com> <20230122020826.x6geac6qjiinr66a@awork3.anarazel.de> <20230123195504.uvlcd3aytn3jg744@awork3.anarazel.de> Comments: In-reply-to Andres Freund message dated "Mon, 23 Jan 2023 11:55:04 -0800" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <2139729.1674514289.0@sss.pgh.pa.us> Date: Mon, 23 Jan 2023 17:51:46 -0500 Message-ID: <2139743.1674514306@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <2139729.1674514289.1@sss.pgh.pa.us> Content-Transfer-Encoding: quoted-printable Andres Freund writes: > It's a fault of the environment if mmap(MAP_HUGETLB) causes a SIGBUS. No= rmally > huge_pages =3D try is harmless, because it'll just fall back. That sourc= e of > SIGBUSes needs to be fixed regardless of anything else - plenty allocato= rs try > to use huge pages for example, so you'll run into problems regardless of > postgres' default. That seems likely to me too. > That said, I'm for allowing to specify options to initdb. Yeah, I think that has enough other potential applications to be worth doing. Here's a quick draft patch (sans user-facing docs as yet). It injects any given values into postgresql.auto.conf, not postgresql.conf proper. I did that mainly because the latter looked beyond the abilities of the primitive string-munging code we have in there, but I think it can be argued to be a reasonable choice anyway. regards, tom lane ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="override-config-options-during-initdb-0.1.patch"; charset="us-ascii" Content-ID: <2139729.1674514289.2@sss.pgh.pa.us> Content-Description: override-config-options-during-initdb-0.1.patch Content-Transfer-Encoding: quoted-printable diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index 7a58c33ace..e6f1f42cae 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 trou= ble */ 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[] =3D { "trust", "reject", "scram-sha-256", "md5", "password", "ident", "radius"= , #ifdef ENABLE_GSS @@ -142,6 +149,8 @@ static char *pwfilename =3D NULL; static char *superuser_password =3D NULL; static const char *authmethodhost =3D NULL; static const char *authmethodlocal =3D NULL; +static _stringlist *extra_guc_names =3D NULL; +static _stringlist *extra_guc_values =3D NULL; static bool debug =3D false; static bool noclean =3D false; static bool noinstructions =3D 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 =3D pg_malloc(sizeof(_stringlist)); + _stringlist *oldentry; + + newentry->str =3D pg_strdup(str); + newentry->next =3D NULL; + if (*listhead =3D=3D NULL) + *listhead =3D newentry; + else + { + for (oldentry =3D *listhead; oldentry->next; oldentry =3D oldentry->nex= t) + /* skip */ ; + oldentry->next =3D 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 =3D sizeof(trial_conns) / sizeof(int); const int bufslen =3D sizeof(trial_bufs) / sizeof(int); int i, - status, test_conns, test_buffs, ok_buffers =3D 0; @@ -903,19 +932,7 @@ test_config_settings(void) test_conns =3D trial_conns[i]; test_buffs =3D MIN_BUFS_FOR_CONNS(test_conns); = - snprintf(cmd, sizeof(cmd), - "\"%s\" --check %s %s " - "-c max_connections=3D%d " - "-c shared_buffers=3D%d " - "-c dynamic_shared_memory_type=3D%s " - "< \"%s\" > \"%s\" 2>&1", - backend_exec, boot_options, extra_options, - test_conns, test_buffs, - dynamic_shared_memory_type, - DEVNULL, DEVNULL); - fflush(NULL); - status =3D system(cmd); - if (status =3D=3D 0) + if (test_specific_config_settings(test_conns, test_buffs)) { ok_buffers =3D test_buffs; break; @@ -940,19 +957,7 @@ test_config_settings(void) break; } = - snprintf(cmd, sizeof(cmd), - "\"%s\" --check %s %s " - "-c max_connections=3D%d " - "-c shared_buffers=3D%d " - "-c dynamic_shared_memory_type=3D%s " - "< \"%s\" > \"%s\" 2>&1", - backend_exec, boot_options, extra_options, - n_connections, test_buffs, - dynamic_shared_memory_type, - DEVNULL, DEVNULL); - fflush(NULL); - status =3D system(cmd); - if (status =3D=3D 0) + if (test_specific_config_settings(n_connections, test_buffs)) break; } n_buffers =3D 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 =3D createPQExpBuffer(); + int status; + _stringlist *gnames, + *gvalues; + + /* Set up the test postmaster invocation */ + printfPQExpBuffer(cmd, + "\"%s\" --check %s %s " + "-c max_connections=3D%d " + "-c shared_buffers=3D%d " + "-c dynamic_shared_memory_type=3D%s", + backend_exec, boot_options, extra_options, + test_conns, test_buffs, + dynamic_shared_memory_type); + + /* Add any user-given setting overrides */ + for (gnames =3D extra_guc_names, gvalues =3D extra_guc_values; + gnames !=3D NULL; /* assume lists have the same length */ + gnames =3D gnames->next, gvalues =3D gvalues->next) + { + appendPQExpBuffer(cmd, " -c %s=3D", gnames->str); + appendShellString(cmd, gvalues->str); + } + + appendPQExpBuffer(cmd, + " < \"%s\" > \"%s\" 2>&1", + DEVNULL, DEVNULL); + + fflush(NULL); + status =3D system(cmd->data); + + destroyPQExpBuffer(cmd); + + return (status =3D=3D 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) !=3D 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 thi= s - * file will override the value of parameters that exists before parse o= f - * 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 =3D 2; /* fixed header lines */ + for (gnames =3D extra_guc_names; gnames !=3D NULL; gnames =3D gnames->ne= xt) + numautoconflines++; + autoconflines =3D (char **) pg_malloc((numautoconflines + 1) * sizeof(ch= ar *)); + autoconflines[0] =3D pg_strdup("# Do not edit this file manually!\n"); autoconflines[1] =3D pg_strdup("# It will be overwritten by the ALTER SY= STEM command.\n"); - autoconflines[2] =3D NULL; + numautoconflines =3D 2; + for (gnames =3D extra_guc_names, gvalues =3D extra_guc_values; + gnames !=3D NULL; /* assume lists have the same length */ + gnames =3D gnames->next, gvalues =3D gvalues->next) + { + autoconflines[numautoconflines++] =3D + psprintf("%s =3D '%s'\n", + gnames->str, + escape_quotes(gvalues->str)); + } + autoconflines[numautoconflines] =3D NULL; = sprintf(path, "%s/postgresql.auto.conf", pg_data); = @@ -1168,7 +1235,7 @@ setup_config(void) if (chmod(path, pg_file_create_mode) !=3D 0) pg_fatal("could not change permissions of \"%s\": %m", path); = - free(conflines); + free(autoconflines); = = /* pg_hba.conf */ @@ -2103,6 +2170,7 @@ usage(const char *progname) printf(_(" -A, --auth=3DMETHOD default authentication method fo= r local connections\n")); printf(_(" --auth-host=3DMETHOD default authentication method fo= r local TCP/IP connections\n")); printf(_(" --auth-local=3DMETHOD default authentication method fo= r local-socket connections\n")); + printf(_(" -c NAME=3DVALUE override default setting for ser= ver parameter\n")); printf(_(" [-D, --pgdata=3D]DATADIR location for this database clust= er\n")); printf(_(" -E, --encoding=3DENCODING set default encoding for new dat= abases\n")); printf(_(" -g, --allow-group-access allow group read/execute on data d= irectory\n")); @@ -2808,7 +2876,8 @@ main(int argc, char *argv[]) = /* process command-line options */ = - while ((c =3D getopt_long(argc, argv, "A:dD:E:gkL:nNsST:U:WX:", long_opt= ions, &option_index)) !=3D -1) + while ((c =3D getopt_long(argc, argv, "A:c:dD:E:gkL:nNsST:U:WX:", + long_options, &option_index)) !=3D -1) { switch (c) { @@ -2831,6 +2900,24 @@ main(int argc, char *argv[]) case 11: authmethodhost =3D pg_strdup(optarg); break; + case 'c': + { + char *buf =3D pg_strdup(optarg); + char *equals =3D strchr(buf, '=3D'); + + 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++ =3D '\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 =3D pg_strdup(optarg); break; ------- =_aaaaaaaaaa0--