[#20922] YSQL, ASH: Update ASH gFlags

Summary:
This diff splits the previous `yb_enable_ash` into two different gflags, a
non-runtime PG preview flag `ysql_yb_ash_enable_infra` and a runtime
PG preview flag `ysql_yb_enable_ash`.

`ysql_yb_ash_enable_infra` is responsible for one-time initialization like
allocating shared memory, starting the ASH collector and creating the
instrumentation hooks.

`ysql_yb_enable_ash` is responsible for sampling and instrumentation for
YSQL queries, YCQL queries, and some background jobs like Flushes and
Compactions

Three new gflag wrappers `ysql_yb_ash_circular_buffer_size`,
`ysql_yb_ash_sampling_interval` and `ysql_yb_ash_sample_size`
are created over previous ASH GUC variables `yb_ash_circular_buffer_size`,
`yb_ash_sampling_interval` and `yb_ash_sample_size`. They are not
changed to preview flags since they are useless without the
`ysql_yb_ash_enable_infra` and `ysql_yb_enable_ash` gflags.
Jira: DB-9904

Test Plan: ./yb_build.sh --cxx-test pg_wrapper-test --gtest_filter PgWrapperFlagsTest.*

Reviewers: jason, hsunder

Reviewed By: jason, hsunder

Subscribers: bogdan, hbhanawat, yql, ybase, amitanand

Differential Revision: https://phorge.dev.yugabyte.com/D32191

java/yb-pgsql/src/test/java/org/yb/pgsql/TestYbAsh.java

@@ -40,9 +40,11 @@ public class TestYbAsh extends BasePgSQLTest {
   private void setAshConfigAndRestartCluster(
       int sampling_interval, int sample_size) throws Exception {
     Map<String, String> flagMap = super.getTServerFlags();
-    flagMap.put("TEST_yb_enable_ash", "true");
-    flagMap.put("ysql_pg_conf_csv", "yb_ash_sampling_interval=" + sampling_interval +
-        ",yb_ash_sample_size=" + sample_size);
+    flagMap.put("allowed_preview_flags_csv", "ysql_yb_ash_enable_infra,ysql_yb_enable_ash");
+    flagMap.put("ysql_yb_ash_enable_infra", "true");
+    flagMap.put("ysql_yb_enable_ash", "true");
+    flagMap.put("ysql_yb_ash_sampling_interval", String.valueOf(sampling_interval));
+    flagMap.put("ysql_yb_ash_sample_size", String.valueOf(sample_size));
     restartClusterWithFlags(Collections.emptyMap(), flagMap);
   }
 
@@ -60,7 +62,7 @@ public void testAshViewWithoutEnablingAsh() throws Exception {
     restartCluster();
     try (Statement statement = connection.createStatement()) {
       runInvalidQuery(statement, "SELECT * FROM " + ASH_VIEW,
-          "TEST_yb_enable_ash gflag must be enabled");
+          "ysql_yb_ash_enable_infra gflag must be enabled");
     }
   }
 

src/postgres/src/backend/postmaster/pgstat.c

@@ -3613,7 +3613,7 @@ pgstat_get_wait_event_type(uint32 wait_event_info)
 	/* report process as not waiting. */
 	if (wait_event_info == 0)
 	{
-		if (IsYugaByteEnabled() && YBEnableAsh())
+		if (IsYugaByteEnabled() && yb_ash_enable_infra)
 			return "YSQLQuery";
 		return NULL;
 	}
@@ -3651,7 +3651,7 @@ pgstat_get_wait_event_type(uint32 wait_event_info)
 			break;
 		default:
 			event_type = "???";
-			if (IsYugaByteEnabled() && YBEnableAsh())
+			if (IsYugaByteEnabled() && yb_ash_enable_infra)
 				event_type = YBCGetWaitEventClass(wait_event_info);
 			break;
 	}
@@ -3675,7 +3675,7 @@ pgstat_get_wait_event(uint32 wait_event_info)
 	/* report process as not waiting. */
 	if (wait_event_info == 0)
 	{
-		if (IsYugaByteEnabled() && YBEnableAsh())
+		if (IsYugaByteEnabled() && yb_ash_enable_infra)
 			return "QueryProcessing";
 		return NULL;
 	}
@@ -3734,7 +3734,7 @@ pgstat_get_wait_event(uint32 wait_event_info)
 			}
 		default:
 			event_name = "unknown wait event";
