[#28877] YSQL: (Auto) ANALYZE will be aborted when a global DDL runs on a different DB

Summary:
9e1c574 and f66094f previous implemented a way for DDLs concurrent with (auto) ANALYZE to be aborted. This approach does not work for the following case
1. Global DDL run against db1 - takes a for share lock on pg_yb_catalog_version row for db1
2. ANALYZE run against db2 - takes a for update lock on  pg_yb_catalog_version row for db2

To solve, this auto ANALYZE is being changed to a take a lock on all rows of pg_yb_catalog_version. This solves the case above and is verified with a unit test. Auto analyze currently does not run ANALYZE on different DBs concurrently so there is no effect on it due to this.

New approach
1. Global/Regular DDL run against db1 - takes a for share lock on pg_yb_catalog_version row for db1
2. ANALYZE run against db2 - takes a for update lock on all rows in pg_yb_catalog_version.

The other possible way to solve this is to have a global DDL lock all rows of pg_yb_catalog_version - however global DDL are not always easy to identify at the start of a DDL, which is when this lock needs to be taken.

Jira: DB-18598

Test Plan: Added new test cases.

Reviewers: pjain, yguan

Reviewed By: yguan

Subscribers: yql

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

Author: iSignal

src/postgres/src/backend/catalog/yb_catalog/yb_catalog_version.c

@@ -105,18 +105,19 @@ YbMaybeLockMasterCatalogVersion()
 	 * When auto analyze is turned on, we don't want ANALYZE DDL triggered by auto analyze to
 	 * abort user DDLs. To achieve this, regular DDLs take a FOR KEY SHARE lock on the catalog version
 	 * row with a high priority (see pg_session.cc) while ANALYZE triggered by auto analyze takes
-	 * a low priority FOR UPDATE lock on the row.
-	 * (1) Global DDLs (i.e., those that modify shared catalog tables) will increment all catalog
-	 *		 versions. We still only lock the catalog version of the current database. So, they might
-	 *		 still face the problem described above that ANALYZE can abort global DDL run on a
-	 *       different DB.
+	 * a low priority FOR UPDATE lock on all rows of the catalog version table.
+	 * (1) Global DDLs (i.e., those that modify shared catalog tables) increment all catalog
+	 *		 versions at the end of the DDL but only lock the catalog version of the current database
+	 *       when they start. To enable them to abort ANALYZE on different DBs that can conflict with
+	 *       them, ANALYZE locks all rows of catalog version table. It is challenging to identify a
+	 *       global DDL at the start of a DDL so the other way around does not work.
 	 * (2) We enable this feature only if the invalidation messages are used and per-database catalog
 	 *		 version mode is enabled.
 	 *
 	 * TODO(#27037): Re-enable table locks check when concurrent DDL is ready.
 	 */
 	if (yb_user_ddls_preempt_auto_analyze &&
-		/* !*YBCGetGFlags()->enable_object_locking_for_table_locks && */
+	/* !*YBCGetGFlags()->enable_object_locking_for_table_locks && */
 		YbIsInvalidationMessageEnabled() && YBIsDBCatalogVersionMode())
 	{
 		elog(DEBUG3, "Locking catalog version for db oid %d", MyDatabaseId);
@@ -923,13 +924,15 @@ YbGetMasterCatalogVersionFromTable(Oid db_oid, uint64_t *version,
 								  false /* is_region_local */ ,
 								  &ybc_stmt));
 
-	Datum		oid_datum = Int32GetDatum(db_oid);
-	YbcPgExpr	pkey_expr = YBCNewConstant(ybc_stmt, oid_attrdesc->atttypid,
-										   oid_attrdesc->attcollation, oid_datum,
-										   false /* is_null */ );
-
-	HandleYBStatus(YBCPgDmlBindColumn(ybc_stmt, 1, pkey_expr));
+	if (!(acquire_lock && yb_use_internal_auto_analyze_service_conn))
+	{
+		Datum		oid_datum = Int32GetDatum(db_oid);
+		YbcPgExpr	pkey_expr = YBCNewConstant(ybc_stmt, oid_attrdesc->atttypid,
+											   oid_attrdesc->attcollation,
+											   oid_datum, false /* is_null */ );
 
+		HandleYBStatus(YBCPgDmlBindColumn(ybc_stmt, 1, pkey_expr));
+	}
 	for (AttrNumber attnum = 1; attnum <= natts; ++attnum)
 		YbDmlAppendTargetRegularAttr(&Desc_pg_yb_catalog_version[attnum - 1],
 									 ybc_stmt);

src/yb/yql/pgwrapper/pg_auto_analyze-test.cc

@@ -1241,8 +1241,10 @@ class PgConcurrentDDLAnalyzeTest : public LibPqTestBase {
 };
 
 TEST_F(PgConcurrentDDLAnalyzeTest, ConcurrentDDLAnalyze) {
+  const std::string another_db_name = "another_db";
   auto* ts1 = cluster_->tserver_daemons()[0];
   auto* ts2 = cluster_->tserver_daemons()[1];
+  auto* ts3 = cluster_->tserver_daemons()[2];
   auto conn1 = ASSERT_RESULT(PGConnBuilder({
                                                .host = ts1->bind_host(),
                                                .port = ts1->ysql_port(),
@@ -1257,7 +1259,9 @@ TEST_F(PgConcurrentDDLAnalyzeTest, ConcurrentDDLAnalyze) {
   const std::string wait_string = "sleeping for 20000000 us before next ddl";
   const std::string wait_time_str = "'20s'";
 
-  ASSERT_OK(conn1.Execute("CREATE TABLE test1(k INT PRIMARY KEY, v INT) SPLIT INTO 1 TABLETS"));
+  ASSERT_OK(conn1.Execute("CREATE DATABASE " + another_db_name));
+  ASSERT_OK(conn1.Execute(
+      "CREATE ROLE role1; CREATE TABLE test1(k INT PRIMARY KEY, v INT) SPLIT INTO 1 TABLETS"));
   ASSERT_OK(conn1.Execute("INSERT INTO test1 (SELECT * FROM generate_series(1,10))"));
 
   // All DDLs on conn1 are delayed 20s prior to commit. Top-level retries are disabled.
@@ -1301,6 +1305,43 @@ TEST_F(PgConcurrentDDLAnalyzeTest, ConcurrentDDLAnalyze) {
   ASSERT_OK(LogWaiter(ts1, wait_string).WaitFor(30s));
   ASSERT_OK(conn2.Execute("ALTER TABLE test4 ADD COLUMN v1 INT"));
   thread_holder.JoinAll();
+
+  auto another_db_conn = ASSERT_RESULT(
+      pgwrapper::PGConnBuilder(
+          {.host = ts3->bind_host(), .port = ts3->ysql_port(), .dbname = another_db_name})
+          .Connect());
+  ASSERT_OK(
+      another_db_conn.Execute("SET yb_max_query_layer_retries=0; SET log_min_messages=DEBUG1;"));
+  ASSERT_OK(another_db_conn.Execute(
+      "CREATE TABLE " + another_db_name + "_test1(k INT PRIMARY KEY, v INT) split into 1 tablets"));
+
+  // Case: Non-global DDL in different databases can run concurrently
+  thread_holder.AddThreadFunctor(
+      [&conn1]() -> void { ASSERT_OK(conn1.Execute("ALTER TABLE test1 ADD COLUMN v1 INT")); });
+  ASSERT_OK(LogWaiter(ts1, wait_string).WaitFor(30s));
+  ASSERT_OK(another_db_conn.Execute("RESET yb_test_delay_next_ddl"));
+  ASSERT_OK(another_db_conn.Execute("ALTER TABLE " + another_db_name + "_test1 ADD COLUMN v1 INT"));
+  thread_holder.JoinAll();
+
+  // Case: A long (auto) ANALYZE can be interrupted by a DDL in another database
+  thread_holder.AddThreadFunctor([&conn1]() -> void {
+    ASSERT_OK(conn1.Execute("SET yb_use_internal_auto_analyze_service_conn=true"));
+    ASSERT_NOK(conn1.Execute("ANALYZE test1"));
+    ASSERT_OK(conn1.Execute("SET yb_use_internal_auto_analyze_service_conn=false"));
+  });
+  ASSERT_OK(LogWaiter(ts1, wait_string).WaitFor(30s));
+  ASSERT_OK(another_db_conn.Execute("ALTER ROLE role1 SET log_min_messages=DEBUG1"));
+  thread_holder.JoinAll();
+
+  // Case: A long running DDL in a different database cannot be interrupted by (auto) ANALYZE
+  ASSERT_OK(another_db_conn.Execute("SET yb_test_delay_next_ddl=" + wait_time_str + ";"));
+  thread_holder.AddThreadFunctor([&another_db_conn]() -> void {
+    ASSERT_OK(another_db_conn.Execute("ALTER ROLE role1 SET log_min_messages=DEBUG2"));
+  });
+  ASSERT_OK(LogWaiter(ts3, wait_string).WaitFor(30s));
+  ASSERT_NOK(conn2.Execute("SET yb_use_internal_auto_analyze_service_conn=true; ANALYZE test1"));
+  ASSERT_OK(conn2.Execute("SET yb_use_internal_auto_analyze_service_conn=false;"));
+  thread_holder.JoinAll();
 }
 
 } // namespace pgwrapper