Turn PGBE_ACTIVITY_SIZE into a GUC variable, track_activity_query_size.

As the buffer could now be a lot larger than before, and copying it could
thus be a lot more expensive than before, use strcpy instead of memcpy to
copy the query string, as was already suggested in comments. Also, only copy
the PgBackendStatus struct and string if the slot is in use.

Patch by Thomas Lee, with some changes by me.

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index f25ed24986c2bfea495a0d4f04f69657249a960a..97cf94526171bbb69b8ad4af23fdd7576cf3604d 100644 (file)
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -1,4 +1,4 @@
-<!-- $PostgreSQL: pgsql/doc/src/sgml/config.sgml,v 1.180 2008/06/14 21:59:59 alvherre Exp $ -->
+<!-- $PostgreSQL: pgsql/doc/src/sgml/config.sgml,v 1.181 2008/06/30 10:58:47 heikki Exp $ -->
 
 <chapter Id="runtime-config">
   <title>Server Configuration</title>
@@ -3331,6 +3331,22 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
       </listitem>
      </varlistentry>
 
+     <varlistentry id="guc-track-activity-query-size" xreflabel="track_activity_query_size">
+      <term><varname>track_activity_query_size</varname> (<type>integer</type>)</term>
+      <indexterm>
+       <primary><varname>track_activity_query_size</> configuration parameter</primary>
+      </indexterm>
+      <listitem>
+       <para>
+       Specifies the number of bytes reserved to track the currently
+       executing command for each active session, for the
+       <structname>pg_stat_activity</>.<structfield>current_query</> field.
+       The default value is 1024. This parameter can only be set at server
+       start.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="guc-track-counts" xreflabel="track_counts">
       <term><varname>track_counts</varname> (<type>boolean</type>)</term>
       <indexterm>

diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index 5d9dcd6c94ce561e88a5e17a80bb69541d9f5099..e3cb0be5bb787dca5d2df77dc32004808328c1f1 100644 (file)
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -13,7 +13,7 @@
  *
  *     Copyright (c) 2001-2008, PostgreSQL Global Development Group
  *
- *     $PostgreSQL: pgsql/src/backend/postmaster/pgstat.c,v 1.175 2008/06/19 00:46:05 alvherre Exp $
+ *     $PostgreSQL: pgsql/src/backend/postmaster/pgstat.c,v 1.176 2008/06/30 10:58:47 heikki Exp $
  * ----------
  */
 #include "postgres.h"
@@ -101,6 +101,7 @@
 bool           pgstat_track_activities = false;
 bool           pgstat_track_counts = false;
 int                    pgstat_track_functions = TRACK_FUNC_OFF;
+int                    pgstat_track_activity_query_size = 1024;
 
 /*
  * BgWriter global statistics counters (unused in other processes).
@@ -2010,6 +2011,7 @@ pgstat_fetch_global(void)
 
 static PgBackendStatus *BackendStatusArray = NULL;
 static PgBackendStatus *MyBEEntry = NULL;
+static char                       *BackendActivityBuffer = NULL;
 
 
 /*
@@ -2020,20 +2022,25 @@ BackendStatusShmemSize(void)
 {
        Size            size;
 
-       size = mul_size(sizeof(PgBackendStatus), MaxBackends);
+       size = add_size(mul_size(sizeof(PgBackendStatus), MaxBackends),
+                                       mul_size(pgstat_track_activity_query_size, MaxBackends));
        return size;
 }
 
 /*
- * Initialize the shared status array during postmaster startup.
+ * Initialize the shared status array and activity string buffer during
+ * postmaster startup.
  */
 void
 CreateSharedBackendStatus(void)
 {
-       Size            size = BackendStatusShmemSize();
+       Size            size;
        bool            found;
+       int                     i;
+       char       *buffer;
 
        /* Create or attach to the shared array */
+       size = mul_size(sizeof(PgBackendStatus), MaxBackends);
        BackendStatusArray = (PgBackendStatus *)
                ShmemInitStruct("Backend Status Array", size, &found);
 
@@ -2044,6 +2051,23 @@ CreateSharedBackendStatus(void)
                 */
                MemSet(BackendStatusArray, 0, size);
        }
+
+       /* Create or attach to the shared activity buffer */
+       size = mul_size(pgstat_track_activity_query_size, MaxBackends);
+       BackendActivityBuffer = (char*)
+               ShmemInitStruct("Backend Activity Buffer", size, &found);
+
+       if (!found)
+       {
+               MemSet(BackendActivityBuffer, 0, size);
+
+               /* Initialize st_activity pointers. */
+               buffer = BackendActivityBuffer;
+               for (i = 0; i < MaxBackends; i++) {
+                       BackendStatusArray[i].st_activity = buffer;
+                       buffer += pgstat_track_activity_query_size;
+               }
+       }
 }
 
 
@@ -2128,7 +2152,7 @@ pgstat_bestart(void)
        beentry->st_waiting = false;
        beentry->st_activity[0] = '\0';
        /* Also make sure the last byte in the string area is always 0 */
-       beentry->st_activity[PGBE_ACTIVITY_SIZE - 1] = '\0';
+       beentry->st_activity[pgstat_track_activity_query_size - 1] = '\0';
 
        beentry->st_changecount++;
        Assert((beentry->st_changecount & 1) == 0);
@@ -2188,7 +2212,7 @@ pgstat_report_activity(const char *cmd_str)
        start_timestamp = GetCurrentStatementStartTimestamp();
 
        len = strlen(cmd_str);