-			if (IsYugaByteEnabled() && YBEnableAsh())
+			if (IsYugaByteEnabled() && yb_ash_enable_infra)
 				event_name = YBCGetWaitEventName(wait_event_info);
 			break;
 	}

src/postgres/src/backend/postmaster/postmaster.c

@@ -1039,7 +1039,7 @@ PostmasterMain(int argc, char *argv[])
 		ApplyLauncherRegister();
 
 	/* Register ASH collector */
-	if (YBIsEnabledInPostgresEnvVar() && YBEnableAsh())
+	if (YBIsEnabledInPostgresEnvVar() && yb_ash_enable_infra)
 		YbAshRegister();
 
 	/*

src/postgres/src/backend/storage/ipc/ipci.c

@@ -157,7 +157,7 @@ CreateSharedMemoryAndSemaphores(int port)
 		size = add_size(size, ShmemBackendArraySize());
 #endif
 
-		if (YBIsEnabledInPostgresEnvVar() && YBEnableAsh())
+		if (YBIsEnabledInPostgresEnvVar() && yb_ash_enable_infra)
 			size = add_size(size, YbAshShmemSize());
 
 		/* freeze the addin request size and include it */
@@ -274,7 +274,7 @@ CreateSharedMemoryAndSemaphores(int port)
 	AsyncShmemInit();
 	BackendRandomShmemInit();
 
-	if (YBIsEnabledInPostgresEnvVar() && YBEnableAsh())
+	if (YBIsEnabledInPostgresEnvVar() && yb_ash_enable_infra)
 		YbAshShmemInit();
 
 #ifdef EXEC_BACKEND

src/postgres/src/backend/utils/init/postinit.c

@@ -704,7 +704,7 @@ InitPostgresImpl(const char *in_dbname, Oid dboid, const char *username,
 	 * constant throughout the session. We don't want to do this during
 	 * bootstrap because it won't have client address anyway.
 	 */
-	if (IsYugaByteEnabled() && YBEnableAsh() && !bootstrap)
+	if (IsYugaByteEnabled() && yb_ash_enable_infra && !bootstrap)
 		YbSetAshClientAddrAndPort();
 
 	if (IsYugaByteEnabled() && !bootstrap)

src/postgres/src/backend/utils/misc/guc.c

@@ -2501,6 +2501,33 @@ static struct config_bool ConfigureNamesBool[] =
 		NULL, NULL, NULL
 	},
 
