From eee5abce46fec45ad40fef75b8d81e8f3ac7dda2 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Wed, 29 Dec 2004 21:36:09 +0000
Subject: [PATCH] Refactor EXEC_BACKEND code so that postmaster child processes
reattach to shared memory as soon as possible, ie, right after
read_backend_variables. The effective difference from the original code is
that this happens before instead of after read_nondefault_variables(), which
loads GUC information and is apparently capable of expanding the backend's
memory allocation more than you'd think it should. This should fix the
failure-to-attach-to-shared-memory reports we've been seeing on Windows. Also
clean up a few bits of unnecessarily grotty EXEC_BACKEND code.
---
src/backend/port/sysv_shmem.c | 75 +++++++++++-------
src/backend/postmaster/postmaster.c | 60 ++++++++------
src/backend/storage/file/fd.c | 119 +++++++++++++++++-----------
src/backend/storage/ipc/ipci.c | 30 ++++---
src/include/postmaster/postmaster.h | 5 +-
src/include/storage/ipc.h | 8 +-
src/include/storage/pg_shmem.h | 4 +-
7 files changed, 178 insertions(+), 123 deletions(-)
diff --git a/src/backend/port/sysv_shmem.c b/src/backend/port/sysv_shmem.c
index 3c3f928ca2..63ff4ad57e 100644
--- a/src/backend/port/sysv_shmem.c
+++ b/src/backend/port/sysv_shmem.c
@@ -10,7 +10,7 @@
* Portions Copyright (c) 1994, Regents of the University of California
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/port/sysv_shmem.c,v 1.40 2004/11/09 21:30:13 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/port/sysv_shmem.c,v 1.41 2004/12/29 21:35:58 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -142,11 +142,7 @@ InternalIpcMemoryCreate(IpcMemoryKey memKey, uint32 size)
on_shmem_exit(IpcMemoryDelete, Int32GetDatum(shmid));
/* OK, should be able to attach to the segment */
-#ifdef EXEC_BACKEND
- memAddress = shmat(shmid, UsedShmemSegAddr, PG_SHMAT_FLAGS);
-#else
memAddress = shmat(shmid, NULL, PG_SHMAT_FLAGS);
-#endif
if (memAddress == (void *) -1)
elog(FATAL, "shmat(id=%d) failed: %m", shmid);
@@ -279,7 +275,7 @@ PGSharedMemoryIsInUse(unsigned long id1, unsigned long id2)
*
* Create a shared memory segment of the given size and initialize its
* standard header. Also, register an on_shmem_exit callback to release
- * the storage. For an exec'ed backend, it just attaches.
+ * the storage.
*
* Dead Postgres segments are recycled if found, but we do not fail upon
* collision with non-Postgres shmem segments. The idea here is to detect and
@@ -303,30 +299,6 @@ PGSharedMemoryCreate(uint32 size, bool makePrivate, int port)
struct stat statbuf;
#endif
-#ifdef EXEC_BACKEND
- /* If Exec case, just attach and return the pointer */
- if (UsedShmemSegAddr != NULL && !makePrivate && IsUnderPostmaster)
- {
- void *origUsedShmemSegAddr = UsedShmemSegAddr;
-
-#ifdef __CYGWIN__
- /* cygipc (currently) appears to not detach on exec. */
- PGSharedMemoryDetach();
- UsedShmemSegAddr = origUsedShmemSegAddr;
-#endif
- elog(DEBUG3, "Attaching to %p", UsedShmemSegAddr);
- hdr = PGSharedMemoryAttach((IpcMemoryKey) UsedShmemSegID, &shmid);
- if (hdr == NULL)
- elog(FATAL, "could not attach to proper memory at fixed address: shmget(key=%d, addr=%p) failed: %m",
- (int) UsedShmemSegID, UsedShmemSegAddr);
- if (hdr != origUsedShmemSegAddr)
- elog(FATAL, "attaching to shared mem returned unexpected address (got %p, expected %p)",
- hdr, UsedShmemSegAddr);
- UsedShmemSegAddr = hdr;
- return hdr;
- }
-#endif
-
/* Room for a header? */
Assert(size > MAXALIGN(sizeof(PGShmemHeader)));
@@ -424,6 +396,49 @@ PGSharedMemoryCreate(uint32 size, bool makePrivate, int port)
return hdr;
}
+#ifdef EXEC_BACKEND
+
+/*
+ * PGSharedMemoryReAttach
+ *
+ * Re-attach to an already existing shared memory segment. In the non
+ * EXEC_BACKEND case this is not used, because postmaster children inherit
+ * the shared memory segment attachment via fork().
+ *
+ * UsedShmemSegID and UsedShmemSegAddr are implicit parameters to this
+ * routine. The caller must have already restored them to the postmaster's
+ * values.
+ */
+void
+PGSharedMemoryReAttach(void)
+{
+ IpcMemoryId shmid;
+ void *hdr;
+ void *origUsedShmemSegAddr = UsedShmemSegAddr;
+
+ Assert(UsedShmemSegAddr != NULL);
+ Assert(IsUnderPostmaster);
+
+#ifdef __CYGWIN__
+ /* cygipc (currently) appears to not detach on exec. */
+ PGSharedMemoryDetach();
+ UsedShmemSegAddr = origUsedShmemSegAddr;
+#endif
+
+ elog(DEBUG3, "Attaching to %p", UsedShmemSegAddr);
+ hdr = (void *) PGSharedMemoryAttach((IpcMemoryKey) UsedShmemSegID, &shmid);
+ if (hdr == NULL)
+ elog(FATAL, "could not reattach to shared memory (key=%d, addr=%p): %m",
+ (int) UsedShmemSegID, UsedShmemSegAddr);
+ if (hdr != origUsedShmemSegAddr)
+ elog(FATAL, "reattaching to shared memory returned unexpected address (got %p, expected %p)",
+ hdr, origUsedShmemSegAddr);
+
+ UsedShmemSegAddr = hdr; /* probably redundant */
+}
+
+#endif /* EXEC_BACKEND */
+
/*
* PGSharedMemoryDetach
*
diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c
index 2e19693b2d..d1ddb1b0d7 100644
--- a/src/backend/postmaster/postmaster.c
+++ b/src/backend/postmaster/postmaster.c
@@ -37,7 +37,7 @@
*
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/postmaster/postmaster.c,v 1.440 2004/11/17 08:30:09 neilc Exp $
+ * $PostgreSQL: pgsql/src/backend/postmaster/postmaster.c,v 1.441 2004/12/29 21:36:03 tgl Exp $
*
* NOTES
*
@@ -1584,10 +1584,8 @@ processCancelRequest(Port *port, void *pkt)
int backendPID;
long cancelAuthCode;
Backend *bp;
-
#ifndef EXEC_BACKEND
Dlelem *curr;
-
#else
int i;
#endif
@@ -3152,10 +3150,31 @@ SubPostmasterMain(int argc, char *argv[])
MyProcPid = getpid(); /* reset MyProcPid */
- /* Read in file-based context */
+ /* In EXEC_BACKEND case we will not have inherited these settings */
+ IsPostmasterEnvironment = true;
+ whereToSendOutput = None;
+
+ /* Setup essential subsystems (to ensure elog() behaves sanely) */
+ MemoryContextInit();
+ InitializeGUCOptions();
+
+ /* Read in the variables file */
memset(&port, 0, sizeof(Port));
read_backend_variables(argv[2], &port);
+ /* Check we got appropriate args */
+ if (argc < 3)
+ elog(FATAL, "invalid subpostmaster invocation");
+
+ /*
+ * If appropriate, physically re-attach to shared memory segment.
+ * We want to do this before going any further to ensure that we
+ * can attach at the same address the postmaster used.
+ */
+ if (strcmp(argv[1], "-forkbackend") == 0 ||
+ strcmp(argv[1], "-forkboot") == 0)
+ PGSharedMemoryReAttach();
+
/*
* Start our win32 signal implementation. This has to be done
* after we read the backend variables, because we need to pick
@@ -3166,19 +3185,9 @@ SubPostmasterMain(int argc, char *argv[])
#endif
/* In EXEC_BACKEND case we will not have inherited these settings */
- IsPostmasterEnvironment = true;
- whereToSendOutput = None;
pqinitmask();
PG_SETMASK(&BlockSig);
- /* Setup essential subsystems */
- MemoryContextInit();
- InitializeGUCOptions();
-
- /* Check we got appropriate args */
- if (argc < 3)
- elog(FATAL, "invalid subpostmaster invocation");
-
/* Read in remaining GUC variables */
read_nondefault_variables();
@@ -3187,7 +3196,7 @@ SubPostmasterMain(int argc, char *argv[])
{
/* BackendRun will close sockets */
- /* Attach process to shared segments */
+ /* Attach process to shared data structures */
CreateSharedMemoryAndSemaphores(false, MaxBackends, 0);
#ifdef USE_SSL
@@ -3208,7 +3217,7 @@ SubPostmasterMain(int argc, char *argv[])
/* Close the postmaster's sockets */
ClosePostmasterPorts(false);
- /* Attach process to shared segments */
+ /* Attach process to shared data structures */
CreateSharedMemoryAndSemaphores(false, MaxBackends, 0);
BootstrapMain(argc - 2, argv + 2);
@@ -3259,6 +3268,7 @@ SubPostmasterMain(int argc, char *argv[])
return 1; /* shouldn't get here */
}
+
#endif /* EXEC_BACKEND */
@@ -3767,10 +3777,11 @@ read_inheritable_socket(SOCKET *dest, InheritableSocket *src)
static void
read_backend_variables(char *id, Port *port)
{
+ BackendParameters param;
+
#ifndef WIN32
/* Non-win32 implementation reads from file */
FILE *fp;
- BackendParameters param;
/* Open file */
fp = AllocateFile(id, PG_BINARY_R);
@@ -3796,25 +3807,23 @@ read_backend_variables(char *id, Port *port)
id, strerror(errno));
exit(1);
}
-
- restore_backend_variables(¶m, port);
#else
/* Win32 version uses mapped file */
HANDLE paramHandle;
- BackendParameters *param;
+ BackendParameters *paramp;
paramHandle = (HANDLE)atol(id);
- param = MapViewOfFile(paramHandle, FILE_MAP_READ, 0, 0, 0);
- if (!param)
+ paramp = MapViewOfFile(paramHandle, FILE_MAP_READ, 0, 0, 0);
+ if (!paramp)
{
write_stderr("could not map view of backend variables: error code %d\n",
(int) GetLastError());
exit(1);
}
- restore_backend_variables(param, port);
+ memcpy(¶m, paramp, sizeof(BackendParameters));
- if (!UnmapViewOfFile(param))
+ if (!UnmapViewOfFile(paramp))
{
write_stderr("could not unmap view of backend variables: error code %d\n",
(int) GetLastError());
@@ -3828,6 +3837,8 @@ read_backend_variables(char *id, Port *port)
exit(1);
}
#endif
+
+ restore_backend_variables(¶m, port);
}
/* Restore critical backend variables from the BackendParameters struct */
@@ -3930,6 +3941,7 @@ ShmemBackendArrayRemove(pid_t pid)
(errmsg_internal("could not find backend entry with pid %d",
(int) pid)));
}
+
#endif /* EXEC_BACKEND */
diff --git a/src/backend/storage/file/fd.c b/src/backend/storage/file/fd.c
index 6ebdaa6f56..3a1afd63a7 100644
--- a/src/backend/storage/file/fd.c
+++ b/src/backend/storage/file/fd.c
@@ -7,7 +7,7 @@
* Portions Copyright (c) 1994, Regents of the University of California
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/storage/file/fd.c,v 1.113 2004/09/16 16:58:32 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/storage/file/fd.c,v 1.114 2004/12/29 21:36:05 tgl Exp $
*
* NOTES:
*
@@ -228,6 +228,7 @@ static File fileNameOpenFile(FileName fileName, int fileFlags, int fileMode);
static char *filepath(const char *filename);
static void AtProcExit_Files(int code, Datum arg);
static void CleanupTempFiles(bool isProcExit);
+static void RemovePgTempFilesInDir(const char *tmpdirname);
/*
@@ -1482,59 +1483,83 @@ RemovePgTempFiles(void)
{
char db_path[MAXPGPATH];
char temp_path[MAXPGPATH];
- char rm_path[MAXPGPATH];
DIR *db_dir;
- DIR *temp_dir;
struct dirent *db_de;
- struct dirent *temp_de;
/*
- * Cycle through pg_tempfiles for all databases and remove old temp
- * files.
+ * Cycle through pgsql_tmp directories for all databases and remove old
+ * temp files.
*/
snprintf(db_path, sizeof(db_path), "%s/base", DataDir);
- if ((db_dir = AllocateDir(db_path)) != NULL)
+ db_dir = AllocateDir(db_path);
+ if (db_dir == NULL)
{
- while ((db_de = readdir(db_dir)) != NULL)
- {
- if (strcmp(db_de->d_name, ".") == 0
-#ifndef EXEC_BACKEND
- /* no PG_TEMP_FILES_DIR in DataDir in non EXEC_BACKEND case */
- || strcmp(db_de->d_name, "..") == 0
+ /* this really should not happen */
+ elog(LOG, "could not open directory \"%s\": %m", db_path);
+ return;
+ }
+
+ while ((db_de = readdir(db_dir)) != NULL)
+ {
+ if (strcmp(db_de->d_name, ".") == 0 ||
+ strcmp(db_de->d_name, "..") == 0)
+ continue;
+
+ snprintf(temp_path, sizeof(temp_path), "%s/%s/%s",
+ db_path, db_de->d_name, PG_TEMP_FILES_DIR);
+ RemovePgTempFilesInDir(temp_path);
+ }
+
+ FreeDir(db_dir);
+
+ /*
+ * In EXEC_BACKEND case there is a pgsql_tmp directory at the top
+ * level of DataDir as well.
+ */
+#ifdef EXEC_BACKEND
+ snprintf(temp_path, sizeof(temp_path), "%s/%s",
+ DataDir, PG_TEMP_FILES_DIR);
+ RemovePgTempFilesInDir(temp_path);
#endif
- )
- continue;
-
- snprintf(temp_path, sizeof(temp_path),
- "%s/%s/%s",
- db_path, db_de->d_name,
- PG_TEMP_FILES_DIR);
- if ((temp_dir = AllocateDir(temp_path)) != NULL)
- {
- while ((temp_de = readdir(temp_dir)) != NULL)
- {
- if (strcmp(temp_de->d_name, ".") == 0 ||
- strcmp(temp_de->d_name, "..") == 0)
- continue;
-
- snprintf(rm_path, sizeof(temp_path),
- "%s/%s/%s/%s",
- db_path, db_de->d_name,
- PG_TEMP_FILES_DIR,
- temp_de->d_name);
-
- if (strncmp(temp_de->d_name,
- PG_TEMP_FILE_PREFIX,
- strlen(PG_TEMP_FILE_PREFIX)) == 0)
- unlink(rm_path);
- else
- elog(LOG,
- "unexpected file found in temporary-files directory: \"%s\"",
- rm_path);
- }
- FreeDir(temp_dir);
- }
- }
- FreeDir(db_dir);
+}
+
+/* Process one pgsql_tmp directory for RemovePgTempFiles */
+static void
+RemovePgTempFilesInDir(const char *tmpdirname)
+{
+ DIR *temp_dir;
+ struct dirent *temp_de;
+ char rm_path[MAXPGPATH];
+
+ temp_dir = AllocateDir(tmpdirname);
+ if (temp_dir == NULL)
+ {
+ /* anything except ENOENT is fishy */
+ if (errno != ENOENT)
+ elog(LOG,
+ "could not open temporary-files directory \"%s\": %m",
+ tmpdirname);
+ return;
+ }
+
+ while ((temp_de = readdir(temp_dir)) != NULL)
+ {
+ if (strcmp(temp_de->d_name, ".") == 0 ||
+ strcmp(temp_de->d_name, "..") == 0)
+ continue;
+
+ snprintf(rm_path, sizeof(rm_path), "%s/%s",
+ tmpdirname, temp_de->d_name);
+
+ if (strncmp(temp_de->d_name,
+ PG_TEMP_FILE_PREFIX,
+ strlen(PG_TEMP_FILE_PREFIX)) == 0)
+ unlink(rm_path); /* note we ignore any error */
+ else
+ elog(LOG,
+ "unexpected file found in temporary-files directory: \"%s\"",
+ rm_path);
}
+
+ FreeDir(temp_dir);
}
diff --git a/src/backend/storage/ipc/ipci.c b/src/backend/storage/ipc/ipci.c
index 1a11508adf..524a3598c2 100644
--- a/src/backend/storage/ipc/ipci.c
+++ b/src/backend/storage/ipc/ipci.c
@@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
- * $PostgreSQL: pgsql/src/backend/storage/ipc/ipci.c,v 1.72 2004/09/29 15:15:55 tgl Exp $
+ * $PostgreSQL: pgsql/src/backend/storage/ipc/ipci.c,v 1.73 2004/12/29 21:36:06 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -19,6 +19,7 @@
#include "access/xlog.h"
#include "miscadmin.h"
#include "postmaster/bgwriter.h"
+#include "postmaster/postmaster.h"
#include "storage/bufmgr.h"
#include "storage/freespace.h"
#include "storage/ipc.h"
@@ -37,14 +38,15 @@
* Creates and initializes shared memory and semaphores.
*
* This is called by the postmaster or by a standalone backend.
- * It is also called by a backend forked from the postmaster under
- * the EXEC_BACKEND case. (In the non EXEC_BACKEND case, backends
- * start life already attached to shared memory.) The initialization
- * functions are set up to simply "attach" to pre-existing shared memory
- * structures in the latter case. We have to do that in order to
- * initialize pointers in local memory that reference the shared structures.
- * (In the non EXEC_BACKEND case, these pointer values are inherited via
- * fork() from the postmaster.)
+ * It is also called by a backend forked from the postmaster in the
+ * EXEC_BACKEND case. In the latter case, the shared memory segment
+ * already exists and has been physically attached to, but we have to
+ * initialize pointers in local memory that reference the shared structures,
+ * because we didn't inherit the correct pointer values from the postmaster
+ * as we do in the fork() scenario. The easiest way to do that is to run
+ * through the same code as before. (Note that the called routines mostly
+ * check IsUnderPostmaster, rather than EXEC_BACKEND, to detect this case.
+ * This is a bit code-wasteful and could be cleaned up.)
*
* If "makePrivate" is true then we only need private memory, not shared
* memory. This is true for a standalone backend, false for a postmaster.
@@ -101,14 +103,16 @@ CreateSharedMemoryAndSemaphores(bool makePrivate,
else
{
/*
- * Attach to the shmem segment. (this should only ever be reached
- * by EXEC_BACKEND code, and only then with makePrivate == false)
+ * We are reattaching to an existing shared memory segment.
+ * This should only be reached in the EXEC_BACKEND case, and
+ * even then only with makePrivate == false.
*/
#ifdef EXEC_BACKEND
Assert(!makePrivate);
- seghdr = PGSharedMemoryCreate(-1, makePrivate, 0);
+ Assert(UsedShmemSegAddr != NULL);
+ seghdr = UsedShmemSegAddr;
#else
- Assert(false);
+ elog(PANIC, "should be attached to shared memory already");
#endif
}
diff --git a/src/include/postmaster/postmaster.h b/src/include/postmaster/postmaster.h
index 230b58ad0f..ce555f0aca 100644
--- a/src/include/postmaster/postmaster.h
+++ b/src/include/postmaster/postmaster.h
@@ -6,7 +6,7 @@
* Portions Copyright (c) 1996-2004, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
- * $PostgreSQL: pgsql/src/include/postmaster/postmaster.h,v 1.7 2004/08/29 05:06:58 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/postmaster/postmaster.h,v 1.8 2004/12/29 21:36:08 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -41,6 +41,9 @@ extern void ClosePostmasterPorts(bool am_syslogger);
#ifdef EXEC_BACKEND
extern pid_t postmaster_forkexec(int argc, char *argv[]);
extern int SubPostmasterMain(int argc, char *argv[]);
+
+extern size_t ShmemBackendArraySize(void);
+extern void ShmemBackendArrayAllocation(void);
#endif
#endif /* _POSTMASTER_H */
diff --git a/src/include/storage/ipc.h b/src/include/storage/ipc.h
index 1920dd688e..0f623da7f8 100644
--- a/src/include/storage/ipc.h
+++ b/src/include/storage/ipc.h
@@ -11,7 +11,7 @@
* Portions Copyright (c) 1996-2004, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
- * $PostgreSQL: pgsql/src/include/storage/ipc.h,v 1.68 2004/08/29 05:06:58 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/storage/ipc.h,v 1.69 2004/12/29 21:36:09 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -33,10 +33,4 @@ extern void CreateSharedMemoryAndSemaphores(bool makePrivate,
int maxBackends,
int port);
-#ifdef EXEC_BACKEND
-/* postmaster.c */
-extern size_t ShmemBackendArraySize(void);
-extern void ShmemBackendArrayAllocation(void);
-#endif
-
#endif /* IPC_H */
diff --git a/src/include/storage/pg_shmem.h b/src/include/storage/pg_shmem.h
index 8b13ece9fe..c3692b7ee0 100644
--- a/src/include/storage/pg_shmem.h
+++ b/src/include/storage/pg_shmem.h
@@ -17,7 +17,7 @@
* Portions Copyright (c) 1996-2004, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
- * $PostgreSQL: pgsql/src/include/storage/pg_shmem.h,v 1.12 2004/11/09 21:30:18 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/storage/pg_shmem.h,v 1.13 2004/12/29 21:36:09 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@@ -41,6 +41,8 @@ typedef struct PGShmemHeader /* standard header for all Postgres shmem */
#ifdef EXEC_BACKEND
extern unsigned long UsedShmemSegID;
extern void *UsedShmemSegAddr;
+
+extern void PGSharedMemoryReAttach(void);
#endif
extern PGShmemHeader *PGSharedMemoryCreate(uint32 size, bool makePrivate,
--
2.49.0