Thank you Robert for the comments.
On Mon, Aug 3, 2020 at 9:19 PM Robert Haas <robertmh...@gmail.com> wrote:
>
> On Fri, Jul 31, 2020 at 11:13 PM Bharath Rupireddy
> <bharath.rupireddyforpostg...@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
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
