From 51f35fce4d3da3735d217c4524001640c1d7a727 Mon Sep 17 00:00:00 2001 From: rahulb-yb Date: Mon, 29 Apr 2024 03:17:21 +0000 Subject: [PATCH] [#21757] YSQL: Support for single-use GUC variables in YSQL Connection Manager Summary: YSQL Connection Manager stores the context of altered GUC variables for each logical connection, and replays the stored context at the beginning of every transaction boundary on the physical connection being used. There are some variables that are implicitly set to a different value after a certain event has occurred. One such example of this is `yb_test_fail_next_ddl`, which returns an error upon the execution of the next DDL statement, and then sets itself to false. These implicit changes cannot be tracked by Connection Manager, and this patch creates the provision for Connection Manager to do so. Changes in this diff: - Remove changes made as a "hack" to pass a test in TestYsqlUpgrade.java - When connection manager is the client, if any of the variables in the single-use category are implicitly changed, send a ParameterStatus packet back to Connection Manager Jira: DB-10631 Test Plan: Jenkins: enable connection manager, test regex: .*TestYsqlUpgrade.* ./yb_build.sh --enable-ysql-conn-mgr-test --java-test org.yb.pgsql.TestYsqlUpgrade#creatingSystemRelsAfterFailure Reviewers: nkumar, mkumar Reviewed By: nkumar Subscribers: yql Differential Revision: https://phorge.dev.yugabyte.com/D34450 --- .../java/org/yb/pgsql/TestYsqlUpgrade.java | 3 --- .../catalog/yb_catalog/yb_catalog_version.c | 3 +++ .../src/backend/utils/misc/pg_yb_utils.c | 7 ++++++ .../utils/misc/yb_ysql_conn_mgr_helper.c | 24 +++++++++++++++++++ .../src/include/yb_ysql_conn_mgr_helper.h | 2 ++ 5 files changed, 36 insertions(+), 3 deletions(-) diff --git a/java/yb-pgsql/src/test/java/org/yb/pgsql/TestYsqlUpgrade.java b/java/yb-pgsql/src/test/java/org/yb/pgsql/TestYsqlUpgrade.java index 9ee738299f6..f46d45eb2c0 100644 --- a/java/yb-pgsql/src/test/java/org/yb/pgsql/TestYsqlUpgrade.java +++ b/java/yb-pgsql/src/test/java/org/yb/pgsql/TestYsqlUpgrade.java @@ -450,9 +450,6 @@ public void creatingSystemRelsAfterFailure() throws Exception { // Letting CatalogManagerBgTasks do the cleanup. Thread.sleep(BuildTypeUtil.adjustTimeout(5000)); - if (isTestRunningWithConnectionManager()) { - stmt.execute("SET yb_test_fail_next_ddl TO false"); - } stmt.execute(ddlSql); String selectSql = "SELECT * FROM simple_system_table"; diff --git a/src/postgres/src/backend/catalog/yb_catalog/yb_catalog_version.c b/src/postgres/src/backend/catalog/yb_catalog/yb_catalog_version.c index 169a4d96ff8..5eada6bebec 100644 --- a/src/postgres/src/backend/catalog/yb_catalog/yb_catalog_version.c +++ b/src/postgres/src/backend/catalog/yb_catalog/yb_catalog_version.c @@ -316,6 +316,9 @@ bool YbIncrementMasterCatalogVersionTableEntry(bool is_breaking_change, if (yb_test_fail_next_inc_catalog_version) { yb_test_fail_next_inc_catalog_version = false; + if (YbIsClientYsqlConnMgr()) + YbSendParameterStatusForConnectionManager("yb_test_fail_next_inc_catalog_version", + "false"); elog(ERROR, "Failed increment catalog version as requested"); } diff --git a/src/postgres/src/backend/utils/misc/pg_yb_utils.c b/src/postgres/src/backend/utils/misc/pg_yb_utils.c index 27dab14d2af..f4bfc3322a3 100644 --- a/src/postgres/src/backend/utils/misc/pg_yb_utils.c +++ b/src/postgres/src/backend/utils/misc/pg_yb_utils.c @@ -1441,7 +1441,12 @@ YBResetEnableNonBreakingDDLMode() /* * Reset yb_make_next_ddl_statement_nonbreaking to avoid its further side * effect that may not be intended. + * + * Also, reset Connection Manager cache if the value was cached to begin + * with. */ + if (YbIsClientYsqlConnMgr() && yb_make_next_ddl_statement_nonbreaking) + YbSendParameterStatusForConnectionManager("yb_make_next_ddl_statement_nonbreaking", "false"); yb_make_next_ddl_statement_nonbreaking = false; } @@ -1544,6 +1549,8 @@ YBDecrementDdlNestingLevel() if (yb_test_fail_next_ddl) { yb_test_fail_next_ddl = false; + if (YbIsClientYsqlConnMgr()) + YbSendParameterStatusForConnectionManager("yb_test_fail_next_ddl", "false"); elog(ERROR, "Failed DDL operation as requested"); } if (ddl_transaction_state.nesting_level == 0) diff --git a/src/postgres/src/backend/utils/misc/yb_ysql_conn_mgr_helper.c b/src/postgres/src/backend/utils/misc/yb_ysql_conn_mgr_helper.c index 5895c9c5411..adbfa79c784 100644 --- a/src/postgres/src/backend/utils/misc/yb_ysql_conn_mgr_helper.c +++ b/src/postgres/src/backend/utils/misc/yb_ysql_conn_mgr_helper.c @@ -862,3 +862,27 @@ YbGetNumYsqlConnMgrConnections(const char *db_name, const char *user_name, shmdt(shmp); return true; } + +/* + * Create a provision to send a ParameterStatus packet back to Connection Manager to + * change the cached value of a certain GUC variable, outside of the usual + * ReportGucOption function. This can be useful for some implicit changes to + * GUC variable values that do not normally send a ParameterStatus packet + * back to Connection Manager. + */ +void +YbSendParameterStatusForConnectionManager(const char *name, const char *value) +{ + Assert(YbIsClientYsqlConnMgr()); + + CHECK_FOR_INTERRUPTS(); + StringInfoData msgbuf; + + pq_beginmessage(&msgbuf, 'S'); + pq_sendstring(&msgbuf, name); + pq_sendstring(&msgbuf, value); + pq_endmessage(&msgbuf); + + pq_flush(); + CHECK_FOR_INTERRUPTS(); +} diff --git a/src/postgres/src/include/yb_ysql_conn_mgr_helper.h b/src/postgres/src/include/yb_ysql_conn_mgr_helper.h index c6d753f2218..843edb9c5b7 100644 --- a/src/postgres/src/include/yb_ysql_conn_mgr_helper.h +++ b/src/postgres/src/include/yb_ysql_conn_mgr_helper.h @@ -94,3 +94,5 @@ extern bool YbGetNumYsqlConnMgrConnections(const char *db_name, uint32_t *num_physical_conn); extern void yb_is_client_ysqlconnmgr_assign_hook(bool newval, void *extra); + +extern void YbSendParameterStatusForConnectionManager(const char *name, const char *value);