+	{
+		{"yb_ash_enable_infra", PGC_POSTMASTER, RESOURCES,
+			gettext_noop("Allocate shared memory for ASH, start the "
+							"background worker, create instrumentation hooks "
+							"and enable querying the yb_active_session_history "
+							"view."),
+			NULL,
+			GUC_NOT_IN_SAMPLE
+		},
+		&yb_ash_enable_infra,
+		false,
+		NULL, NULL, NULL
+	},
+
+	{
+		{"yb_enable_ash", PGC_SIGHUP, STATS_COLLECTOR,
+			gettext_noop("Starts sampling and instrumenting YSQL and YCQL queries, "
+						 "and various background activities. This does nothing if "
+						 "ysql_yb_ash_enable_infra is disabled."),
+			NULL,
+			GUC_NOT_IN_SAMPLE
+		},
+		&yb_enable_ash,
+		false,
+		yb_enable_ash_check_hook, NULL, NULL
+	},
+
 	/* End-of-list marker */
 	{
 		{NULL, 0, 0, NULL, NULL}, NULL, false, NULL, NULL, NULL
@@ -3946,19 +3973,18 @@ static struct config_int ConfigureNamesInt[] =
 
 	{
 		{"yb_ash_circular_buffer_size", PGC_POSTMASTER, STATS_MONITORING,
-			gettext_noop("Size of the circular buffer that stores wait events"),
+			gettext_noop("Size (in KiBs) of ASH circular buffer that stores the samples"),
 			NULL,
-			GUC_NO_SHOW_ALL | GUC_NO_RESET_ALL | GUC_NOT_IN_SAMPLE |
-			GUC_DISALLOW_IN_FILE | GUC_UNIT_KB
+			GUC_UNIT_KB
 		},
 		&yb_ash_circular_buffer_size,
 		16 * 1024, 0, INT_MAX,
 		NULL, NULL, NULL
 	},
 
 	{
-		{"yb_ash_sampling_interval", PGC_SUSET, STATS_MONITORING,
-			gettext_noop("Duration between each sample"),
+		{"yb_ash_sampling_interval", PGC_SIGHUP, STATS_MONITORING,
+			gettext_noop("Time (in milliseconds) between two consecutive sampling events"),
 			NULL,
 			GUC_UNIT_MS
 		},
@@ -3968,8 +3994,8 @@ static struct config_int ConfigureNamesInt[] =
 	},
 
 	{
-		{"yb_ash_sample_size", PGC_SUSET, STATS_MONITORING,
-			gettext_noop("Number of wait events captured in each sample"),
+		{"yb_ash_sample_size", PGC_SIGHUP, STATS_MONITORING,
+			gettext_noop("Number of samples captured from each component per sampling event"),
 			NULL
 		},
 		&yb_ash_sample_size,

src/postgres/src/backend/utils/misc/pg_yb_utils.c

@@ -857,10 +857,13 @@ YBInitPostgresBackend(
 		callbacks.ConstructArrayDatum = &YbConstructArrayDatum;
 		callbacks.CheckUserMap = &check_usermap;
 		callbacks.PgstatReportWaitStart = &yb_pgstat_report_wait_start;
-		YBCInitPgGate(type_table, count, callbacks, session_id, &MyProc->yb_ash_metadata,
-					  &MyProc->yb_is_ash_metadata_set);
+		YBCPgAshConfig ash_config;
+		ash_config.metadata = &MyProc->yb_ash_metadata;
+		ash_config.is_metadata_set = &MyProc->yb_is_ash_metadata_set;
+		ash_config.yb_enable_ash = &yb_enable_ash;
+		YBCInitPgGate(type_table, count, callbacks, session_id, &ash_config);
 		YBCInstallTxnDdlHook();
-		if (YBEnableAsh())
+		if (yb_ash_enable_infra)
 			YbAshInstallHooks();
 
 		/*
@@ -880,7 +883,7 @@ YBInitPostgresBackend(
 		 * mapped to PG backends.
 		 */
 		yb_pgstat_add_session_info(YBCPgGetSessionID());
-		if (YBEnableAsh())
+		if (yb_ash_enable_infra)
 			YbAshSetSessionId(YBCPgGetSessionID());
 	}
 }

src/postgres/src/backend/utils/misc/yb_ash.c

@@ -51,6 +51,8 @@
 #include "yb/yql/pggate/util/ybc_util.h"
 
 /* GUC variables */
+bool yb_ash_enable_infra;
+bool yb_enable_ash;
 int yb_ash_circular_buffer_size;
 int yb_ash_sampling_interval_ms;
 int yb_ash_sample_size;
@@ -101,6 +103,17 @@ static void uchar_to_uuid(unsigned char *in, pg_uuid_t *out);
 static void client_ip_to_string(unsigned char *client_addr, uint16 client_port,
 								uint8_t addr_family, char *client_ip);
 
+bool
+yb_enable_ash_check_hook(bool *newval, void **extra, GucSource source)
+{
+	if (*newval && !yb_ash_enable_infra)
+	{
+		GUC_check_errdetail("ysql_yb_ash_enable_infra must be enabled.");
+		return false;
+	}
+	return true;
+}
+
 void
 YbAshRegister(void)
 {
@@ -205,11 +218,14 @@ yb_ash_post_parse_analyze(ParseState *pstate, Query *query)
 	 * pg_stat_statements does. query_id can also be zero when pg_stat_statements
 	 * is disabled, then this field won't be useful for ASH users at all.
 	 */
-	uint64 query_id = query->queryId != 0
-					  ? query->queryId
-					  : yb_ash_utility_query_id(pstate->p_sourcetext, query->stmt_len,
-					  							query->stmt_location);
-	yb_set_ash_metadata(query_id);
+	if (yb_enable_ash)
+	{
+		uint64 query_id = query->queryId != 0
+						? query->queryId
+						: yb_ash_utility_query_id(pstate->p_sourcetext, query->stmt_len,
+													query->stmt_location);
+		yb_set_ash_metadata(query_id);
+	}
 }
 
 static void
@@ -219,7 +235,7 @@ yb_ash_ExecutorStart(QueryDesc *queryDesc, int eflags)
 	 * In case of prepared statements, the 'Parse' phase might be skipped.
 	 * We set the ASH metadata here if it's not been set yet.
 	 */
-	if (MyProc->yb_is_ash_metadata_set == false)
+	if (yb_enable_ash && MyProc->yb_is_ash_metadata_set == false)
 	{
 		/* Query id can be zero here only if pg_stat_statements is disabled */
 		uint64 query_id = queryDesc->plannedstmt->queryId != 0
@@ -248,7 +264,8 @@ yb_ash_ExecutorEnd(QueryDesc *queryDesc)
 	 * Unset ASH metadata. Utility statements do not go through this
 	 * code path.
 	 */
-	yb_unset_ash_metadata();
+	if (yb_enable_ash)
+		yb_unset_ash_metadata();
 }
 
 static void
@@ -270,7 +287,7 @@ yb_ash_ProcessUtility(PlannedStmt *pstmt, const char *queryString,
 	 * Unset ASH metadata in case of utility statements. This function
 	 * might recurse, and we only want to unset in the last step.
 	 */
-	if (YBGetDdlNestingLevel() == 0)
+	if (yb_enable_ash && YBGetDdlNestingLevel() == 0)
 		yb_unset_ash_metadata();
 }
 
@@ -420,7 +437,7 @@ YbAshMain(Datum main_arg)
 					(errmsg("bgworker yb_ash signal: processed SIGHUP")));
 		}
 
-		if (yb_ash_sample_size > 0)
+		if (yb_enable_ash && yb_ash_sample_size > 0)
 		{
 			sample_time = GetCurrentTimestamp();
 
@@ -541,7 +558,7 @@ yb_active_session_history(PG_FUNCTION_ARGS)
 	if (!yb_ash)
 		ereport(ERROR,
 				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-				 errmsg("TEST_yb_enable_ash gflag must be enabled")));
+				 errmsg("ysql_yb_ash_enable_infra gflag must be enabled")));
 
 	/* check to see if caller supports us returning a tuplestore */
 	if (rsinfo == NULL || !IsA(rsinfo, ReturnSetInfo))

