Can a background worker exist without shared memory access for EXEC_BACKEND cases?
Hi,
I tried to implement a static background(bg) worker without shared
memory access (BGWORKER_SHMEM_ACCESS), it worked fine on Linux machine
where EXEC_BACKEND is not defined(thanks to the fork() implementation
which does great job to get the global state from the
postmaster(parent) to bg worker(child)).
However, the problem arised, when I switched to EXEC_BACKEND mode, it
seems it doesn't. I digged a bit and the following is my analysis: for
EXEC_BACKEND cases, (say on Windows platforms where fork() doesn't
exist) the way postmaster creates a background worker is entirely
different. It is done through SubPostmasterMain and the global state
from the postmaster is shared with the background worker via shared
memory. MyLatch variable also gets created in shared mode. And having
no shared memory access for the worker for EXEC_BACKEND cases(in
StartBackgroundWorker, the shared memory segments get detached), the
worker fails to receive all the global state from the postmaster.
Looks like the background worker needs to have the
BGWORKER_SHMEM_ACCESS flag while registering for EXEC_BACKEND cases.
Please feel free to correct me if I miss anything here.
If the above analysis looks fine, then please find a patch that adds
some info in bgworker.sgml and bgworker.h.
Thoughts?
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
Attachments:
v1-Background-worker-shared-memory-access-for-EXEC_B.patchapplication/x-patch; name=v1-Background-worker-shared-memory-access-for-EXEC_B.patchDownload
From 6117cf28a14ed1861f9cae2f83fccadf75504c43 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddy@enterprisedb.com>
Date: Fri, 31 Jul 2020 16:42:05 +0530
Subject: [PATCH v1] Background worker shared memory access for EXEC_BACKEND
cases
For EXEC_BACKEND cases, (say on Windows platforms where fork()
doesn't exist) the way postmaster creates a background worker is
different. It is done through SubPostmasterMain and the global
state from the postmaster is shared with the background worker
via shared memory. And having no shared memory access for the
worker for EXEC_BACKEND cases, (in StartBackgroundWorker, the
shared memory segments get detached) the worker fails to receive
all the global state from the postmaster. Looks like the background
worker needs to have the BGWORKER_SHMEM_ACCESS flag while
registering for EXEC_BACKEND cases.
---
doc/src/sgml/bgworker.sgml | 13 ++++++++++++-
src/include/postmaster/bgworker.h | 14 ++++++++++++++
2 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/bgworker.sgml b/doc/src/sgml/bgworker.sgml
index 6e1cf121de..7e2b22eedc 100644
--- a/doc/src/sgml/bgworker.sgml
+++ b/doc/src/sgml/bgworker.sgml
@@ -89,7 +89,18 @@ typedef struct BackgroundWorker
cannot access any of <productname>PostgreSQL's</productname> shared
data structures, such as heavyweight or lightweight locks, shared
buffers, or any custom data structures which the worker itself may
- wish to create and use.
+ wish to create and use. If the background worker doesn't need shared
+ memory access, on non-EXEC_BACKEND cases (such as Unix/Linux platforms)
+ it is fine, since the worker gets all the global state required from the
+ postmaster by the way the fork() is implemented. However, for EXEC_BACKEND
+ cases, (say on Windows platforms where fork() doesn't exist) the way
+ postmaster creates a background worker is different. It is done through
+ SubPostmasterMain and the global state from the postmaster is shared
+ with the background worker via shared memory. And having no shared memory
+ access for the worker for EXEC_BACKEND cases, (in StartBackgroundWorker,
+ the shared memory segments get detached) the worker fails to receive all the
+ global state from the postmaster. Hence the background worker needs to have
+ BGWORKER_SHMEM_ACCESS flag while registering for EXEC_BACKEND cases.
</para>
</listitem>
</varlistentry>
diff --git a/src/include/postmaster/bgworker.h b/src/include/postmaster/bgworker.h
index 4c6ebaa41b..88d1ea74f0 100644
--- a/src/include/postmaster/bgworker.h
+++ b/src/include/postmaster/bgworker.h
@@ -51,6 +51,20 @@
*/
#define BGWORKER_SHMEM_ACCESS 0x0001
+/*
+ * If the background worker doesn't need shared memory access, on non-EXEC_BACKEND
+ * cases (such as Unix/Linux platforms) it is fine, since the worker gets all the
+ * global state required from the postmaster by the way the fork() is implemented.
+ * However, for EXEC_BACKEND cases, (say on Windows platforms where fork() doesn't
+ * exist) the way postmaster creates a background worker is different. It is done
+ * through SubPostmasterMain and the global state from the postmaster is shared
+ * with the background worker via shared memory. And having no shared memory access
+ * for the worker for EXEC_BACKEND cases, (in StartBackgroundWorker, the shared
+ * memory segments get detached) the worker fails to receive all the global state
+ * from the postmaster. Hence the background worker needs to have BGWORKER_SHMEM_ACCESS
+ * flag while registering for EXEC_BACKEND cases.
+ */
+
/*
* This flag means the bgworker requires a database connection. The connection
* is not established automatically; the worker must establish it later.
--
2.25.1
On Fri, Jul 31, 2020 at 11:13 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
memory. MyLatch variable also gets created in shared mode. And having
no shared memory access for the worker for EXEC_BACKEND cases(in
StartBackgroundWorker, the shared memory segments get detached), the
worker fails to receive all the global state from the postmaster.
What exactly do you mean by "all the global state"?
It's certainly true that if you declare some random static variable
and initialize it in the postmaster, and you don't take any special
precautions to propagate that into workers, then on an EXEC_BACKEND
build, it won't be set in the workers. That's why, for example, most
of the *ShmemInit() functions are written like this:
TwoPhaseState = ShmemInitStruct("Prepared Transaction Table",
TwoPhaseShmemSize(),
&found);
if (!IsUnderPostmaster)
...initialize the data structure...
else
Assert(found);
The assignment to TwoPhaseState is unconditional, because in an
EXEC_BACKEND build that's going to be done in every process, and
otherwise the variable won't be set. But the initialization of the
shared data structure happens conditionally, because that needs to be
done only once.
See also the BackendParameters stuff, which arranges to pass down a
bunch of things to exec'd backends.
I am not necessarily opposed to trying to clarify the documentation
and/or comments here, but "global state" is a fuzzy term that doesn't
really mean anything to me.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Thank you Robert for the comments.
On Mon, Aug 3, 2020 at 9:19 PM Robert Haas <robertmhaas@gmail.com> wrote:
On Fri, Jul 31, 2020 at 11:13 PM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:memory. MyLatch variable also gets created in shared mode. And having
no shared memory access for the worker for EXEC_BACKEND cases(in
StartBackgroundWorker, the shared memory segments get detached), the
worker fails to receive all the global state from the postmaster.What exactly do you mean by "all the global state"?
My intention was exactly to refer to the variables specified in
BackendParameters struct.
It's certainly true that if you declare some random static variable
and initialize it in the postmaster, and you don't take any special
precautions to propagate that into workers, then on an EXEC_BACKEND
build, it won't be set in the workers. That's why, for example, most
of the *ShmemInit() functions are written like this:TwoPhaseState = ShmemInitStruct("Prepared Transaction Table",
TwoPhaseShmemSize(),
&found);
if (!IsUnderPostmaster)
...initialize the data structure...
else
Assert(found);The assignment to TwoPhaseState is unconditional, because in an
EXEC_BACKEND build that's going to be done in every process, and
otherwise the variable won't be set. But the initialization of the
shared data structure happens conditionally, because that needs to be
done only once.See also the BackendParameters stuff, which arranges to pass down a
bunch of things to exec'd backends.
I could get these points earlier in my initial analysis. In fact, I
could figure out the flow on Windows, how these parameters are shared
using a shared file(CreateFileMapping(), MapViewOfFile()), and the
shared file name being passed as an argv[2] to the child process, and
the way child process uses this file name to read the backend
parameters in read_backend_variables().
I am not necessarily opposed to trying to clarify the documentation
and/or comments here, but "global state" is a fuzzy term that doesn't
really mean anything to me.
How about having "backend parameters from the postmaster....." as is
being referred to in the internal_forkexec() function comments? I
rephrased the comments adding "backend parameters.." and removing
"global state". Please find the v2 patch attached.
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
Attachments:
v2-Background-worker-shared-memory-access-for-EXEC_B.patchapplication/x-patch; name=v2-Background-worker-shared-memory-access-for-EXEC_B.patchDownload
From f44eb86d221bd6215f85a940610e67e7e9af3524 Mon Sep 17 00:00:00 2001
From: Bharath Rupireddy <bharath.rupireddy@enterprisedb.com>
Date: Tue, 4 Aug 2020 09:45:01 +0530
Subject: [PATCH v2] Background worker shared memory access for EXEC_BACKEND
cases
If the background worker doesn't need shared memory access, on non-EXEC_BACKEND
cases (such as Unix/Linux platforms) it is fine, since the worker gets all the
backend parameters(See BackendParameters struct) required from the postmaster
by the way the fork() is implemented. However, for EXEC_BACKEND cases, (say on
Windows platforms where fork() doesn't exist) the way postmaster creates a
background worker is different. It is done through SubPostmasterMain and the
backend parameters from the postmaster are shared with the background worker via
shared memory. And having no shared memory access for the worker for EXEC_BACKEND
cases, (in StartBackgroundWorker, the shared memory segments get detached) the
worker fails to receive the backend parameters from the postmaster. Hence the
background worker needs to have BGWORKER_SHMEM_ACCESS flag while registering for
EXEC_BACKEND cases.
---
doc/src/sgml/bgworker.sgml | 14 +++++++++++++-
src/include/postmaster/bgworker.h | 15 +++++++++++++++
2 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/doc/src/sgml/bgworker.sgml b/doc/src/sgml/bgworker.sgml
index 6e1cf121de..483d599e55 100644
--- a/doc/src/sgml/bgworker.sgml
+++ b/doc/src/sgml/bgworker.sgml
@@ -89,7 +89,19 @@ typedef struct BackgroundWorker
cannot access any of <productname>PostgreSQL's</productname> shared
data structures, such as heavyweight or lightweight locks, shared
buffers, or any custom data structures which the worker itself may
- wish to create and use.
+ wish to create and use. If the background worker doesn't need shared
+ memory access, on non-EXEC_BACKEND cases (such as Unix/Linux platforms)
+ it is fine, since the worker gets all the backend parameters(See
+ <structname>BackendParameters</structname> structure) required from the
+ postmaster by the way the fork() is implemented. However, for EXEC_BACKEND
+ cases, (say on Windows platforms where fork() doesn't exist) the way postmaster
+ creates a background worker is different. It is done through SubPostmasterMain
+ and the backend parameters from the postmaster are shared with the background
+ worker via shared memory. And having no shared memory access for the worker
+ for EXEC_BACKEND cases, (in StartBackgroundWorker, the shared memory segments
+ get detached) the worker fails to receive the backend parameters from the
+ postmaster. Hence the background worker needs to have BGWORKER_SHMEM_ACCESS
+ flag while registering for EXEC_BACKEND cases.
</para>
</listitem>
</varlistentry>
diff --git a/src/include/postmaster/bgworker.h b/src/include/postmaster/bgworker.h
index 4c6ebaa41b..6ebb00ceda 100644
--- a/src/include/postmaster/bgworker.h
+++ b/src/include/postmaster/bgworker.h
@@ -51,6 +51,21 @@
*/
#define BGWORKER_SHMEM_ACCESS 0x0001
+/*
+ * If the background worker doesn't need shared memory access, on non-EXEC_BACKEND
+ * cases (such as Unix/Linux platforms) it is fine, since the worker gets all the
+ * backend parameters(See BackendParameters struct) required from the postmaster
+ * by the way the fork() is implemented. However, for EXEC_BACKEND cases, (say on
+ * Windows platforms where fork() doesn't exist) the way postmaster creates a
+ * background worker is different. It is done through SubPostmasterMain and the
+ * backend parameters from the postmaster are shared with the background worker via
+ * shared memory. And having no shared memory access for the worker for EXEC_BACKEND
+ * cases, (in StartBackgroundWorker, the shared memory segments get detached) the
+ * worker fails to receive the backend parameters from the postmaster. Hence the
+ * background worker needs to have BGWORKER_SHMEM_ACCESS flag while registering for
+ * EXEC_BACKEND cases.
+ */
+
/*
* This flag means the bgworker requires a database connection. The connection
* is not established automatically; the worker must establish it later.
--
2.25.1
On Tue, Aug 4, 2020 at 7:27 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
I could get these points earlier in my initial analysis. In fact, I
could figure out the flow on Windows, how these parameters are shared
using a shared file(CreateFileMapping(), MapViewOfFile()), and the
shared file name being passed as an argv[2] to the child process, and
the way child process uses this file name to read the backend
parameters in read_backend_variables().
Doesn't that happen even if the background worker isn't declared to
use BGWORKER_SHMEM_ACCESS? See StartBackgroundWorker(): IIUC, we start
with shared memory access, then afterwards detach.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Tue, Aug 4, 2020 at 10:20 PM Robert Haas <robertmhaas@gmail.com> wrote:
On Tue, Aug 4, 2020 at 7:27 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:I could get these points earlier in my initial analysis. In fact, I
could figure out the flow on Windows, how these parameters are shared
using a shared file(CreateFileMapping(), MapViewOfFile()), and the
shared file name being passed as an argv[2] to the child process, and
the way child process uses this file name to read the backend
parameters in read_backend_variables().Doesn't that happen even if the background worker isn't declared to
use BGWORKER_SHMEM_ACCESS? See StartBackgroundWorker(): IIUC, we start
with shared memory access, then afterwards detach.
Yes, the bg worker starts with shared memory access even with no
BGWORKER_SHMEM_ACCESS and later it gets detached in
StartBackgroundWorker() with PGSharedMemoryDetach().
if ((worker->bgw_flags & BGWORKER_SHMEM_ACCESS) == 0)
{
dsm_detach_all();
PGSharedMemoryDetach();
}
In EXEC_BACKEND cases, right after PGSharedMemoryDetach(), the bg
worker will no longer be able to access the backend parameters, see
below(I tried this on my Ubuntu machine with a bgworker with no
BGWORKER_SHMEM_ACCESS flag and defined EXEC_BACKEND macro in
pg_config_manual.h) :
(gdb) p *MyLatch
Cannot access memory at address 0x7fd60424a6b4
(gdb) p *ShmemVariableCache
Cannot access memory at address 0x7fd58427bf80
(gdb) p ProcStructLock
$10 = (slock_t *) 0x7fd60429bd00 <error: Cannot access memory at
address 0x7fd60429bd00>
(gdb) p *AuxiliaryProcs
Cannot access memory at address 0x7fd60424cc60
(gdb) p *ProcGlobal
Cannot access memory at address 0x7fd604232880
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
On Wed, Aug 5, 2020 at 7:24 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
In EXEC_BACKEND cases, right after PGSharedMemoryDetach(), the bg
worker will no longer be able to access the backend parameters, see
below(I tried this on my Ubuntu machine with a bgworker with no
BGWORKER_SHMEM_ACCESS flag and defined EXEC_BACKEND macro in
pg_config_manual.h) :(gdb) p *MyLatch
Cannot access memory at address 0x7fd60424a6b4
(gdb) p *ShmemVariableCache
Cannot access memory at address 0x7fd58427bf80
(gdb) p ProcStructLock
$10 = (slock_t *) 0x7fd60429bd00 <error: Cannot access memory at
address 0x7fd60429bd00>
(gdb) p *AuxiliaryProcs
Cannot access memory at address 0x7fd60424cc60
(gdb) p *ProcGlobal
Cannot access memory at address 0x7fd604232880
Well all of those are pointers into the main shared memory segment,
which is expected to be inaccessible after it is detached. Hopefully
nobody should be surprised that if you don't specify
BGWORKER_SHMEM_ACCESS, you can't access data stored in shared memory.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Wed, Aug 5, 2020 at 5:16 PM Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Aug 5, 2020 at 7:24 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:In EXEC_BACKEND cases, right after PGSharedMemoryDetach(), the bg
worker will no longer be able to access the backend parameters, see
below(I tried this on my Ubuntu machine with a bgworker with no
BGWORKER_SHMEM_ACCESS flag and defined EXEC_BACKEND macro in
pg_config_manual.h) :(gdb) p *MyLatch
Cannot access memory at address 0x7fd60424a6b4
(gdb) p *ShmemVariableCache
Cannot access memory at address 0x7fd58427bf80
(gdb) p ProcStructLock
$10 = (slock_t *) 0x7fd60429bd00 <error: Cannot access memory at
address 0x7fd60429bd00>
(gdb) p *AuxiliaryProcs
Cannot access memory at address 0x7fd60424cc60
(gdb) p *ProcGlobal
Cannot access memory at address 0x7fd604232880Well all of those are pointers into the main shared memory segment,
which is expected to be inaccessible after it is detached. Hopefully
nobody should be surprised that if you don't specify
BGWORKER_SHMEM_ACCESS, you can't access data stored in shared memory.
Right.
Will the proposed patch(v2) having some info in bgworker.sgml and
bgworker.h be ever useful to the users in some way?
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
On Wed, Aug 5, 2020 at 9:02 AM Bharath Rupireddy
<bharath.rupireddyforpostgres@gmail.com> wrote:
Will the proposed patch(v2) having some info in bgworker.sgml and
bgworker.h be ever useful to the users in some way?
Well, it says things that aren't true, so, no, it's not useful. Your
patch claims that "the worker fails to receive the backend parameters
from the postmaster," but that's not the case. SubPostmasterMain()
first calls read_backend_variables() which calls
restore_backend_variables(); then later it calls
StartBackgroundWorker() which does PGSharedMemoryDetach(). So the
values of the backend variables *are* available in the worker
processes. Your debugger output also shows this: if
restore_backend_variables() weren't running in the child processes,
those variables would all be NULL, but you show them all at different
addresses in the 0x7fd... range, which is presumably where the shared
memory segment was mapped.
The reason you can't print out the results of dereferencing the
variables with * is because the memory to which the variables point is
no longer mapped in the process, not because the variables haven't
been initialized. If you looked at a variable that wasn't a pointer to
shared memory, but rather, say, an integer, like max_safe_fds or
MyCancelKey, I think you'd find that the value was preserved just
fine. I think you're confusing the pointer with the data to which it
points.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company