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 1fcbb54d13aa5630c6128f130860cd14746596a4..a54c4138233de19fefd92029e73d64f4fde14542 100644 (file)
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -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 f178fe31d3c0102fa9aa7d6782bc21cc45fa76f6..34a2406a9a4e9ab2943d94948c215c73e0571e6e 100644 (file)
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -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 fa964370a9968530b443734c2bfb481f3d22f93f..456560ba3bc40b17de762560b0f0a5a18d861d23 100644 (file)
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -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 a9f5501143a1108403bc54caec91787e3c1c522e..8abcc2f26496e70104a2008dc15be41f4ea7d5a1 100644 (file)
--- a/src/include/pgstat.h
+++ b/src/include/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