src/postgres/src/common/pg_yb_common.c

@@ -229,13 +229,3 @@ YBColocateDatabaseByDefault()
 	}
 	return cached_value;
 }
-
-bool
-YBEnableAsh()
-{
-	static int cached_value = -1;
-	if (cached_value == -1)
-		cached_value = YBCIsEnvVarTrue("FLAGS_TEST_yb_enable_ash");
-
-	return cached_value;
-}

src/postgres/src/include/common/pg_yb_common.h

@@ -150,9 +150,4 @@ extern const bool kTestOnlyUseOSDefaultCollation;
  */
 extern bool YBColocateDatabaseByDefault();
 
-/**
- * Returns true if Active Session History should be enabled.
- */
-extern bool YBEnableAsh();
-
 #endif /* PG_YB_COMMON_H */

src/postgres/src/include/pgstat.h

@@ -21,6 +21,7 @@
 #include "utils/hsearch.h"
 #include "utils/relcache.h"
 
+#include "yb_ash.h"
 
 /* ----------
  * Paths for the statistics files (relative to installation's $PGDATA).
@@ -1447,6 +1448,10 @@ pgstat_report_wait_end(void)
 static inline uint32
 yb_pgstat_report_wait_start(uint32 wait_event_info)
 {
+	/* If ASH is disabled, do nothing */
+	if (!yb_enable_ash)
+		return wait_event_info;
+
 	uint32 prev_wait_event_info = 0;
 	volatile PGPROC *proc = MyProc;
 

src/postgres/src/include/yb_ash.h

@@ -27,9 +27,12 @@
 
 #include "postgres.h"
 #include "storage/proc.h"
+#include "utils/guc.h"
 #include "utils/timestamp.h"
 
 /* GUC variables */
+extern bool yb_ash_enable_infra;
+extern bool yb_enable_ash;
 extern int yb_ash_circular_buffer_size;
 extern int yb_ash_sampling_interval_ms;
 extern int yb_ash_sample_size;
@@ -47,4 +50,8 @@ extern bool YbAshStoreSample(PGPROC *proc, int num_procs,
 							 TimestampTz sample_time,
 							 int *samples_stored);
 
+extern bool yb_enable_ash_check_hook(bool *newval,
+									 void **extra,
+									 GucSource source);
+
 #endif							/* YB_ASH_H */