]> granicus.if.org Git - postgresql/commitdiff
Refactor EXEC_BACKEND code so that postmaster child processes reattach
authorTom Lane <tgl@sss.pgh.pa.us>
Wed, 29 Dec 2004 21:36:09 +0000 (21:36 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Wed, 29 Dec 2004 21:36:09 +0000 (21:36 +0000)
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
src/backend/postmaster/postmaster.c
src/backend/storage/file/fd.c
src/backend/storage/ipc/ipci.c
src/include/postmaster/postmaster.h
src/include/storage/ipc.h
src/include/storage/pg_shmem.h

index 3c3f928ca2d72da58586ff93013db5ce008b3aaf..63ff4ad57ef8ea0a5a5b195328c35e021fcb538a 100644 (file)
@@ -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
  *
index 2e19693b2d463ffb36e554c243055523ad6579fa..d1ddb1b0d72c1ab101cf5eec7cd78b5ccaa5137e 100644 (file)
@@ -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(&param, 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(&param, 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(&param, 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 */
 
 
index 6ebdaa6f567e841dcd67d7cd999f8e84811e704b..3a1afd63a70ac7c5ea4b2a39a58ac50a01dcaeed 100644 (file)
@@ -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);
 }
index 1a11508adf18edf17f8070d53cc3126b2651a92c..524a3598c258971aa2cf76bda915a38db4c54ca1 100644 (file)
@@ -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"
  *             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
        }
 
index 230b58ad0f8668c6e194ecb7278b04c704a356a0..ce555f0aca2cefc1f9a77f6369e27d256ca936f6 100644 (file)
@@ -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 */
index 1920dd688e3ebba45771bd6e3a1b4b397ee6ae1f..0f623da7f8e940f562424086fbc251c4fe864965 100644 (file)
@@ -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 */
index 8b13ece9fe2c2d936bf241f008b3109937c4840e..c3692b7ee0df36370fabf7c130ca8328c9db43f0 100644 (file)
@@ -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,