Sunday, June 22, 2008

[PATCHES] A GUC variable to replace PGBE_ACTIVITY_SIZE

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

No comments: