Re: Global variables in exec() - Mailing list pgsql-patches

Applied.

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

Bruce Momjian wrote:
> Tom Lane wrote:
> > Bruce Momjian <pgman@candle.pha.pa.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@candle.pha.pa.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@postgresql.org)

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