public inbox for [email protected]help / color / mirror / Atom feed
initdb / bootstrap design 6+ messages / 5 participants [nested] [flat]
* initdb / bootstrap design @ 2022-02-16 02:12 Andres Freund <[email protected]> 2022-02-16 03:14 ` Re: initdb / bootstrap design John Naylor <[email protected]> 2022-02-16 10:47 ` Re: initdb / bootstrap design Peter Eisentraut <[email protected]> 2022-02-16 18:24 ` Re: initdb / bootstrap design Tom Lane <[email protected]> 0 siblings, 3 replies; 6+ messages in thread From: Andres Freund @ 2022-02-16 02:12 UTC (permalink / raw) To: pgsql-hackers Hi, [1] reminded me of a topic that I wanted to bring up at some point: To me the division of labor between initdb and bootstrap doesn't make much sense anymore: initdb reads postgres.bki, replaces a few tokens, starts postgres in bootstrap mode, and then painstakenly feeds bootstrap.bki lines to the server. Given that bootstrap mode parsing is a dedicated parser, only invoked from a single point, what's the point of initdb doing the preprocessing and then incurring pipe overhead? Sure, there's a few tokens that we replace in initdb. As it turns out there's only two rows that are actually variable. The username of the initial superuser in pg_authid and the pg_database row for template 1, where encoding, lc_collate and lc_ctype varies. The rest is all compile time constant replacements we could do as part of genbki.pl. It seems we could save a good number of context switches by opening postgres.bki just before boot_yyparse() in BootstrapModeMain() and having the parser read it. The pg_authid / pg_database rows we could just do via explicit insertions in BootstrapModeMain(), provided by commandline args? Similarly, since the introduction of extensions at the latest, the server knows how to execute SQL from a file. Why don't we just process information_schema.sql, system_views.sql et al that way? If we don't need a dedicated "input" mode feeding boot_yyparse() in bootstrap mode anymore (because bootstrap mode feeds it from postgres.bki directly), we likely could avoid the restart between bootstrap and single user mode. Afaics that only really is needed because we need to send SQL after bootstrap_template1(). That'd likely be a nice speedup, because we don't need to write the bootstrap contents from shared buffers to the OS just to read them back in single user mode. I don't plan to work on this immediately, but I thought it's worth bringing up anyway. Greetings, Andres Freund [1] https://www.postgresql.org/message-id/20220216012953.6d7bzmsblqou3ru4%40alap3.anarazel.de ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: initdb / bootstrap design 2022-02-16 02:12 initdb / bootstrap design Andres Freund <[email protected]> @ 2022-02-16 03:14 ` John Naylor <[email protected]> 2 siblings, 0 replies; 6+ messages in thread From: John Naylor @ 2022-02-16 03:14 UTC (permalink / raw) To: Andres Freund <[email protected]>; +Cc: pgsql-hackers On Wed, Feb 16, 2022 at 9:12 AM Andres Freund <[email protected]> wrote: > To me the division of labor between initdb and bootstrap doesn't make much > sense anymore: [...] > I don't plan to work on this immediately, but I thought it's worth bringing up > anyway. Added TODO item "Rationalize division of labor between initdb and bootstrap" -- John Naylor EDB: http://www.enterprisedb.com ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: initdb / bootstrap design 2022-02-16 02:12 initdb / bootstrap design Andres Freund <[email protected]> @ 2022-02-16 10:47 ` Peter Eisentraut <[email protected]> 2022-02-16 17:53 ` Re: initdb / bootstrap design Andres Freund <[email protected]> 2 siblings, 1 reply; 6+ messages in thread From: Peter Eisentraut @ 2022-02-16 10:47 UTC (permalink / raw) To: Andres Freund <[email protected]>; pgsql-hackers On 16.02.22 03:12, Andres Freund wrote: > Sure, there's a few tokens that we replace in initdb. As it turns out there's > only two rows that are actually variable. The username of the initial > superuser in pg_authid and the pg_database row for template 1, where encoding, > lc_collate and lc_ctype varies. The rest is all compile time constant > replacements we could do as part of genbki.pl. > > It seems we could save a good number of context switches by opening > postgres.bki just before boot_yyparse() in BootstrapModeMain() and having the > parser read it. The pg_authid / pg_database rows we could just do via > explicit insertions in BootstrapModeMain(), provided by commandline args? I think we could do the locale setup by updating the pg_database row of template1 after bootstrap, as in the attached patch. (The order of proceedings in the surrounding function might need some refinement in a final patch.) I suspect we could do the treatment of pg_authid similarly. From 10143067fb35191aaa53ce2e5c4a20c4601b7528 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <[email protected]> Date: Wed, 16 Feb 2022 11:43:10 +0100 Subject: [PATCH] Simplify locale setup of template1 in initdb --- src/bin/initdb/initdb.c | 23 +++++------------------ src/include/catalog/pg_database.dat | 6 +++--- 2 files changed, 8 insertions(+), 21 deletions(-) diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index 97f15971e2..42e42ca4a4 100644 --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c @@ -621,15 +621,6 @@ get_id(void) return pg_strdup(username); } -static char * -encodingid_to_string(int enc) -{ - char result[20]; - - sprintf(result, "%d", enc); - return pg_strdup(result); -} - /* * get the encoding id for a given encoding name */ @@ -1396,15 +1387,6 @@ bootstrap_template1(void) bki_lines = replace_token(bki_lines, "POSTGRES", escape_quotes_bki(username)); - bki_lines = replace_token(bki_lines, "ENCODING", - encodingid_to_string(encodingid)); - - bki_lines = replace_token(bki_lines, "LC_COLLATE", - escape_quotes_bki(lc_collate)); - - bki_lines = replace_token(bki_lines, "LC_CTYPE", - escape_quotes_bki(lc_ctype)); - /* Also ensure backend isn't confused by this environment var: */ unsetenv("PGCLIENTENCODING"); @@ -1886,6 +1868,11 @@ make_template0(FILE *cmdfd) NULL }; + PG_CMD_PRINTF("UPDATE pg_database " + " SET encoding = %d, datcollate = '%s', datctype = '%s' " + " WHERE datname = 'template1';\n\n", + encodingid, escape_quotes(lc_collate), escape_quotes(lc_ctype)); + for (line = template0_setup; *line; line++) PG_CMD_PUTS(*line); } diff --git a/src/include/catalog/pg_database.dat b/src/include/catalog/pg_database.dat index e7e42d6023..6bca1ec54b 100644 --- a/src/include/catalog/pg_database.dat +++ b/src/include/catalog/pg_database.dat @@ -14,9 +14,9 @@ { oid => '1', oid_symbol => 'TemplateDbOid', descr => 'default template for new databases', - datname => 'template1', encoding => 'ENCODING', datistemplate => 't', + datname => 'template1', encoding => '0', datistemplate => 't', datallowconn => 't', datconnlimit => '-1', datfrozenxid => '0', - datminmxid => '1', dattablespace => 'pg_default', datcollate => 'LC_COLLATE', - datctype => 'LC_CTYPE', datacl => '_null_' }, + datminmxid => '1', dattablespace => 'pg_default', datcollate => 'C', + datctype => 'C', datacl => '_null_' }, ] -- 2.35.1 Attachments: [text/plain] 0001-Simplify-locale-setup-of-template1-in-initdb.patch (2.4K, ../../[email protected]/2-0001-Simplify-locale-setup-of-template1-in-initdb.patch) download | inline diff: From 10143067fb35191aaa53ce2e5c4a20c4601b7528 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <[email protected]> Date: Wed, 16 Feb 2022 11:43:10 +0100 Subject: [PATCH] Simplify locale setup of template1 in initdb --- src/bin/initdb/initdb.c | 23 +++++------------------ src/include/catalog/pg_database.dat | 6 +++--- 2 files changed, 8 insertions(+), 21 deletions(-) diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index 97f15971e2..42e42ca4a4 100644 --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c @@ -621,15 +621,6 @@ get_id(void) return pg_strdup(username); } -static char * -encodingid_to_string(int enc) -{ - char result[20]; - - sprintf(result, "%d", enc); - return pg_strdup(result); -} - /* * get the encoding id for a given encoding name */ @@ -1396,15 +1387,6 @@ bootstrap_template1(void) bki_lines = replace_token(bki_lines, "POSTGRES", escape_quotes_bki(username)); - bki_lines = replace_token(bki_lines, "ENCODING", - encodingid_to_string(encodingid)); - - bki_lines = replace_token(bki_lines, "LC_COLLATE", - escape_quotes_bki(lc_collate)); - - bki_lines = replace_token(bki_lines, "LC_CTYPE", - escape_quotes_bki(lc_ctype)); - /* Also ensure backend isn't confused by this environment var: */ unsetenv("PGCLIENTENCODING"); @@ -1886,6 +1868,11 @@ make_template0(FILE *cmdfd) NULL }; + PG_CMD_PRINTF("UPDATE pg_database " + " SET encoding = %d, datcollate = '%s', datctype = '%s' " + " WHERE datname = 'template1';\n\n", + encodingid, escape_quotes(lc_collate), escape_quotes(lc_ctype)); + for (line = template0_setup; *line; line++) PG_CMD_PUTS(*line); } diff --git a/src/include/catalog/pg_database.dat b/src/include/catalog/pg_database.dat index e7e42d6023..6bca1ec54b 100644 --- a/src/include/catalog/pg_database.dat +++ b/src/include/catalog/pg_database.dat @@ -14,9 +14,9 @@ { oid => '1', oid_symbol => 'TemplateDbOid', descr => 'default template for new databases', - datname => 'template1', encoding => 'ENCODING', datistemplate => 't', + datname => 'template1', encoding => '0', datistemplate => 't', datallowconn => 't', datconnlimit => '-1', datfrozenxid => '0', - datminmxid => '1', dattablespace => 'pg_default', datcollate => 'LC_COLLATE', - datctype => 'LC_CTYPE', datacl => '_null_' }, + datminmxid => '1', dattablespace => 'pg_default', datcollate => 'C', + datctype => 'C', datacl => '_null_' }, ] -- 2.35.1 ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: initdb / bootstrap design 2022-02-16 02:12 initdb / bootstrap design Andres Freund <[email protected]> 2022-02-16 10:47 ` Re: initdb / bootstrap design Peter Eisentraut <[email protected]> @ 2022-02-16 17:53 ` Andres Freund <[email protected]> 0 siblings, 0 replies; 6+ messages in thread From: Andres Freund @ 2022-02-16 17:53 UTC (permalink / raw) To: Peter Eisentraut <[email protected]>; +Cc: pgsql-hackers Hi, On 2022-02-16 11:47:31 +0100, Peter Eisentraut wrote: > On 16.02.22 03:12, Andres Freund wrote: > > Sure, there's a few tokens that we replace in initdb. As it turns out there's > > only two rows that are actually variable. The username of the initial > > superuser in pg_authid and the pg_database row for template 1, where encoding, > > lc_collate and lc_ctype varies. The rest is all compile time constant > > replacements we could do as part of genbki.pl. > > > > It seems we could save a good number of context switches by opening > > postgres.bki just before boot_yyparse() in BootstrapModeMain() and having the > > parser read it. The pg_authid / pg_database rows we could just do via > > explicit insertions in BootstrapModeMain(), provided by commandline args? > > I think we could do the locale setup by updating the pg_database row of > template1 after bootstrap, as in the attached patch. Another solution could be to have bootstrap create template0 instead of template1. I think for template0 it'd more accurate to have a hardcoded C collation and ascii encoding (which I don't think we actually have?). > I suspect we could do the treatment of pg_authid similarly. Yea. Greetings, Andres Freund ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: initdb / bootstrap design 2022-02-16 02:12 initdb / bootstrap design Andres Freund <[email protected]> @ 2022-02-16 18:24 ` Tom Lane <[email protected]> 2 siblings, 0 replies; 6+ messages in thread From: Tom Lane @ 2022-02-16 18:24 UTC (permalink / raw) To: Andres Freund <[email protected]>; +Cc: pgsql-hackers Andres Freund <[email protected]> writes: > Sure, there's a few tokens that we replace in initdb. As it turns out there's > only two rows that are actually variable. The username of the initial > superuser in pg_authid and the pg_database row for template 1, where encoding, > lc_collate and lc_ctype varies. The rest is all compile time constant > replacements we could do as part of genbki.pl. I remembered the reason why it's done that way: if we replaced those values during genbki.pl, the contents of postgres.bki would become architecture-dependent, belying its distribution as a "share" file. While we don't absolutely have to continue treating postgres.bki as architecture-independent, I'm skeptical that there's enough win here to justify a packaging change. initdb is already plenty fast enough for any plausible production usage; it's cases like check-world where we wish it were faster. So I'm thinking what we really ought to pursue is the idea that's been kicked around more than once of capturing the post-initdb state of a cluster's files and just doing "cp -a" to duplicate that later in the test run. regards, tom lane ^ permalink raw reply [nested|flat] 6+ messages in thread
* [PATCH v1 4/8] convert PROC_HDR->startupBufferPinWaitBufId to an atomic @ 2026-07-09 19:26 Nathan Bossart <[email protected]> 0 siblings, 0 replies; 6+ messages in thread From: Nathan Bossart @ 2026-07-09 19:26 UTC (permalink / raw) --- src/backend/storage/lmgr/proc.c | 12 +++--------- src/include/storage/proc.h | 2 +- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index 9d6e69175a5..5f314efb289 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -239,7 +239,7 @@ ProcGlobalShmemInit(void *arg) dlist_init(&ProcGlobal->autovacFreeProcs); dlist_init(&ProcGlobal->bgworkerFreeProcs); dlist_init(&ProcGlobal->walsenderFreeProcs); - ProcGlobal->startupBufferPinWaitBufId = -1; + pg_atomic_init_u32(&ProcGlobal->startupBufferPinWaitBufId, -1); pg_atomic_init_u32(&ProcGlobal->avLauncherProc, INVALID_PROC_NUMBER); pg_atomic_init_u32(&ProcGlobal->walwriterProc, INVALID_PROC_NUMBER); pg_atomic_init_u32(&ProcGlobal->checkpointerProc, INVALID_PROC_NUMBER); @@ -768,10 +768,7 @@ InitAuxiliaryProcess(void) void SetStartupBufferPinWaitBufId(int bufid) { - /* use volatile pointer to prevent code rearrangement */ - volatile PROC_HDR *procglobal = ProcGlobal; - - procglobal->startupBufferPinWaitBufId = bufid; + pg_atomic_write_u32(&ProcGlobal->startupBufferPinWaitBufId, bufid); } /* @@ -780,10 +777,7 @@ SetStartupBufferPinWaitBufId(int bufid) int GetStartupBufferPinWaitBufId(void) { - /* use volatile pointer to prevent code rearrangement */ - volatile PROC_HDR *procglobal = ProcGlobal; - - return procglobal->startupBufferPinWaitBufId; + return pg_atomic_read_u32(&ProcGlobal->startupBufferPinWaitBufId); } /* diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index 03a1a466fa8..e2034255124 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -499,7 +499,7 @@ typedef struct PROC_HDR /* Current shared estimate of appropriate spins_per_delay value */ int spins_per_delay; /* Buffer id of the buffer that Startup process waits for pin on, or -1 */ - int startupBufferPinWaitBufId; + pg_atomic_uint32 startupBufferPinWaitBufId; } PROC_HDR; extern PGDLLIMPORT PROC_HDR *ProcGlobal; -- 2.50.1 (Apple Git-155) --Ot5x3ffkWEuxilWO Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=v1-0005-convert-Sharedsort-currentWorker-workersFinished-.patch ^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2026-07-09 19:26 UTC | newest] Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2022-02-16 02:12 initdb / bootstrap design Andres Freund <[email protected]> 2022-02-16 03:14 ` John Naylor <[email protected]> 2022-02-16 10:47 ` Peter Eisentraut <[email protected]> 2022-02-16 17:53 ` Andres Freund <[email protected]> 2022-02-16 18:24 ` Tom Lane <[email protected]> 2026-07-09 19:26 [PATCH v1 4/8] convert PROC_HDR->startupBufferPinWaitBufId to an atomic Nathan Bossart <[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