-       len = pg_mbcliplen(cmd_str, len, PGBE_ACTIVITY_SIZE - 1);
+       len = pg_mbcliplen(cmd_str, len, pgstat_track_activity_query_size - 1);
 
        /*
         * Update my status entry, following the protocol of bumping
@@ -2267,6 +2291,7 @@ pgstat_read_current_status(void)
        volatile PgBackendStatus *beentry;
        PgBackendStatus *localtable;
        PgBackendStatus *localentry;
+       char                    *localactivity;
        int                     i;
 
        Assert(!pgStatRunningInCollector);
@@ -2278,6 +2303,9 @@ pgstat_read_current_status(void)
        localtable = (PgBackendStatus *)
                MemoryContextAlloc(pgStatLocalContext,
                                                   sizeof(PgBackendStatus) * MaxBackends);
+       localactivity = (char *)
+               MemoryContextAlloc(pgStatLocalContext,
+                                                  pgstat_track_activity_query_size * MaxBackends);
        localNumBackends = 0;
 
        beentry = BackendStatusArray;
@@ -2295,11 +2323,17 @@ pgstat_read_current_status(void)
                {
                        int                     save_changecount = beentry->st_changecount;
 
-                       /*
-                        * XXX if PGBE_ACTIVITY_SIZE is really large, it might be best to
-                        * use strcpy not memcpy for copying the activity string?
-                        */
-                       memcpy(localentry, (char *) beentry, sizeof(PgBackendStatus));
+                       localentry->st_procpid = beentry->st_procpid;
+                       if (localentry->st_procpid > 0)
+                       {
+                               memcpy(localentry, (char *) beentry, sizeof(PgBackendStatus));
+                               /*
+                                * strcpy is safe even if the string is modified concurrently,
+                                * because there's always a \0 at the end of the buffer.
+                                */
+                               strcpy(localactivity, (char *) beentry->st_activity);
+                               localentry->st_activity = localactivity;
+                       }
 
                        if (save_changecount == beentry->st_changecount &&
                                (save_changecount & 1) == 0)
@@ -2314,6 +2348,7 @@ pgstat_read_current_status(void)
                if (localentry->st_procpid > 0)
                {
                        localentry++;
+                       localactivity += pgstat_track_activity_query_size;
                        localNumBackends++;
                }
        }

diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index b3409111b11bf69cedd9af5ed4038e7be42708ed..76c6843fd6cb8c5ae89aa2acfdb6b9df264687ba 100644 (file)
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -10,7 +10,7 @@
  * Written by Peter Eisentraut <peter_e@gmx.net>.
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.456 2008/05/28 09:04:06 mha Exp $
+ *       $PostgreSQL: pgsql/src/backend/utils/misc/guc.c,v 1.457 2008/06/30 10:58:47 heikki Exp $
  *
  *--------------------------------------------------------------------
  */
@@ -1848,6 +1848,15 @@ static struct config_int ConfigureNamesInt[] =
                -1, -1, INT_MAX, NULL, NULL
        },
 
+       {
+               {"track_activity_query_size", PGC_POSTMASTER, RESOURCES_MEM,
+                       gettext_noop("Sets the size reserved for pg_stat_activity.current_query, in bytes."),
+                       NULL,
+               },
+               &pgstat_track_activity_query_size,
+               1024, 100, 102400, NULL, NULL
+       },
+
        /* End-of-list marker */
        {
                {NULL, 0, 0, NULL, NULL}, NULL, 0, 0, 0, NULL, NULL

diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 033a74b2d4ec03b4e67a205098c5a88c73d98230..552d856ae29e3dbf8b4e9272073fb9b3cc4895e9 100644 (file)
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -364,6 +364,7 @@
 #track_activities = on
 #track_counts = on
 #track_functions = none                        # none, pl, all
+#track_activity_query_size = 1024
 #update_process_title = on
 
 

diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index 9b48f42bb0f7468dff9ae9523271274a214f0fd5..fabc5fb4a07cc61ebfa9544c60186645e10d123c 100644 (file)
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -5,7 +5,7 @@
  *
  *     Copyright (c) 2001-2008, PostgreSQL Global Development Group
  *
- *     $PostgreSQL: pgsql/src/include/pgstat.h,v 1.76 2008/06/19 00:46:05 alvherre Exp $
+ *     $PostgreSQL: pgsql/src/include/pgstat.h,v 1.77 2008/06/30 10:58:47 heikki Exp $
  * ----------
  */
 #ifndef PGSTAT_H
@@ -509,9 +509,6 @@ typedef struct PgStat_GlobalStats
  * ----------
  */
 
-/* Max length of st_activity string ... perhaps replace with a GUC var? */
-#define PGBE_ACTIVITY_SIZE     1024
-
 /* ----------
  * PgBackendStatus
  *
@@ -551,7 +548,7 @@ typedef struct PgBackendStatus
        bool            st_waiting;
 
        /* current command string; MUST be null-terminated */
-       char            st_activity[PGBE_ACTIVITY_SIZE];
+       char       *st_activity;
 } PgBackendStatus;
 
 /*
@@ -578,6 +575,7 @@ typedef struct PgStat_FunctionCallUsage
 extern bool pgstat_track_activities;
 extern bool pgstat_track_counts;
 extern int     pgstat_track_functions;
+extern int     pgstat_track_activity_query_size;
 
 /*
  * BgWriter statistics counters are updated directly by bgwriter and bufmgr