[#25578] YSQL: Move enable_ysql_conn_mgr out of preview

Summary: Move `enable_ysql_conn_mgr` out of preview. Removed `enable_ysql_conn_mgr` gflag from preview, and ported all the related tests. By default this flag would remain set to false. Setting it to true would be done for GA. Jira: DB-14832 Test Plan: Jenkins: enable connection manager, all tests Reviewers: skumar, rbarigidad, hsunder, skurapati, shikhar.sahay Reviewed By: hsunder Subscribers: ybase, yugaware Differential Revision: https://phorge.dev.yugabyte.com/D41158
yugabyte · Jan 13, 2025 · 2baf0ea · 2baf0ea
1 parent ca6dfe4
commit 2baf0ea
Show file tree

Hide file tree

Showing 16 changed files with 7 additions and 68 deletions.
diff --git a/java/yb-client/src/test/java/org/yb/minicluster/MiniYBCluster.java b/java/yb-client/src/test/java/org/yb/minicluster/MiniYBCluster.java
@@ -722,7 +722,6 @@ public void startTServer(Map<String, String> tserverFlags,
     if (clusterParameters.startYsqlConnMgr) {
       tsCmdLine.add("--ysql_conn_mgr_port=" + Integer.toString(ysqlConnMgrPort));
       tsCmdLine.add("--enable_ysql_conn_mgr=true");
-      tsCmdLine.add("--allowed_preview_flags_csv=enable_ysql_conn_mgr");
     }
 
     if (tserverFlags != null) {

diff --git a/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgAlterTable.java b/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgAlterTable.java
@@ -36,14 +36,6 @@
 public class TestPgAlterTable extends BasePgSQLTest {
   private static final Logger LOG = LoggerFactory.getLogger(TestPgAlterTable.class);
 
-  @Override
-  protected Map<String, String> getTServerFlags() {
-    Map<String, String> flagMap = super.getTServerFlags();
-    if (isTestRunningWithConnectionManager())
-      flagMap.put("allowed_preview_flags_csv", "enable_ysql_conn_mgr");
-    return flagMap;
-  }
-
   @Test
   public void testAddColumnIfNotExists() throws Exception {
     testAddColumnIfNotExistsInternal(true);

diff --git a/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgAuthorization.java b/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgAuthorization.java
@@ -58,7 +58,7 @@ protected Map<String, String> getTServerFlags() {
     flags.put("ysql_hba_conf", CUSTOM_PG_HBA_CONFIG);
     if(isTestRunningWithConnectionManager()) {
        flags.put("allowed_preview_flags_csv",
-                ",enable_ysql_conn_mgr,ysql_conn_mgr_version_matching,"
+                "ysql_conn_mgr_version_matching,"
                 + "ysql_conn_mgr_version_matching_connect_higher_version");
       flags.put("enable_ysql_conn_mgr", "true");
       flags.put("ysql_conn_mgr_version_matching", "true");

diff --git a/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgRegressColocatedTablesWithTablespaces.java b/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgRegressColocatedTablesWithTablespaces.java
@@ -61,9 +61,6 @@ protected void customizeMiniClusterBuilder(MiniYBClusterBuilder builder) {
   @Override
   protected Map<String, String> getTServerFlags() {
     Map<String, String> flagMap = super.getTServerFlags();
-    if (isTestRunningWithConnectionManager()) {
-      flagMap.put("allowed_preview_flags_csv", "enable_ysql_conn_mgr");
-    }
     flagMap.put("ysql_enable_colocated_tables_with_tablespaces", "true");
     return flagMap;
   }

diff --git a/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgRegressPublication.java b/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgRegressPublication.java
@@ -23,14 +23,6 @@
  */
 @RunWith(value = YBTestRunner.class)
 public class TestPgRegressPublication extends BasePgRegressTest {
-  @Override
-  protected Map<String, String> getTServerFlags() {
-    Map<String, String> flagMap = super.getTServerFlags();
-    if (isTestRunningWithConnectionManager()) {
-      flagMap.put("allowed_preview_flags_csv", "enable_ysql_conn_mgr");
-    }
-    return flagMap;
-  }
 
   @Override
   public int getTestMethodTimeoutSec() {

diff --git a/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgRegressReplicaIdentity.java b/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgRegressReplicaIdentity.java
@@ -28,14 +28,6 @@ public int getTestMethodTimeoutSec() {
     return 1800;
   }
 
-  @Override
-  protected Map<String, String> getTServerFlags() {
-    Map<String, String> flagMap = super.getTServerFlags();
-    if (isTestRunningWithConnectionManager())
-      flagMap.put("allowed_preview_flags_csv", "enable_ysql_conn_mgr");
-    return flagMap;
-  }
-
   @Test
   public void testPgRegressTable() throws Exception {
     runPgRegressTest("yb_replica_identity_schedule");

diff --git a/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgRegressReplicationSlot.java b/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgRegressReplicationSlot.java
@@ -33,8 +33,6 @@ protected Map<String, String> getTServerFlags() {
     Map<String, String> flagMap = super.getTServerFlags();
 
     if (isTestRunningWithConnectionManager()) {
-      flagMap.put("allowed_preview_flags_csv",
-          "enable_ysql_conn_mgr");
       flagMap.put("enable_ysql_conn_mgr", "true");
     }
     flagMap.put(

diff --git a/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgRegressThirdPartyExtensionsPgCron.java b/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgRegressThirdPartyExtensionsPgCron.java
@@ -37,9 +37,6 @@ protected Map<String, String> getMasterFlags() {
   @Override
   protected Map<String, String> getTServerFlags() {
     Map<String, String> flagMap = super.getTServerFlags();
-    if (isTestRunningWithConnectionManager()) {
-      flagMap.put("allowed_preview_flags_csv", "enable_ysql_conn_mgr");
-    }
     flagMap.put("enable_pg_cron", "true");
     return flagMap;
   }

diff --git a/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgReplicationSlot.java b/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgReplicationSlot.java
@@ -71,8 +71,6 @@ protected int getInitialNumTServers() {
   protected Map<String, String> getTServerFlags() {
     Map<String, String> flagMap = super.getTServerFlags();
     if (isTestRunningWithConnectionManager()) {
-      String preview_flags = "enable_ysql_conn_mgr";
-      flagMap.put("allowed_preview_flags_csv",preview_flags);
       flagMap.put("ysql_conn_mgr_stats_interval", "1");
     }
     flagMap.put(

diff --git a/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgUniqueConstraint.java b/java/yb-pgsql/src/test/java/org/yb/pgsql/TestPgUniqueConstraint.java
@@ -401,12 +401,6 @@ public void createIndexViolatingUniqueness() throws Exception {
       assertTrue(miniCluster.getClient().setFlag(hp,
           "enable_transactional_ddl_gc", "false"));
     }
-    if (isTestRunningWithConnectionManager()) {
-      for (HostAndPort hp : miniCluster.getTabletServers().keySet()) {
-          assertTrue(miniCluster.getClient().setFlag(hp,
-              "allowed_preview_flags_csv", "enable_ysql_conn_mgr"));
-      }
-    }
     try (Statement stmt = connection.createStatement()) {
       runInvalidQuery(
           stmt,

diff --git a/java/yb-pgsql/src/test/java/org/yb/pgsql/TestYbAsh.java b/java/yb-pgsql/src/test/java/org/yb/pgsql/TestYbAsh.java
@@ -42,17 +42,9 @@ public class TestYbAsh extends BasePgSQLTest {
   private static final int ASH_SAMPLE_SIZE = 500;
   private static final String ASH_VIEW = "yb_active_session_history";
 
-  private Map<String, String> getTServerFlagMapWithPreviewFlags() throws Exception {
-    Map<String, String> flagMap = super.getTServerFlags();
-    if (isTestRunningWithConnectionManager()) {
-     flagMap.put("allowed_preview_flags_csv", "enable_ysql_conn_mgr");
-    }
-    return flagMap;
-  }
-
   private void setAshConfigAndRestartCluster(
       int sampling_interval, int sample_size, int circular_buffer_size) throws Exception {
-    Map<String, String> flagMap = getTServerFlagMapWithPreviewFlags();
+    Map<String, String> flagMap = super.getTServerFlags();
     flagMap.put("ysql_yb_enable_ash", "true");
     flagMap.put("ysql_yb_ash_sampling_interval_ms", String.valueOf(sampling_interval));
     flagMap.put("ysql_yb_ash_sample_size", String.valueOf(sample_size));
@@ -64,7 +56,7 @@ private void setAshConfigAndRestartCluster(
   }
 
   private void resetAshConfigAndRestartCluster() throws Exception {
-    Map<String, String> flagMap = getTServerFlagMapWithPreviewFlags();
+    Map<String, String> flagMap = super.getTServerFlags();
     flagMap.put("ysql_yb_enable_ash", "false");
     restartClusterWithFlags(Collections.emptyMap(), flagMap);
   }

diff --git a/java/yb-pgsql/src/test/java/org/yb/pgsql/TestYsqlDump.java b/java/yb-pgsql/src/test/java/org/yb/pgsql/TestYsqlDump.java
@@ -240,7 +240,6 @@ public void ysqlDumpColocatedDB() throws Exception {
   public void ysqlDumpColocatedTablesWithTablespaces() throws Exception {
     markClusterNeedsRecreation();
     restartClusterWithClusterBuilder(cb -> {
-      cb.addCommonFlag("allowed_preview_flags_csv", "enable_ysql_conn_mgr");
       cb.addCommonFlag("ysql_enable_colocated_tables_with_tablespaces", "true");
       cb.addCommonTServerFlag("placement_cloud", "testCloud");
       cb.addCommonTServerFlag("placement_region", "testRegion");

diff --git a/java/yb-ysql-conn-mgr/src/test/java/org/yb/ysqlconnmgr/BaseYsqlConnMgr.java b/java/yb-ysql-conn-mgr/src/test/java/org/yb/ysqlconnmgr/BaseYsqlConnMgr.java
@@ -82,7 +82,6 @@ protected Map<String, String> getTServerFlags() {
     Map<String, String> flagMap = super.getTServerFlags();
 
     flagMap.put("enable_ysql_conn_mgr", "true");
-    flagMap.put("allowed_preview_flags_csv", "enable_ysql_conn_mgr");
     flagMap.put("ysql_conn_mgr_dowarmup", "false");
     if (warmup_random_mode) {
       flagMap.put("TEST_ysql_conn_mgr_dowarmup_all_pools_mode", "random");
@@ -139,18 +138,18 @@ protected void enableVersionMatchingAndRestartCluster(boolean higher_version_mat
         throws Exception {
     Map<String, String> tsFlagMap = new HashMap<>();
     tsFlagMap.put("allowed_preview_flags_csv",
-            ",enable_ysql_conn_mgr,ysql_conn_mgr_version_matching");
+            "ysql_conn_mgr_version_matching");
     tsFlagMap.put("enable_ysql_conn_mgr", "true");
     tsFlagMap.put("ysql_conn_mgr_version_matching", "true");
 
     if (higher_version_matching) {
         tsFlagMap.put("allowed_preview_flags_csv",
-                ",enable_ysql_conn_mgr,ysql_conn_mgr_version_matching,"
+                "ysql_conn_mgr_version_matching,"
                 + "ysql_conn_mgr_version_matching_connect_higher_version");
         tsFlagMap.put("ysql_conn_mgr_version_matching_connect_higher_version", "true");
     } else {
        tsFlagMap.put("allowed_preview_flags_csv",
-                ",enable_ysql_conn_mgr,ysql_conn_mgr_version_matching,"
+                "ysql_conn_mgr_version_matching,"
                 + "ysql_conn_mgr_version_matching_connect_higher_version");
         tsFlagMap.put("ysql_conn_mgr_version_matching_connect_higher_version", "false");
     }

diff --git a/java/yb-yugabyted/src/test/java/org/yb/yugabyted/TestYugabytedConnMgr.java b/java/yb-yugabyted/src/test/java/org/yb/yugabyted/TestYugabytedConnMgr.java
@@ -31,7 +31,6 @@ public TestYugabytedConnMgr() {
         clusterConfigurations = new ArrayList<>();
         Map<String, String> tserverFlags = new HashMap<>();
         tserverFlags.put("enable_ysql_conn_mgr", "true");
-        tserverFlags.put("allowed_preview_flags_csv", "{enable_ysql_conn_mgr}");
 
         for (int i = 0; i < clusterParameters.numNodes; i++) {
             MiniYugabytedNodeConfigurations nodeConfigurations =
@@ -59,25 +58,17 @@ public void testConnMgr() throws Exception {
         JSONObject jsonObject = new JSONObject(jsonResponse);
         JSONArray flags = jsonObject.getJSONArray("flags");
 
-        boolean allowedPreviewFlagsCsvFound = false;
         boolean enableYsqlConnMgrFound = false;
 
         for (int i = 0; i < flags.length(); i++) {
             JSONObject flag = flags.getJSONObject(i);
-            if (flag.getString("name").equals("allowed_preview_flags_csv") &&
-                flag.getString("value").equals("enable_ysql_conn_mgr") &&
-                flag.getString("type").equals("Custom")) {
-                allowedPreviewFlagsCsvFound = true;
-            }
             if (flag.getString("name").equals("enable_ysql_conn_mgr") &&
                 flag.getString("value").equals("true") &&
                 flag.getString("type").equals("Custom")) {
                 enableYsqlConnMgrFound = true;
             }
         }
 
-        assertTrue("allowed_preview_flags_csv flag should be found",
-                                                    allowedPreviewFlagsCsvFound);
         assertTrue("enable_ysql_conn_mgr flag should be found",
                                                     enableYsqlConnMgrFound);
     }

diff --git a/.../src/main/java/com/yugabyte/yw/commissioner/tasks/subtasks/KubernetesCommandExecutor.java b/.../src/main/java/com/yugabyte/yw/commissioner/tasks/subtasks/KubernetesCommandExecutor.java
@@ -1258,7 +1258,6 @@ private String generateHelmOverride() {
         // If given, use it as an override.
         int internalYsqlServerRpcPort = DEFAULT_INTERNAL_YSQL_SERVER_RPC_PORT;
         tserverGFlags.put("enable_ysql_conn_mgr", "true");
-        tserverGFlags.put("allowed_preview_flags_csv", "enable_ysql_conn_mgr");
         tserverGFlags.put("ysql_conn_mgr_port", String.valueOf(ysqlServerRpcPort));
         tserverGFlags.put(
             "pgsql_proxy_bind_address",

diff --git a/src/yb/tserver/tablet_server.cc b/src/yb/tserver/tablet_server.cc
@@ -165,7 +165,7 @@ DEFINE_UNKNOWN_int32(cql_proxy_webserver_port, 0, "Webserver port for CQL proxy"
 DEFINE_NON_RUNTIME_string(pgsql_proxy_bind_address, "", "Address to bind the PostgreSQL proxy to");
 DECLARE_int32(pgsql_proxy_webserver_port);
 
-DEFINE_NON_RUNTIME_PREVIEW_bool(enable_ysql_conn_mgr, false,
+DEFINE_NON_RUNTIME_bool(enable_ysql_conn_mgr, false,
     "Enable Ysql Connection Manager for the cluster. Tablet Server will start a "
     "Ysql Connection Manager process as a child process.");