Avoid updating our PgBackendStatus entry when track_activities is off.

tglsfdc · tglsfdc · commit a0c2492b957a · 2013-04-03T14:13:34.000-04:00
The point of turning off track_activities is to avoid this reporting overhead, but a thinko in commit 4f42b54 caused pgstat_report_activity() to perform half of its updates anyway. Fix that, and also make sure that we clear all the now-disabled fields when transitioning to the non-reporting state.
diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
@@ -2520,7 +2520,7 @@ pgstat_beshutdown_hook(int code, Datum arg)
  * pgstat_report_activity() -
  *
  *	Called from tcop/postgres.c to report what the backend is actually doing
- *	(usually "<IDLE>" or the start of the query to be executed).
+ *	(but note cmd_str can be NULL for certain cases).
  *
  * All updates of the status entry follow the protocol of bumping
  * st_changecount before and after.  We use a volatile pointer here to
@@ -2540,36 +2540,40 @@ pgstat_report_activity(BackendState state, const char *cmd_str)
 	if (!beentry)
 		return;
 
-	/*
-	 * To minimize the time spent modifying the entry, fetch all the needed
-	 * data first.
-	 */
-	current_timestamp = GetCurrentTimestamp();
-
-	if (!pgstat_track_activities && beentry->st_state != STATE_DISABLED)
+	if (!pgstat_track_activities)
 	{
-		/*
-		 * Track activities is disabled, but we have a non-disabled state set.
-		 * That means the status changed - so as our last update, tell the
-		 * collector that we disabled it and will no longer update.
-		 */
-		beentry->st_changecount++;
-		beentry->st_state = STATE_DISABLED;
-		beentry->st_state_start_timestamp = current_timestamp;
-		beentry->st_changecount++;
-		Assert((beentry->st_changecount & 1) == 0);
+		if (beentry->st_state != STATE_DISABLED)
+		{
+			/*
+			 * track_activities is disabled, but we last reported a
+			 * non-disabled state.  As our final update, change the state and
+			 * clear fields we will not be updating anymore.
+			 */
+			beentry->st_changecount++;
+			beentry->st_state = STATE_DISABLED;
+			beentry->st_state_start_timestamp = 0;
+			beentry->st_activity[0] = '\0';
+			beentry->st_activity_start_timestamp = 0;
+			/* st_xact_start_timestamp and st_waiting are also disabled */
+			beentry->st_xact_start_timestamp = 0;
+			beentry->st_waiting = false;
+			beentry->st_changecount++;
+			Assert((beentry->st_changecount & 1) == 0);
+		}
 		return;
 	}
 
 	/*
-	 * Fetch more data before we start modifying the entry
+	 * To minimize the time spent modifying the entry, fetch all the needed
+	 * data first.
 	 */
 	start_timestamp = GetCurrentStatementStartTimestamp();
 	if (cmd_str != NULL)
 	{
 		len = pg_mbcliplen(cmd_str, strlen(cmd_str),
 						   pgstat_track_activity_query_size - 1);
 	}
+	current_timestamp = GetCurrentTimestamp();
 
 	/*
 	 * Now update the status entry
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
@@ -665,15 +665,8 @@ pg_stat_get_activity(PG_FUNCTION_ARGS)
 					nulls[4] = true;
 					break;
 			}
-			if (beentry->st_state == STATE_UNDEFINED ||
-				beentry->st_state == STATE_DISABLED)
-			{
-				values[5] = CStringGetTextDatum("");
-			}
-			else
-			{
-				values[5] = CStringGetTextDatum(beentry->st_activity);
-			}
+
+			values[5] = CStringGetTextDatum(beentry->st_activity);
 			values[6] = BoolGetDatum(beentry->st_waiting);
 
 			if (beentry->st_xact_start_timestamp != 0)
