A GUC variable to replace PGBE_ACTIVITY_SIZE - Mailing list pgsql-patches

Attached is a patch providing a new GUC variable called
"track_activity_query_size", as previously discussed on pgsql-hackers here:

http://archives.postgresql.org/pgsql-hackers/2008-06/msg00814.php

This is my first patch for postgres, so I'm sure it may need some love.
In particular:

* Should it be possible to set this new variable via a command-line
option ala shared_buffers?
* I'm not exactly sure about the best way to add unit/regr tests for
this change.
* I'm also not sure what's required in the way of updates to the
documentation.

Any comments or suggestions would be most appreciated.

Cheers,
T
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index f178fe3..912c1cf 100644
--- 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 = PGBE_DEFAULT_ACTIVITY_SIZE;

 /*
  * 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;


 /*
@@ -2025,7 +2027,26 @@ BackendStatusShmemSize(void)
 }

 /*
- * Initialize the shared status array during postmaster startup.
+ * Ensures that every element of BackendStatusArray has a valid st_activity
+ * pointer.
+ */
+static void
+pgstat_initialize_activity_pointers(void)
+{
+    Size i;
+    PgBackendStatus* beentry = BackendStatusArray;
+    char*             buffer = BackendActivityBuffer;
+
+    for (i = 0; i < MaxBackends; i++) {
+        beentry->st_activity = buffer;
+        buffer += pgstat_track_activity_query_size;
+        beentry++;
+    }
+}
+
+/*
+ * Initialize the shared status array & activity buffer during postmaster
+ * startup.
  */
 void
 CreateSharedBackendStatus(void)
@@ -2044,6 +2065,17 @@ CreateSharedBackendStatus(void)
          */
         MemSet(BackendStatusArray, 0, size);
     }
+
+    size = mul_size(pgstat_track_activity_query_size, MaxBackends);
+    BackendActivityBuffer = (char*)
+        ShmemInitStruct("Backend Activity Buffer", size, &found);
+
+    if (!found)
+    {
+        MemSet(BackendActivityBuffer, 0, size);
+    }
+
+    pgstat_initialize_activity_pointers();
 }


@@ -2128,7 +2160,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 +2220,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 +2299,7 @@ pgstat_read_current_status(void)
     volatile PgBackendStatus *beentry;
     PgBackendStatus *localtable;
     PgBackendStatus *localentry;
+    char            *localactivity;
     int            i;

     Assert(!pgStatRunningInCollector);
@@ -2278,6 +2311,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;
@@ -2296,10 +2332,14 @@ 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?
+             * XXX if pgstat_track_activity_query_size is really large,
+             * it might be best to use strcpy not memcpy for copying the
+             * activity string?
              */
             memcpy(localentry, (char *) beentry, sizeof(PgBackendStatus));
+            memcpy(localactivity, (char *) beentry->st_activity,
+                    pgstat_track_activity_query_size);
+            localentry->st_activity = localactivity;

             if (save_changecount == beentry->st_changecount &&
                 (save_changecount & 1) == 0)
@@ -2314,9 +2354,10 @@ pgstat_read_current_status(void)
         if (localentry->st_procpid > 0)
         {
             localentry++;
+            localactivity += pgstat_track_activity_query_size;
             localNumBackends++;
         }
-    }
+     }

     /* Set the pointer only after completion of a valid table */
     localBackendStatusTable = localtable;
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index fa96437..b7f1302 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -1311,6 +1311,15 @@ static struct config_int ConfigureNamesInt[] =
     },

     {
+        {"track_activity_query_size", PGC_POSTMASTER, RESOURCES_MEM,
+            gettext_noop("Sets the maximum number of characters that will be displayed for
pg_stat_activity.current_query."),
+            NULL,
+        },
+        &pgstat_track_activity_query_size,
+        PGBE_DEFAULT_ACTIVITY_SIZE, 100, 102400, NULL, NULL
+    },
+
+    {
         {"temp_buffers", PGC_USERSET, RESOURCES_MEM,
             gettext_noop("Sets the maximum number of temporary buffers used by each session."),
             NULL,
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index a9f5501..03b9465 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -509,8 +509,8 @@ typedef struct PgStat_GlobalStats
  * ----------
  */

-/* Max length of st_activity string ... perhaps replace with a GUC var? */
-#define PGBE_ACTIVITY_SIZE    1024
+/* Default length of st_activity string (see backend_activity_size GUC) */
+#define PGBE_DEFAULT_ACTIVITY_SIZE    1024

 /* ----------
  * PgBackendStatus
@@ -551,7 +551,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 +578,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