Re: Global variables in exec()

From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL-patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: Global variables in exec()
Date: 2003-05-06 23:35:09
Message-ID: 200305062335.h46NZ9t08029@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-patches


Applied.

---------------------------------------------------------------------------

Bruce Momjian wrote:
> Tom Lane wrote:
> > Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us> writes:
> > > -
> > > - typedef uint32 IpcMemoryKey; /* shared memory key passed to shmget(2) */
> >
> > IpcMemoryKey is a SysV-specific typedef and has *no* business being
> > moved out of the sysv-specific port file. Once again I urge you to
> > consider making a Windows-specific shmem port file, instead of tromping
> > all over the Unix code in order to support an API that Windows doesn't
> > like in the first place :-(
>
> I don't have the time or experience to do a Win32-specific shared memory
> implementation. PeerDirect used a SysV emulation file, and that is what
> I am using. Also, I want Unix to support fork/exec so we can test any
> fork/exec problems in Unix, so I will need that parameter to be passed
> anyway for Unix.
>
> We only support one shared memory implementation right now, but if we
> add another, we can modify this code to abstract out that data type.
>
> All the ExecBackend stuff is marked so it will be easy to modify later.
>
> > > ! /* database name at the end because it might contain commas */
> > > ! sprintf(pbuf, "%d,%d,%s", port->sock, UsedShmemSegID, port->database_name);
> >
> > snprintf please. I don't think there's any guaranteed limit on the size
> > of port->database_name these days.
>
> OK, fixed to snprintf. I did define pbuf as:
>
> + char pbuf[NAMEDATALEN + 256];
>
> becuase I assume a database name can't be more than NAMEDATALEN. The
> 256 is for the other integers passed.
>
> > > + sscanf(optarg, "%d,%d,", &MyProcPort->sock, &UsedShmemSegID);
> > > + DBName = strdup(strrchr(optarg, ',') + 1);
> >
> > What happens when the dbname contains a comma?
>
> Oops, good catch. I was careful to put the database name at the end to
> handle such cases, but forgot it here.
>
> Here is a new patch that adds the shared memory param for bootstrap.
> PeerDirect only passed GUC and the two values of socket file descriptor
> and shared memory id on the command line, so I have all those covered
> now.
>
> At the same time, I modified bootstrap's -p parameter to take the
> database name as a parameter just like the ordinary backend -p does.
>
> --
> Bruce Momjian | http://candle.pha.pa.us
> pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
> + If your life is a hard drive, | 13 Roberts Road
> + Christ can be your backup. | Newtown Square, Pennsylvania 19073

> Index: src/backend/bootstrap/bootstrap.c
> ===================================================================
> RCS file: /cvsroot/pgsql-server/src/backend/bootstrap/bootstrap.c,v
> retrieving revision 1.154
> diff -c -c -r1.154 bootstrap.c
> *** src/backend/bootstrap/bootstrap.c 6 May 2003 05:15:45 -0000 1.154
> --- src/backend/bootstrap/bootstrap.c 6 May 2003 14:58:36 -0000
> ***************
> *** 36,41 ****
> --- 36,42 ----
> #include "miscadmin.h"
> #include "storage/freespace.h"
> #include "storage/ipc.h"
> + #include "storage/pg_shmem.h"
> #include "storage/proc.h"
> #include "tcop/tcopprot.h"
> #include "utils/builtins.h"
> ***************
> *** 252,258 ****
> * variable */
> }
>
> ! while ((flag = getopt(argc, argv, "B:d:D:Fo:px:")) != -1)
> {
> switch (flag)
> {
> --- 253,259 ----
> * variable */
> }
>
> ! while ((flag = getopt(argc, argv, "B:d:D:Fo:p:x:")) != -1)
> {
> switch (flag)
> {
> ***************
> *** 283,290 ****
> --- 284,302 ----
> xlogop = atoi(optarg);
> break;
> case 'p':
> + {
> /* indicates fork from postmaster */
> + char *p;
> + #ifdef EXEC_BACKEND
> + sscanf(optarg, "%d,", &UsedShmemSegID);
> + p = strchr(optarg, ',');
> + if (p)
> + dbname = strdup(p+1);
> + #else
> + dbname = strdup(optarg);
> + #endif
> break;
> + }
> case 'B':
> SetConfigOption("shared_buffers", optarg, PGC_POSTMASTER, PGC_S_ARGV);
> break;
> ***************
> *** 292,305 ****
> usage();
> break;
> }
> ! } /* while */
>
> ! if (argc - optind != 1)
> usage();
>
> - dbname = argv[optind];
> -
> - Assert(dbname);
>
> if (IsUnderPostmaster && ExecBackend && MyProc /* ordinary backend */)
> {
> --- 304,319 ----
> usage();
> break;
> }
> ! }
>
> ! if (!dbname && argc - optind == 1)
> ! {
> ! dbname = argv[optind];
> ! optind++;
> ! }
> ! if (!dbname || argc != optind)
> usage();
>
>
> if (IsUnderPostmaster && ExecBackend && MyProc /* ordinary backend */)
> {
> Index: src/backend/port/sysv_shmem.c
> ===================================================================
> RCS file: /cvsroot/pgsql-server/src/backend/port/sysv_shmem.c,v
> retrieving revision 1.7
> diff -c -c -r1.7 sysv_shmem.c
> *** src/backend/port/sysv_shmem.c 24 Apr 2003 21:24:36 -0000 1.7
> --- src/backend/port/sysv_shmem.c 6 May 2003 14:58:37 -0000
> ***************
> *** 34,46 ****
> #include "storage/ipc.h"
> #include "storage/pg_shmem.h"
>
> -
> - typedef uint32 IpcMemoryKey; /* shared memory key passed to shmget(2) */
> typedef int IpcMemoryId; /* shared memory ID returned by shmget(2) */
>
> #define IPCProtection (0600) /* access/modify by user only */
>
>
> static void *InternalIpcMemoryCreate(IpcMemoryKey memKey, uint32 size);
> static void IpcMemoryDetach(int status, Datum shmaddr);
> static void IpcMemoryDelete(int status, Datum shmId);
> --- 34,48 ----
> #include "storage/ipc.h"
> #include "storage/pg_shmem.h"
>
> typedef int IpcMemoryId; /* shared memory ID returned by shmget(2) */
>
> #define IPCProtection (0600) /* access/modify by user only */
>
>
> + #ifdef EXEC_BACKEND
> + IpcMemoryKey UsedShmemSegID = 0;
> + #endif
> +
> static void *InternalIpcMemoryCreate(IpcMemoryKey memKey, uint32 size);
> static void IpcMemoryDetach(int status, Datum shmaddr);
> static void IpcMemoryDelete(int status, Datum shmId);
> ***************
> *** 300,309 ****
> /* Room for a header? */
> Assert(size > MAXALIGN(sizeof(PGShmemHeader)));
>
> ! /* Loop till we find a free IPC key */
> ! NextShmemSegID = port * 1000;
>
> ! for (NextShmemSegID++;; NextShmemSegID++)
> {
> IpcMemoryId shmid;
>
> --- 302,315 ----
> /* Room for a header? */
> Assert(size > MAXALIGN(sizeof(PGShmemHeader)));
>
> ! #ifdef EXEC_BACKEND
> ! if (UsedShmemSegID != 0)
> ! NextShmemSegID = UsedShmemSegID;
> ! else
> ! #endif
> ! NextShmemSegID = port * 1000 + 1;
>
> ! for (;;NextShmemSegID++)
> {
> IpcMemoryId shmid;
>
> ***************
> *** 394,399 ****
> --- 400,410 ----
> */
> hdr->totalsize = size;
> hdr->freeoffset = MAXALIGN(sizeof(PGShmemHeader));
> +
> + #ifdef EXEC_BACKEND
> + if (!makePrivate && UsedShmemSegID == 0)
> + UsedShmemSegID = NextShmemSegID;
> + #endif
>
> return hdr;
> }
> Index: src/backend/postmaster/postmaster.c
> ===================================================================
> RCS file: /cvsroot/pgsql-server/src/backend/postmaster/postmaster.c,v
> retrieving revision 1.321
> diff -c -c -r1.321 postmaster.c
> *** src/backend/postmaster/postmaster.c 3 May 2003 05:13:18 -0000 1.321
> --- src/backend/postmaster/postmaster.c 6 May 2003 14:58:40 -0000
> ***************
> *** 97,102 ****
> --- 97,103 ----
> #include "nodes/nodes.h"
> #include "storage/fd.h"
> #include "storage/ipc.h"
> + #include "storage/pg_shmem.h"
> #include "storage/pmsignal.h"
> #include "storage/proc.h"
> #include "access/xlog.h"
> ***************
> *** 2214,2219 ****
> --- 2215,2223 ----
> int ac;
> char debugbuf[32];
> char protobuf[32];
> + #ifdef EXEC_BACKEND
> + char pbuf[NAMEDATALEN + 256];
> + #endif
> int i;
> int status;
> struct timeval now;
> ***************
> *** 2434,2441 ****
> * database to use. -p marks the end of secure switches.
> */
> av[ac++] = "-p";
> av[ac++] = port->database_name;
> !
> /*
> * Pass the (insecure) option switches from the connection request.
> * (It's OK to mangle port->cmdline_options now.)
> --- 2438,2451 ----
> * database to use. -p marks the end of secure switches.
> */
> av[ac++] = "-p";
> + #ifdef EXEC_BACKEND
> + Assert(UsedShmemSegID != 0);
> + /* database name at the end because it might contain commas */
> + snprintf(pbuf, NAMEDATALEN + 256, "%d,%d,%s", port->sock, UsedShmemSegID, port->database_name);
> + av[ac++] = pbuf;
> + #else
> av[ac++] = port->database_name;
> ! #endif
> /*
> * Pass the (insecure) option switches from the connection request.
> * (It's OK to mangle port->cmdline_options now.)
> ***************
> *** 2712,2717 ****
> --- 2722,2730 ----
> int ac = 0;
> char nbbuf[32];
> char xlbuf[32];
> + #ifdef EXEC_BACKEND
> + char pbuf[NAMEDATALEN + 256];
> + #endif
>
> #ifdef LINUX_PROFILE
> setitimer(ITIMER_PROF, &prof_itimer, NULL);
> ***************
> *** 2762,2768 ****
> --- 2775,2788 ----
> av[ac++] = xlbuf;
>
> av[ac++] = "-p";
> + #ifdef EXEC_BACKEND
> + Assert(UsedShmemSegID != 0);
> + /* database name at the end because it might contain commas */
> + snprintf(pbuf, NAMEDATALEN + 256, "%d,%s", UsedShmemSegID, "template1");
> + av[ac++] = pbuf;
> + #else
> av[ac++] = "template1";
> + #endif
>
> av[ac] = (char *) NULL;
>
> Index: src/backend/storage/ipc/shmem.c
> ===================================================================
> RCS file: /cvsroot/pgsql-server/src/backend/storage/ipc/shmem.c,v
> retrieving revision 1.67
> diff -c -c -r1.67 shmem.c
> *** src/backend/storage/ipc/shmem.c 4 Sep 2002 20:31:25 -0000 1.67
> --- src/backend/storage/ipc/shmem.c 6 May 2003 14:58:40 -0000
> ***************
> *** 365,372 ****
> hash_search(ShmemIndex, (void *) &item, HASH_REMOVE, NULL);
> LWLockRelease(ShmemIndexLock);
>
> ! elog(WARNING, "ShmemInitStruct: cannot allocate '%s'",
> ! name);
> *foundPtr = FALSE;
> return NULL;
> }
> --- 365,371 ----
> hash_search(ShmemIndex, (void *) &item, HASH_REMOVE, NULL);
> LWLockRelease(ShmemIndexLock);
>
> ! elog(WARNING, "ShmemInitStruct: cannot allocate '%s'", name);
> *foundPtr = FALSE;
> return NULL;
> }
> Index: src/backend/tcop/postgres.c
> ===================================================================
> RCS file: /cvsroot/pgsql-server/src/backend/tcop/postgres.c,v
> retrieving revision 1.335
> diff -c -c -r1.335 postgres.c
> *** src/backend/tcop/postgres.c 6 May 2003 05:15:45 -0000 1.335
> --- src/backend/tcop/postgres.c 6 May 2003 14:59:27 -0000
> ***************
> *** 51,56 ****
> --- 51,57 ----
> #include "rewrite/rewriteHandler.h"
> #include "storage/freespace.h"
> #include "storage/ipc.h"
> + #include "storage/pg_shmem.h"
> #include "storage/proc.h"
> #include "tcop/fastpath.h"
> #include "tcop/pquery.h"
> ***************
> *** 1987,1993 ****
> --- 1988,2005 ----
> */
> if (secure)
> {
> + char *p;
> + #ifdef EXEC_BACKEND
> + sscanf(optarg, "%d,%d,", &MyProcPort->sock, &UsedShmemSegID);
> + /* Grab dbname as last param */
> + p = strchr(optarg, ',');
> + if (p)
> + p = strchr(p+1, ',');
> + if (p)
> + dbname = strdup(p+1);
> + #else
> dbname = strdup(optarg);
> + #endif
> secure = false; /* subsequent switches are NOT
> * secure */
> ctx = PGC_BACKEND;
> Index: src/include/storage/pg_shmem.h
> ===================================================================
> RCS file: /cvsroot/pgsql-server/src/include/storage/pg_shmem.h,v
> retrieving revision 1.4
> diff -c -c -r1.4 pg_shmem.h
> *** src/include/storage/pg_shmem.h 4 Sep 2002 20:31:45 -0000 1.4
> --- src/include/storage/pg_shmem.h 6 May 2003 14:59:28 -0000
> ***************
> *** 24,29 ****
> --- 24,31 ----
> #ifndef PG_SHMEM_H
> #define PG_SHMEM_H
>
> + typedef uint32 IpcMemoryKey; /* shared memory key passed to shmget(2) */
> +
> typedef struct PGShmemHeader /* standard header for all Postgres shmem */
> {
> int32 magic; /* magic # to identify Postgres segments */
> ***************
> *** 33,38 ****
> --- 35,44 ----
> uint32 freeoffset; /* offset to first free space */
> } PGShmemHeader;
>
> +
> + #ifdef EXEC_BACKEND
> + extern IpcMemoryKey UsedShmemSegID;
> + #endif
>
> extern PGShmemHeader *PGSharedMemoryCreate(uint32 size, bool makePrivate,
> int port);

>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
> (send "unregister YourEmailAddressHere" to majordomo(at)postgresql(dot)org)

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

In response to

Browse pgsql-patches by date

  From Date Subject
Next Message Bruce Momjian 2003-05-07 03:51:32 Re: Disable alternate locations on Win32
Previous Message Bruce Momjian 2003-05-06 15:21:37 Re: Global variables in exec()