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

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);