Rethink prior patch to filter out dead backend entries from the pgstats
authorTom Lane
Fri, 16 Dec 2005 04:03:40 +0000 (04:03 +0000)
committerTom Lane
Fri, 16 Dec 2005 04:03:40 +0000 (04:03 +0000)
file.  The original code probed the PGPROC array separately for each PID,
which was not good for large numbers of backends: not only is the runtime
O(N^2) but most of it is spent holding ProcArrayLock.  Instead, take the
lock just once and copy the active PIDs into an array, then use qsort
and bsearch so that the lookup time is more like O(N log N).

src/backend/postmaster/pgstat.c
src/backend/storage/ipc/procarray.c
src/include/storage/procarray.h

index 82cf1bc4ef1ec4453f36086ac878b7163e0df492..ae20a3bb710a14b550abcd82a97ec831a05508e2 100644 (file)
@@ -13,7 +13,7 @@
  *
  * Copyright (c) 2001-2005, PostgreSQL Global Development Group
  *
- * $PostgreSQL: pgsql/src/backend/postmaster/pgstat.c,v 1.112 2005/11/22 18:17:17 momjian Exp $
+ * $PostgreSQL: pgsql/src/backend/postmaster/pgstat.c,v 1.113 2005/12/16 04:03:40 tgl Exp $
  * ----------
  */
 #include "postgres.h"
@@ -2453,6 +2453,16 @@ pgstat_write_statsfile(void)
    }
 }
 
+/*
+ * qsort/bsearch comparison routine for PIDs
+ *
+ * We assume PIDs are nonnegative, so there's no overflow risk
+ */
+static int
+comparePids(const void *v1, const void *v2)
+{
+   return *((const int *) v1) - *((const int *) v2);
+}
 
 /* ----------
  * pgstat_read_statsfile() -
@@ -2478,7 +2488,7 @@ pgstat_read_statsfile(HTAB **dbhash, Oid onlydb,
    int         maxbackends = 0;
    int         havebackends = 0;
    bool        found;
-   bool        check_pids;
+   int        *live_pids;
    MemoryContext use_mcxt;
    int         mcxt_flags;
 
@@ -2497,13 +2507,17 @@ pgstat_read_statsfile(HTAB **dbhash, Oid onlydb,
    {
        use_mcxt = NULL;
        mcxt_flags = 0;
-       check_pids = false;
+       live_pids = NULL;
    }
    else
    {
        use_mcxt = TopTransactionContext;
        mcxt_flags = HASH_CONTEXT;
-       check_pids = true;
+       live_pids = GetAllBackendPids();
+       /* Sort the PID array so we can use bsearch */
+       if (live_pids[0] > 1)
+           qsort((void *) &live_pids[1], live_pids[0], sizeof(int),
+                 comparePids);
    }
 
    /*
@@ -2706,7 +2720,13 @@ pgstat_read_statsfile(HTAB **dbhash, Oid onlydb,
                /*
                 * If possible, check PID to verify still running
                 */
-               if (check_pids && !IsBackendPid(beentry->procpid))
+               if (live_pids &&
+                   (live_pids[0] == 0 ||
+                    bsearch((void *) &beentry->procpid,
+                            (void *) &live_pids[1],
+                            live_pids[0],
+                            sizeof(int),
+                            comparePids) == NULL))
                {
                    /*
                     * Note: we could send a BETERM message to tell the
index cafadeb90542d3167ac3c2669fb656ffdb54a19b..f6330807480457b010b50d3bf1ae0b4077265616 100644 (file)
@@ -23,7 +23,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/storage/ipc/procarray.c,v 1.9 2005/12/11 21:02:18 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/storage/ipc/procarray.c,v 1.10 2005/12/16 04:03:40 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -732,6 +732,42 @@ IsBackendPid(int pid)
    return (BackendPidGetProc(pid) != NULL);
 }
 
+/*
+ * GetAllBackendPids -- get an array of all current backends' PIDs
+ *
+ * The result is a palloc'd array with the number of active backends in
+ * entry [0], their PIDs in entries [1] .. [n].  The caller must bear in
+ * mind that the result may already be obsolete when returned.
+ */
+int *
+GetAllBackendPids(void)
+{
+   int        *result;
+   int         npids;
+   ProcArrayStruct *arrayP = procArray;
+   int         index;
+
+   result = (int *) palloc((MaxBackends + 1) * sizeof(int));
+   npids = 0;
+
+   LWLockAcquire(ProcArrayLock, LW_SHARED);
+
+   for (index = 0; index < arrayP->numProcs; index++)
+   {
+       PGPROC     *proc = arrayP->procs[index];
+
+       if (proc->pid != 0)     /* ignore dummy procs */
+           result[++npids] = proc->pid;
+   }
+
+   LWLockRelease(ProcArrayLock);
+
+   Assert(npids <= MaxBackends);
+
+   result[0] = npids;
+   return result;
+}
+
 /*
  * CountActiveBackends --- count backends (other than myself) that are in
  *     active transactions.  This is used as a heuristic to decide if
index 68c615afc6e750a73e5abaff60ee899fc1ea172b..61441bbf49e22f7c52c5c41aa6c9fb3cda02b221 100644 (file)
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2005, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/storage/procarray.h,v 1.6 2005/10/15 02:49:46 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/storage/procarray.h,v 1.7 2005/12/16 04:03:40 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -29,6 +29,7 @@ extern TransactionId GetOldestXmin(bool allDbs);
 extern PGPROC *BackendPidGetProc(int pid);
 extern int BackendXidGetPid(TransactionId xid);
 extern bool IsBackendPid(int pid);
+extern int *GetAllBackendPids(void);
 extern bool DatabaseHasActiveBackends(Oid databaseId, bool ignoreMyself);
 
 extern int CountActiveBackends(void);