Set appropriate processing mode for auxiliary processes.
Hi hackers,
After several refactoring iterations, auxiliary processes are no
longer initialized from the bootstrapper. I think using the
InitProcessing mode for initializing auxiliary processes is more
appropriate.
Best Regards,
Xing
Attachments:
v1-0001-Set-appropriate-processing-mode-for-auxiliary-pro.patchtext/x-patch; charset=US-ASCII; name=v1-0001-Set-appropriate-processing-mode-for-auxiliary-pro.patchDownload
From 50fb1ccc7259e0ce8512dee236cbfe11635c15fc Mon Sep 17 00:00:00 2001
From: Xing Guo <higuoxing@gmail.com>
Date: Thu, 9 May 2024 20:57:48 +0800
Subject: [PATCH v1] Set appropriate processing mode for auxiliary processes.
After several refactoring iterations, auxiliary processes are no longer
initialized from the bootstrapper. Using the InitProcessing mode for
initializing auxiliary processes is more appropriate.
---
src/backend/postmaster/auxprocess.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/postmaster/auxprocess.c b/src/backend/postmaster/auxprocess.c
index 78f4263eeb..ca4bfa2ed5 100644
--- a/src/backend/postmaster/auxprocess.c
+++ b/src/backend/postmaster/auxprocess.c
@@ -54,7 +54,7 @@ AuxiliaryProcessMainCommon(void)
init_ps_display(NULL);
- SetProcessingMode(BootstrapProcessing);
+ SetProcessingMode(InitProcessing);
IgnoreSystemIndexes = true;
/*
--
2.45.0
On 09/05/2024 16:12, Xing Guo wrote:
Hi hackers,
After several refactoring iterations, auxiliary processes are no
longer initialized from the bootstrapper. I think using the
InitProcessing mode for initializing auxiliary processes is more
appropriate.
At first I was sure this was introduced by my refactorings in v17, but
in fact it's been like this forever. I agree that InitProcessing makes
much more sense. The ProcessingMode variable is initialized to
InitProcessing, so I think we can simply remove that line from
AuxiliaryProcessMainCommon(). There are existing
"SetProcessingMode(InitProcessing)" calls in other Main functions too
(AutoVacLauncherMain, BackgroundWorkerMain, etc.), and I think those can
also be removed.
--
Heikki Linnakangas
Neon (https://neon.tech)
On Thu, May 9, 2024 at 10:13 PM Heikki Linnakangas <hlinnaka@iki.fi> wrote:
On 09/05/2024 16:12, Xing Guo wrote:
Hi hackers,
After several refactoring iterations, auxiliary processes are no
longer initialized from the bootstrapper. I think using the
InitProcessing mode for initializing auxiliary processes is more
appropriate.At first I was sure this was introduced by my refactorings in v17, but
in fact it's been like this forever. I agree that InitProcessing makes
much more sense. The ProcessingMode variable is initialized to
InitProcessing, so I think we can simply remove that line from
AuxiliaryProcessMainCommon(). There are existing
"SetProcessingMode(InitProcessing)" calls in other Main functions too
(AutoVacLauncherMain, BackgroundWorkerMain, etc.), and I think those can
also be removed.
Good catch! I agree with you.
Best Regards,
Xing.
Attachments:
v2-0001-Remove-redundant-SetProcessingMode-InitProcessing.patchtext/x-patch; charset=US-ASCII; name=v2-0001-Remove-redundant-SetProcessingMode-InitProcessing.patchDownload
From 210e97c88fd49853d2a6304763ef7e8ad65e1b79 Mon Sep 17 00:00:00 2001
From: Xing Guo <higuoxing@gmail.com>
Date: Thu, 9 May 2024 20:57:48 +0800
Subject: [PATCH v2] Remove redundant SetProcessingMode(InitProcessing) calls.
After several refactoring iterations, auxiliary processes are no longer
initialized from the bootstrapper. Using the InitProcessing mode for
initializing auxiliary processes is more appropriate. Since the global
variable Mode is initialized to InitProcessing, we can remove redundant
calls of SetProcessingMode(InitProcessing).
---
src/backend/postmaster/autovacuum.c | 4 ----
src/backend/postmaster/auxprocess.c | 1 -
src/backend/postmaster/bgworker.c | 2 --
src/backend/replication/logical/slotsync.c | 2 --
src/backend/tcop/postgres.c | 2 --
5 files changed, 11 deletions(-)
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 9a925a10cd..5a1cfabf19 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -380,8 +380,6 @@ AutoVacLauncherMain(char *startup_data, size_t startup_data_len)
if (PostAuthDelay)
pg_usleep(PostAuthDelay * 1000000L);
- SetProcessingMode(InitProcessing);
-
/*
* Set up signal handlers. We operate on databases much like a regular
* backend, so we use the same signal handling. See equivalent code in
@@ -1373,8 +1371,6 @@ AutoVacWorkerMain(char *startup_data, size_t startup_data_len)
MyBackendType = B_AUTOVAC_WORKER;
init_ps_display(NULL);
- SetProcessingMode(InitProcessing);
-
/*
* Set up signal handlers. We operate on databases much like a regular
* backend, so we use the same signal handling. See equivalent code in
diff --git a/src/backend/postmaster/auxprocess.c b/src/backend/postmaster/auxprocess.c
index 78f4263eeb..0faf7df0b6 100644
--- a/src/backend/postmaster/auxprocess.c
+++ b/src/backend/postmaster/auxprocess.c
@@ -54,7 +54,6 @@ AuxiliaryProcessMainCommon(void)
init_ps_display(NULL);
- SetProcessingMode(BootstrapProcessing);
IgnoreSystemIndexes = true;
/*
diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index cf64a4beb2..5bbe02bbc3 100644
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -746,8 +746,6 @@ BackgroundWorkerMain(char *startup_data, size_t startup_data_len)
MyBackendType = B_BG_WORKER;
init_ps_display(worker->bgw_name);
- SetProcessingMode(InitProcessing);
-
/* Apply PostAuthDelay */
if (PostAuthDelay > 0)
pg_usleep(PostAuthDelay * 1000000L);
diff --git a/src/backend/replication/logical/slotsync.c b/src/backend/replication/logical/slotsync.c
index f1f44d41ef..72857694a5 100644
--- a/src/backend/replication/logical/slotsync.c
+++ b/src/backend/replication/logical/slotsync.c
@@ -1342,8 +1342,6 @@ ReplSlotSyncWorkerMain(char *startup_data, size_t startup_data_len)
init_ps_display(NULL);
- SetProcessingMode(InitProcessing);
-
/*
* Create a per-backend PGPROC struct in shared memory. We must do this
* before we access any shared memory.
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 2dff28afce..4b2de1dc70 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -4161,8 +4161,6 @@ PostgresMain(const char *dbname, const char *username)
Assert(dbname != NULL);
Assert(username != NULL);
- SetProcessingMode(InitProcessing);
-
/*
* Set up signal handlers. (InitPostmasterChild or InitStandaloneProcess
* has already set up BlockSig and made that the active signal mask.)
--
2.45.0
Sorry, forget to add an assertion to guard our codes in my previous patch.
Attachments:
v3-0001-Remove-redundant-SetProcessingMode-InitProcessing.patchtext/x-patch; charset=US-ASCII; name=v3-0001-Remove-redundant-SetProcessingMode-InitProcessing.patchDownload
From 4f2d70fb27e3ff23b8572c5e679c791daa1f6665 Mon Sep 17 00:00:00 2001
From: Xing Guo <higuoxing@gmail.com>
Date: Thu, 9 May 2024 20:57:48 +0800
Subject: [PATCH v3] Remove redundant SetProcessingMode(InitProcessing) calls.
After several refactoring iterations, auxiliary processes are no longer
initialized from the bootstrapper. Using the InitProcessing mode for
initializing auxiliary processes is more appropriate. Since the global
variable Mode is initialized to InitProcessing, we can remove redundant
calls of SetProcessingMode(InitProcessing).
---
src/backend/postmaster/autovacuum.c | 4 ++--
src/backend/postmaster/auxprocess.c | 3 ++-
src/backend/postmaster/bgworker.c | 2 +-
src/backend/replication/logical/slotsync.c | 2 +-
src/backend/tcop/postgres.c | 2 +-
5 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 9a925a10cd..928754b51c 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -380,7 +380,7 @@ AutoVacLauncherMain(char *startup_data, size_t startup_data_len)
if (PostAuthDelay)
pg_usleep(PostAuthDelay * 1000000L);
- SetProcessingMode(InitProcessing);
+ Assert(GetProcessingMode() == InitProcessing);
/*
* Set up signal handlers. We operate on databases much like a regular
@@ -1373,7 +1373,7 @@ AutoVacWorkerMain(char *startup_data, size_t startup_data_len)
MyBackendType = B_AUTOVAC_WORKER;
init_ps_display(NULL);
- SetProcessingMode(InitProcessing);
+ Assert(GetProcessingMode() == InitProcessing);
/*
* Set up signal handlers. We operate on databases much like a regular
diff --git a/src/backend/postmaster/auxprocess.c b/src/backend/postmaster/auxprocess.c
index 78f4263eeb..b21eae7136 100644
--- a/src/backend/postmaster/auxprocess.c
+++ b/src/backend/postmaster/auxprocess.c
@@ -54,7 +54,8 @@ AuxiliaryProcessMainCommon(void)
init_ps_display(NULL);
- SetProcessingMode(BootstrapProcessing);
+ Assert(GetProcessingMode() == InitProcessing);
+
IgnoreSystemIndexes = true;
/*
diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index cf64a4beb2..dc20a25345 100644
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -746,7 +746,7 @@ BackgroundWorkerMain(char *startup_data, size_t startup_data_len)
MyBackendType = B_BG_WORKER;
init_ps_display(worker->bgw_name);
- SetProcessingMode(InitProcessing);
+ Assert(GetProcessingMode() == InitProcessing);
/* Apply PostAuthDelay */
if (PostAuthDelay > 0)
diff --git a/src/backend/replication/logical/slotsync.c b/src/backend/replication/logical/slotsync.c
index f1f44d41ef..2653ce0342 100644
--- a/src/backend/replication/logical/slotsync.c
+++ b/src/backend/replication/logical/slotsync.c
@@ -1342,7 +1342,7 @@ ReplSlotSyncWorkerMain(char *startup_data, size_t startup_data_len)
init_ps_display(NULL);
- SetProcessingMode(InitProcessing);
+ Assert(GetProcessingMode() == InitProcessing);
/*
* Create a per-backend PGPROC struct in shared memory. We must do this
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 2dff28afce..b9fee13612 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -4161,7 +4161,7 @@ PostgresMain(const char *dbname, const char *username)
Assert(dbname != NULL);
Assert(username != NULL);
- SetProcessingMode(InitProcessing);
+ Assert(GetProcessingMode() == InitProcessing);
/*
* Set up signal handlers. (InitPostmasterChild or InitStandaloneProcess
--
2.45.0
Heikki Linnakangas <hlinnaka@iki.fi> writes:
At first I was sure this was introduced by my refactorings in v17, but
in fact it's been like this forever. I agree that InitProcessing makes
much more sense. The ProcessingMode variable is initialized to
InitProcessing, so I think we can simply remove that line from
AuxiliaryProcessMainCommon(). There are existing
"SetProcessingMode(InitProcessing)" calls in other Main functions too
(AutoVacLauncherMain, BackgroundWorkerMain, etc.), and I think those can
also be removed.
This only works if the postmaster can be trusted never to change the
variable; else children could inherit some other value via fork().
In that connection, it seems a bit scary that postmaster.c contains a
couple of calls "SetProcessingMode(NormalProcessing)". It looks like
they are in functions that should only be executed by child processes,
but should we try to move them somewhere else? Another idea could be
to add an Assert to SetProcessingMode that insists that it can't be
executed by the postmaster.
regards, tom lane
On Thu, May 9, 2024 at 11:19 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Heikki Linnakangas <hlinnaka@iki.fi> writes:
At first I was sure this was introduced by my refactorings in v17, but
in fact it's been like this forever. I agree that InitProcessing makes
much more sense. The ProcessingMode variable is initialized to
InitProcessing, so I think we can simply remove that line from
AuxiliaryProcessMainCommon(). There are existing
"SetProcessingMode(InitProcessing)" calls in other Main functions too
(AutoVacLauncherMain, BackgroundWorkerMain, etc.), and I think those can
also be removed.This only works if the postmaster can be trusted never to change the
variable; else children could inherit some other value via fork().
In that connection, it seems a bit scary that postmaster.c contains a
couple of calls "SetProcessingMode(NormalProcessing)". It looks like
they are in functions that should only be executed by child processes,
but should we try to move them somewhere else?
After checking calls to "SetProcessingMode(NormalProcessing)" in the
postmaster.c, they are used in background worker specific functions
(BackgroundWorkerInitializeConnectionByOid and
BackgroundWorkerInitializeConnection). So I think it's a good idea to
move these functions to bgworker.c. Then, we can get rid of calling
"SetProcessingMode(NormalProcessing)" in postmaster.c.
I also noticed that there's an unnecessary call to
"BackgroundWorkerInitializeConnection" in worker_spi.c (The worker_spi
launcher has set the dboid correctly).
Best Regards,
Xing.
Attachments:
v4-0001-Move-bgworker-specific-logic-to-bgworker.c.patchtext/x-patch; charset=US-ASCII; name=v4-0001-Move-bgworker-specific-logic-to-bgworker.c.patchDownload
From 4dbe54ef746203740202e181f731b7e08bb6e4ea Mon Sep 17 00:00:00 2001
From: Xing Guo <higuoxing@gmail.com>
Date: Fri, 10 May 2024 10:34:09 +0800
Subject: [PATCH v4 1/3] Move bgworker specific logic to bgworker.c.
---
src/backend/postmaster/bgworker.c | 83 +++++++++++++++++++++++++++++
src/backend/postmaster/postmaster.c | 83 -----------------------------
2 files changed, 83 insertions(+), 83 deletions(-)
diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index cf64a4beb2..0ab64ae3ec 100644
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -1240,6 +1240,89 @@ TerminateBackgroundWorker(BackgroundWorkerHandle *handle)
SendPostmasterSignal(PMSIGNAL_BACKGROUND_WORKER_CHANGE);
}
+/*
+ * Connect background worker to a database.
+ */
+void
+BackgroundWorkerInitializeConnection(const char *dbname, const char *username, uint32 flags)
+{
+ BackgroundWorker *worker = MyBgworkerEntry;
+ bits32 init_flags = 0; /* never honor session_preload_libraries */
+
+ /* ignore datallowconn? */
+ if (flags & BGWORKER_BYPASS_ALLOWCONN)
+ init_flags |= INIT_PG_OVERRIDE_ALLOW_CONNS;
+ /* ignore rolcanlogin? */
+ if (flags & BGWORKER_BYPASS_ROLELOGINCHECK)
+ init_flags |= INIT_PG_OVERRIDE_ROLE_LOGIN;
+
+ /* XXX is this the right errcode? */
+ if (!(worker->bgw_flags & BGWORKER_BACKEND_DATABASE_CONNECTION))
+ ereport(FATAL,
+ (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+ errmsg("database connection requirement not indicated during registration")));
+
+ InitPostgres(dbname, InvalidOid, /* database to connect to */
+ username, InvalidOid, /* role to connect as */
+ init_flags,
+ NULL); /* no out_dbname */
+
+ /* it had better not gotten out of "init" mode yet */
+ if (!IsInitProcessingMode())
+ ereport(ERROR,
+ (errmsg("invalid processing mode in background worker")));
+ SetProcessingMode(NormalProcessing);
+}
+
+/*
+ * Connect background worker to a database using OIDs.
+ */
+void
+BackgroundWorkerInitializeConnectionByOid(Oid dboid, Oid useroid, uint32 flags)
+{
+ BackgroundWorker *worker = MyBgworkerEntry;
+ bits32 init_flags = 0; /* never honor session_preload_libraries */
+
+ /* ignore datallowconn? */
+ if (flags & BGWORKER_BYPASS_ALLOWCONN)
+ init_flags |= INIT_PG_OVERRIDE_ALLOW_CONNS;
+ /* ignore rolcanlogin? */
+ if (flags & BGWORKER_BYPASS_ROLELOGINCHECK)
+ init_flags |= INIT_PG_OVERRIDE_ROLE_LOGIN;
+
+ /* XXX is this the right errcode? */
+ if (!(worker->bgw_flags & BGWORKER_BACKEND_DATABASE_CONNECTION))
+ ereport(FATAL,
+ (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+ errmsg("database connection requirement not indicated during registration")));
+
+ InitPostgres(NULL, dboid, /* database to connect to */
+ NULL, useroid, /* role to connect as */
+ init_flags,
+ NULL); /* no out_dbname */
+
+ /* it had better not gotten out of "init" mode yet */
+ if (!IsInitProcessingMode())
+ ereport(ERROR,
+ (errmsg("invalid processing mode in background worker")));
+ SetProcessingMode(NormalProcessing);
+}
+
+/*
+ * Block/unblock signals in a background worker
+ */
+void
+BackgroundWorkerBlockSignals(void)
+{
+ sigprocmask(SIG_SETMASK, &BlockSig, NULL);
+}
+
+void
+BackgroundWorkerUnblockSignals(void)
+{
+ sigprocmask(SIG_SETMASK, &UnBlockSig, NULL);
+}
+
/*
* Look up (and possibly load) a bgworker entry point function.
*
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 7f3170a8f0..472eb80fd1 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -4148,89 +4148,6 @@ MaxLivePostmasterChildren(void)
max_wal_senders + max_worker_processes);
}
-/*
- * Connect background worker to a database.
- */
-void
-BackgroundWorkerInitializeConnection(const char *dbname, const char *username, uint32 flags)
-{
- BackgroundWorker *worker = MyBgworkerEntry;
- bits32 init_flags = 0; /* never honor session_preload_libraries */
-
- /* ignore datallowconn? */
- if (flags & BGWORKER_BYPASS_ALLOWCONN)
- init_flags |= INIT_PG_OVERRIDE_ALLOW_CONNS;
- /* ignore rolcanlogin? */
- if (flags & BGWORKER_BYPASS_ROLELOGINCHECK)
- init_flags |= INIT_PG_OVERRIDE_ROLE_LOGIN;
-
- /* XXX is this the right errcode? */
- if (!(worker->bgw_flags & BGWORKER_BACKEND_DATABASE_CONNECTION))
- ereport(FATAL,
- (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
- errmsg("database connection requirement not indicated during registration")));
-
- InitPostgres(dbname, InvalidOid, /* database to connect to */
- username, InvalidOid, /* role to connect as */
- init_flags,
- NULL); /* no out_dbname */
-
- /* it had better not gotten out of "init" mode yet */
- if (!IsInitProcessingMode())
- ereport(ERROR,
- (errmsg("invalid processing mode in background worker")));
- SetProcessingMode(NormalProcessing);
-}
-
-/*
- * Connect background worker to a database using OIDs.
- */
-void
-BackgroundWorkerInitializeConnectionByOid(Oid dboid, Oid useroid, uint32 flags)
-{
- BackgroundWorker *worker = MyBgworkerEntry;
- bits32 init_flags = 0; /* never honor session_preload_libraries */
-
- /* ignore datallowconn? */
- if (flags & BGWORKER_BYPASS_ALLOWCONN)
- init_flags |= INIT_PG_OVERRIDE_ALLOW_CONNS;
- /* ignore rolcanlogin? */
- if (flags & BGWORKER_BYPASS_ROLELOGINCHECK)
- init_flags |= INIT_PG_OVERRIDE_ROLE_LOGIN;
-
- /* XXX is this the right errcode? */
- if (!(worker->bgw_flags & BGWORKER_BACKEND_DATABASE_CONNECTION))
- ereport(FATAL,
- (errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
- errmsg("database connection requirement not indicated during registration")));
-
- InitPostgres(NULL, dboid, /* database to connect to */
- NULL, useroid, /* role to connect as */
- init_flags,
- NULL); /* no out_dbname */
-
- /* it had better not gotten out of "init" mode yet */
- if (!IsInitProcessingMode())
- ereport(ERROR,
- (errmsg("invalid processing mode in background worker")));
- SetProcessingMode(NormalProcessing);
-}
-
-/*
- * Block/unblock signals in a background worker
- */
-void
-BackgroundWorkerBlockSignals(void)
-{
- sigprocmask(SIG_SETMASK, &BlockSig, NULL);
-}
-
-void
-BackgroundWorkerUnblockSignals(void)
-{
- sigprocmask(SIG_SETMASK, &UnBlockSig, NULL);
-}
-
/*
* Start a new bgworker.
* Starting time conditions must have been checked already.
--
2.45.0
v4-0002-Remove-redundant-SetProcessingMode-InitProcessing.patchtext/x-patch; charset=US-ASCII; name=v4-0002-Remove-redundant-SetProcessingMode-InitProcessing.patchDownload
From fa210ff72c4e4e55ee2d339070a142c134b3650c Mon Sep 17 00:00:00 2001
From: Xing Guo <higuoxing@gmail.com>
Date: Fri, 10 May 2024 10:31:19 +0800
Subject: [PATCH v4 2/3] Remove redundant SetProcessingMode(InitProcessing)
calls.
After several refactoring iterations, auxiliary processes are no longer
initialized from the bootstrapper. Using the InitProcessing mode for
initializing auxiliary processes is more appropriate. Since the global
variable Mode is initialized to InitProcessing, we can remove redundant
calls of SetProcessingMode(InitProcessing).
---
src/backend/postmaster/autovacuum.c | 4 ++--
src/backend/postmaster/auxprocess.c | 3 ++-
src/backend/postmaster/bgworker.c | 2 +-
src/backend/replication/logical/slotsync.c | 2 +-
src/backend/tcop/postgres.c | 2 +-
5 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 9a925a10cd..928754b51c 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -380,7 +380,7 @@ AutoVacLauncherMain(char *startup_data, size_t startup_data_len)
if (PostAuthDelay)
pg_usleep(PostAuthDelay * 1000000L);
- SetProcessingMode(InitProcessing);
+ Assert(GetProcessingMode() == InitProcessing);
/*
* Set up signal handlers. We operate on databases much like a regular
@@ -1373,7 +1373,7 @@ AutoVacWorkerMain(char *startup_data, size_t startup_data_len)
MyBackendType = B_AUTOVAC_WORKER;
init_ps_display(NULL);
- SetProcessingMode(InitProcessing);
+ Assert(GetProcessingMode() == InitProcessing);
/*
* Set up signal handlers. We operate on databases much like a regular
diff --git a/src/backend/postmaster/auxprocess.c b/src/backend/postmaster/auxprocess.c
index 78f4263eeb..b21eae7136 100644
--- a/src/backend/postmaster/auxprocess.c
+++ b/src/backend/postmaster/auxprocess.c
@@ -54,7 +54,8 @@ AuxiliaryProcessMainCommon(void)
init_ps_display(NULL);
- SetProcessingMode(BootstrapProcessing);
+ Assert(GetProcessingMode() == InitProcessing);
+
IgnoreSystemIndexes = true;
/*
diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index 0ab64ae3ec..92ec1c0e94 100644
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -746,7 +746,7 @@ BackgroundWorkerMain(char *startup_data, size_t startup_data_len)
MyBackendType = B_BG_WORKER;
init_ps_display(worker->bgw_name);
- SetProcessingMode(InitProcessing);
+ Assert(GetProcessingMode() == InitProcessing);
/* Apply PostAuthDelay */
if (PostAuthDelay > 0)
diff --git a/src/backend/replication/logical/slotsync.c b/src/backend/replication/logical/slotsync.c
index f1f44d41ef..2653ce0342 100644
--- a/src/backend/replication/logical/slotsync.c
+++ b/src/backend/replication/logical/slotsync.c
@@ -1342,7 +1342,7 @@ ReplSlotSyncWorkerMain(char *startup_data, size_t startup_data_len)
init_ps_display(NULL);
- SetProcessingMode(InitProcessing);
+ Assert(GetProcessingMode() == InitProcessing);
/*
* Create a per-backend PGPROC struct in shared memory. We must do this
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 2dff28afce..b9fee13612 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -4161,7 +4161,7 @@ PostgresMain(const char *dbname, const char *username)
Assert(dbname != NULL);
Assert(username != NULL);
- SetProcessingMode(InitProcessing);
+ Assert(GetProcessingMode() == InitProcessing);
/*
* Set up signal handlers. (InitPostmasterChild or InitStandaloneProcess
--
2.45.0
v4-0003-Minor-improvement-to-connection-initialization-fo.patchtext/x-patch; charset=US-ASCII; name=v4-0003-Minor-improvement-to-connection-initialization-fo.patchDownload
From c445fbefa327edb7518da88c2783c17e6715a572 Mon Sep 17 00:00:00 2001
From: Xing Guo <higuoxing@gmail.com>
Date: Fri, 10 May 2024 10:46:04 +0800
Subject: [PATCH v4 3/3] Minor improvement to connection initialization for
worker_spi.
---
src/test/modules/worker_spi/worker_spi.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/test/modules/worker_spi/worker_spi.c b/src/test/modules/worker_spi/worker_spi.c
index 7e1042f4ab..23f4b165dc 100644
--- a/src/test/modules/worker_spi/worker_spi.c
+++ b/src/test/modules/worker_spi/worker_spi.c
@@ -167,11 +167,8 @@ worker_spi_main(Datum main_arg)
BackgroundWorkerUnblockSignals();
/* Connect to our database */
- if (OidIsValid(dboid))
- BackgroundWorkerInitializeConnectionByOid(dboid, roleoid, flags);
- else
- BackgroundWorkerInitializeConnection(worker_spi_database,
- worker_spi_role, flags);
+ Assert(OidIsValid(dboid));
+ BackgroundWorkerInitializeConnectionByOid(dboid, roleoid, flags);
/*
* Disable parallel query for workers started with BYPASS_ALLOWCONN or
--
2.45.0
On 10/05/2024 05:58, Xing Guo wrote:
On Thu, May 9, 2024 at 11:19 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Heikki Linnakangas <hlinnaka@iki.fi> writes:
At first I was sure this was introduced by my refactorings in v17, but
in fact it's been like this forever. I agree that InitProcessing makes
much more sense. The ProcessingMode variable is initialized to
InitProcessing, so I think we can simply remove that line from
AuxiliaryProcessMainCommon(). There are existing
"SetProcessingMode(InitProcessing)" calls in other Main functions too
(AutoVacLauncherMain, BackgroundWorkerMain, etc.), and I think those can
also be removed.This only works if the postmaster can be trusted never to change the
variable; else children could inherit some other value via fork().
In that connection, it seems a bit scary that postmaster.c contains a
couple of calls "SetProcessingMode(NormalProcessing)". It looks like
they are in functions that should only be executed by child processes,
but should we try to move them somewhere else?After checking calls to "SetProcessingMode(NormalProcessing)" in the
postmaster.c, they are used in background worker specific functions
(BackgroundWorkerInitializeConnectionByOid and
BackgroundWorkerInitializeConnection). So I think it's a good idea to
move these functions to bgworker.c. Then, we can get rid of calling
"SetProcessingMode(NormalProcessing)" in postmaster.c.
Committed these first two patches. Thank you!
I also noticed that there's an unnecessary call to
"BackgroundWorkerInitializeConnection" in worker_spi.c (The worker_spi
launcher has set the dboid correctly).
No, you can call the launcher function with "dboid=0", and it's also 0
in the "static" registration at end of _PG_init(). This causes
regression tests to fail too because of that. So I left out that patch.
--
Heikki Linnakangas
Neon (https://neon.tech)