[#25362] YSQL: PG15: Block DDLs during ysql major upgrade

Summary:
Block updates to the ysql catalog during ysql major (pg15) upgrade. For temp table DDLs, pg performs the write using the `force_catalog_modifications` flag. Updating this logic to also include the initdb, and binary upgrade (pg_upgrade) phases.
yb-master rejects all updates to the ysql catalog during the ysql major upgrade, unless it has the `force_catalog_modifications` flag set.
DDLs are allowed only after the upgrade has finalized (ysql catalog state `DONE`). In this state older catalog is fully blocked even if `force_catalog_modifications` flag is set.

- Move catalog state related logic from `YsqlCatalogConfig` to `YsqlInitDBAndMajorUpgradeHandler`.
- Move `YBIsMajorUpgradeInitDb` from `pg_yb_common.c` to `ybc_util.cc`, and cache the env var.
- Disable YSQL webserver in binary upgrade mode. The webserver fails to start since it cannot bind to the port. The normal postgres process started by the tserver uses this port. This webserver is not needed, so instead of trying to assign another port, its better to not run it at all.
- Remove warning line from `MasterTabletServer::GetMessenger`. The caller is expected to handle the nullptr, and so this is just spamming the logs.
- Bumped the build to 2024.2.1.0-b116
- Handle the case where yb-master is downgraded in the middle of an ysql catalog upgrade by crashing the master with a friendly error message.

Fixes #25362
Jira: DB-14586

Test Plan: ysql_major_upgrade_ddl_blocking-test

Reviewers: smishra, telgersma

Reviewed By: telgersma

Subscribers: svc_phabricator, yql, ybase

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

pg15_tests/test_upgrade.sh

@@ -33,7 +33,6 @@ restart_node_2_in_pg15
 
 # Demonstrate simultaneous access for DMLs before the upgrade has been finalized. (DDLs are not
 # allowed, and rollback to PG11 is still possible.)
-# YB_TODO: Test that DDLs are prohibited when the functionality is implemented.
 
 # Insert from PG15
 diff <(ysqlsh 2 <<EOT | sed 's/ *$//'

src/postgres/src/common/pg_yb_common.c

@@ -240,13 +240,6 @@ YBColocateDatabaseByDefault()
 	return cached_value;
 }
 
-bool
-YBIsMajorUpgradeInitDb()
-{
-	return YBCIsEnvVarTrueWithDefault("YB_PG_MAJOR_UPGRADE_INITDB",
-									  false /* default_value */);
-}
-
 Oid YBGetDatabaseOidFromEnv(const char *database_name)
 {
 	char *env_var = psprintf("YB_DATABASE_OID_%s", database_name);

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

@@ -151,11 +151,6 @@ extern const bool kTestOnlyUseOSDefaultCollation;
  */
 extern bool YBColocateDatabaseByDefault();
 
-/**
- * Returns whether we're doing an initdb for a ysql major upgrade.
- */
-extern bool YBIsMajorUpgradeInitDb();
-
 /**
  * Returns the OID for database_name from the environment, if it exists and is
  * valid. Otherwise, returns InvalidOid.

src/postgres/yb-extensions/yb_pg_metrics/yb_pg_metrics.c

@@ -770,28 +770,32 @@ _PG_init(void)
 
 	BackgroundWorker worker;
 
-	/* Registering the YSQL webserver as a background worker */
-	MemSet(&worker, 0, sizeof(BackgroundWorker));
-	strcpy(worker.bgw_name, "YSQL webserver");
-	worker.bgw_flags = BGWORKER_SHMEM_ACCESS;
-	worker.bgw_start_time = BgWorkerStart_PostmasterStart;
-	/* Value of 1 allows the background worker for webserver to restart */
-	worker.bgw_restart_time = 1;
-	worker.bgw_main_arg = (Datum) 0;
-	strcpy(worker.bgw_library_name, "yb_pg_metrics");
-	strcpy(worker.bgw_function_name, "webserver_worker_main");
-	worker.bgw_notify_pid = 0;
-	if (getenv("FLAGS_yb_webserver_oom_score_adj") != NULL)
-		strncpy(worker.bgw_oom_score_adj,
-				getenv("FLAGS_yb_webserver_oom_score_adj"),
-				BGW_MAXLEN);
-
-	RegisterBackgroundWorker(&worker);
-	/*
-	 * Set the value of the hooks.
-	 */
+	if (!IsBinaryUpgrade)
+	{
+		/* Registering the YSQL webserver as a background worker */
+		MemSet(&worker, 0, sizeof(BackgroundWorker));
+		strcpy(worker.bgw_name, "YSQL webserver");
+		worker.bgw_flags = BGWORKER_SHMEM_ACCESS;
+		worker.bgw_start_time = BgWorkerStart_PostmasterStart;
+		/* Value of 1 allows the background worker for webserver to restart */
+		worker.bgw_restart_time = 1;
+		worker.bgw_main_arg = (Datum) 0;
+		strcpy(worker.bgw_library_name, "yb_pg_metrics");
+		strcpy(worker.bgw_function_name, "webserver_worker_main");
+		worker.bgw_notify_pid = 0;
+		if (getenv("FLAGS_yb_webserver_oom_score_adj") != NULL)
+			strncpy(worker.bgw_oom_score_adj,
+					getenv("FLAGS_yb_webserver_oom_score_adj"),
+					BGW_MAXLEN);
+
+		RegisterBackgroundWorker(&worker);
+	}
+
+  /*
+   * Set the value of the hooks.
+   */
 
-	prev_shmem_request_hook = shmem_request_hook;
+  prev_shmem_request_hook = shmem_request_hook;
 	shmem_request_hook = ybpgm_shmem_request;
 
 	prev_shmem_startup_hook = shmem_startup_hook;

src/yb/common/entity_ids.cc

@@ -33,10 +33,8 @@ namespace {
 // Dev Note: When the major catalog version changes, meaning, for the YugabyteDB version that uses
 // PG17+ for the YSQL layer, update kPgPreviousUuidVersion to kPgCurrentUuidVersion, and increase
 // kPgCurrentUuidVersion by 1.
-// PG11
-constexpr uint8_t kPgPreviousUuidVersion = 0;
-// PG15
-constexpr uint8_t kPgCurrentUuidVersion = 1;
+constexpr uint8_t kPgPreviousUuidVersion = 0;  // PG11
+constexpr uint8_t kPgCurrentUuidVersion = 1;   // PG15
 
 }  // namespace
 
@@ -55,7 +53,7 @@ const TableId kPgProcTableId = GetPgsqlTableId(kTemplate1Oid, kPgProcTableOid);
 const TableId kPgYbCatalogVersionTableId =
     GetPgsqlTableId(kTemplate1Oid, kPgYbCatalogVersionTableOid);
 const TableId kPgYbCatalogVersionTableIdPriorVersion =
-    GetPgsqlTableIdPriorVersion(kTemplate1Oid, kPgYbCatalogVersionTableOid);
+    GetPriorVersionYsqlCatalogTableId(kTemplate1Oid, kPgYbCatalogVersionTableOid);
 const TableId kPgTablespaceTableId =
     GetPgsqlTableId(kTemplate1Oid, kPgTablespaceTableOid);
 const TableId kPgSequencesDataTableId =
@@ -69,8 +67,6 @@ const string kPgSequencesDataNamespaceId =
 
 namespace {
 
-YB_STRONGLY_TYPED_BOOL(IsCurrentVersion);
-
 // Layout of Postgres database and table 4-byte oids in a YugaByte 16-byte table UUID:
 //
 // +-----------------------------------------------------------------------------------------------+
@@ -104,6 +100,15 @@ void UuidSetTableIds(const uint32_t table_oid, uuid* id) {
   id->data[15] = table_oid & 0xFF;
 }
 
+inline void UuidSetPgVersion(uuid& id, bool is_current_version) {
+  id.data[9] = is_current_version ? kPgCurrentUuidVersion : kPgPreviousUuidVersion;
+}
+
+inline bool IsCurrentPgVersion(const TableId& table_id) {
+  const auto binary_id = a2b_hex(table_id);
+  return binary_id[9] == kPgCurrentUuidVersion;
+}
+
 std::string UuidToString(uuid* id) {
   // Set variant that is stored in octet 7, which is index 8, since indexes count backwards.
   // Variant must be 0b10xxxxxx for RFC 4122 UUID variant 1.
@@ -118,24 +123,36 @@ std::string UuidToString(uuid* id) {
 }
 
 TableId GetPgsqlTableIdInternal(
-    const uint32_t database_oid, const uint32_t table_oid, IsCurrentVersion is_current_version) {
+    const uint32_t database_oid, const uint32_t table_oid, bool is_current_version) {
   uuid id = boost::uuids::nil_uuid();
   UuidSetDatabaseId(database_oid, &id);
 
   // For catalog tables, we need to set the correct version in id.data[9], which is "pgv" in the
   // above diagram. Note that normal object IDs are versionless, always id.data[9] == 0.
   if (table_oid < kPgFirstNormalObjectId) {
-    if (is_current_version) {
-      id.data[9] = kPgCurrentUuidVersion;
-    } else {
-      id.data[9] = kPgPreviousUuidVersion;
-    }
+    UuidSetPgVersion(id, is_current_version);
+  } else {
+    LOG_IF(DFATAL, !is_current_version) << "User table IDs do not have prior versions.";
   }
 
   UuidSetTableIds(table_oid, &id);
   return UuidToString(&id);
 }
 
+bool IsYsqlCatalogTable(const TableId& table_id) {
+  if (!IsPgsqlId(table_id)) {
+    return false;
+  }
+  Result<uint32_t> oid_res = GetPgsqlTableOid(table_id);
+  if (!oid_res.ok()) {
+    YB_LOG_EVERY_N_SECS(WARNING, 5)
+        << "Invalid PostgreSQL table id " << table_id << ": " << oid_res.status();
+    return false;
+  }
+
+  return *oid_res < kPgFirstNormalObjectId;
+}
+
 } // namespace
 
 NamespaceId GetPgsqlNamespaceId(const uint32_t database_oid) {
@@ -145,11 +162,7 @@ NamespaceId GetPgsqlNamespaceId(const uint32_t database_oid) {
 }
 
 TableId GetPgsqlTableId(const uint32_t database_oid, const uint32_t table_oid) {
-  return GetPgsqlTableIdInternal(database_oid, table_oid, IsCurrentVersion::kTrue);
-}
-
-TableId GetPgsqlTableIdPriorVersion(const uint32_t database_oid, const uint32_t table_oid) {
-  return GetPgsqlTableIdInternal(database_oid, table_oid, IsCurrentVersion::kFalse);
+  return GetPgsqlTableIdInternal(database_oid, table_oid, /*is_current_version=*/true);
 }
 
 TablegroupId GetPgsqlTablegroupId(const uint32_t database_oid, const uint32_t tablegroup_oid) {
@@ -231,29 +244,24 @@ Result<uint32_t> GetPgsqlTablespaceOid(const TablespaceId& tablespace_id) {
   return GetPgsqlOid(tablespace_id, 0, "tablespace id");
 }
 
-bool IsPriorVersionCatalogId(const TableId& hex_id) {
-  if (!IsPgsqlId(hex_id)) {
-    return false;
-  }
-  Result<uint32_t> res_oid = GetPgsqlTableOid(hex_id);
-  if (!res_oid.ok()) {
+TableId GetPriorVersionYsqlCatalogTableId(const uint32_t database_oid, const uint32_t table_oid) {
+  return GetPgsqlTableIdInternal(database_oid, table_oid, /*is_current_version=*/false);
+}
+
+bool IsPriorVersionYsqlCatalogTable(const TableId& table_id) {
+  if (!IsYsqlCatalogTable(table_id)) {
     return false;
   }
-  uint32_t oid = *res_oid;
-  std::string id = a2b_hex(hex_id);
-  // Dev Note: When the major catalog version changes, meaning, for the YugabyteDB version that uses
-  // PG17+ for the YSQL layer, the check for oid being less than kPgFirstNormalObjectId will no
-  // longer be necessary because user tables will keep version 0 forever and the previous version
-  // will be > 0.
-  return id[9] == kPgPreviousUuidVersion && oid < kPgFirstNormalObjectId;
+
+  return !IsCurrentPgVersion(table_id);
 }
 
-bool IsCurrentVersionCatalogId(const TableId& hex_id) {
-  if (!IsPgsqlId(hex_id)) {
+bool IsCurrentVersionYsqlCatalogTable(const TableId& table_id) {
+  if (!IsYsqlCatalogTable(table_id)) {
     return false;
   }
-  std::string id = a2b_hex(hex_id);
-  return id[9] == kPgCurrentUuidVersion;
+
+  return IsCurrentPgVersion(table_id);
 }
 
 namespace xrepl {

src/yb/common/entity_ids.h

@@ -52,7 +52,6 @@ NamespaceId GetPgsqlNamespaceId(uint32_t database_oid);
 
 // Get YB table id for a Postgres table.
 TableId GetPgsqlTableId(uint32_t database_oid, uint32_t table_oid);
-TableId GetPgsqlTableIdPriorVersion(uint32_t database_oid, uint32_t table_oid);
 
 // Get YB tablegroup id for a Postgres tablegroup.
 TablegroupId GetPgsqlTablegroupId(uint32_t database_oid, uint32_t tablegroup_oid);
@@ -72,12 +71,15 @@ Result<uint32_t> GetPgsqlDatabaseOidByTableId(const TableId& table_id);
 Result<uint32_t> GetPgsqlDatabaseOidByTablegroupId(const TablegroupId& tablegroup_id);
 Result<uint32_t> GetPgsqlTablespaceOid(const TablespaceId& tablespace_id);
 
-// Called with any table UUID, is it a catalog ID for the prior version (pre-ysql major catalog
-// upgrade)? All other cases return false.
-bool IsPriorVersionCatalogId(const TableId& id);
+// NOTE: Only catalog table oids are allowed.
+TableId GetPriorVersionYsqlCatalogTableId(uint32_t database_oid, uint32_t table_oid);
 
-// Called with any table UUID, is it a catalog ID for the current version? All other cases return
-// false.
-bool IsCurrentVersionCatalogId(const TableId& id);
+// If this is a YSQL catalog table, then is it the previous version?
+// Returns false for user tables.
+bool IsPriorVersionYsqlCatalogTable(const TableId& table_id);
+
+// If this is a YSQL catalog table, then is it the current version?
+// Returns false for user tables.
+bool IsCurrentVersionYsqlCatalogTable(const TableId& table_id);
 
 }  // namespace yb

src/yb/integration-tests/CMakeLists.txt

@@ -267,6 +267,7 @@ ADD_YB_TEST(upgrade-tests/basic_upgrade-test)
 ADD_YB_TEST(upgrade-tests/pg15_upgrade-test)
 ADD_YB_TEST(upgrade-tests/pg15_upgrade_pgregress-test)
 ADD_YB_TEST(upgrade-tests/ysql_major_upgrade_rpcs-test)
+ADD_YB_TEST(upgrade-tests/ysql_major_upgrade_ddl_blocking-test)
 
 set(YB_TEST_LINK_LIBS_SAVED ${YB_TEST_LINK_LIBS})
 set(YB_TEST_LINK_LIBS ${YB_TEST_LINK_LIBS} cassandra)

src/yb/integration-tests/upgrade-tests/builds.xml

@@ -57,6 +57,21 @@ NOTE:
       https://s3.us-west-2.amazonaws.com/uploads.dev.yugabyte.com/local-provider-test/2024.2.0.0-b58/yugabyte-2024.2.0.0-d78199a0d53b4ab2bd86e6805a46a38fad783ba2-release-clang-darwin-arm64.tar.gz
     </darwin_release_arm64>
   </build>
+  <build version="2024.2.1.0">
+    <build_number>b116</build_number>
+    <linux_debug_x86>
+      https://s3.us-west-2.amazonaws.com/uploads.dev.yugabyte.com/local-provider-test/2024.2.1.0-b116/yugabyte-2024.2.1.0-e1f9880f523ef6b5f3563ec7c6061319e826b74a-debug-clang17-centos-x86_64.tar.gz
+    </linux_debug_x86>
+    <linux_release_x86>
+      https://s3.us-west-2.amazonaws.com/uploads.dev.yugabyte.com/local-provider-test/2024.2.1.0-b116/yugabyte-2024.2.1.0-e1f9880f523ef6b5f3563ec7c6061319e826b74a-release-clang17-centos-x86_64.tar.gz
+    </linux_release_x86>
+    <darwin_debug_arm64>
+      https://s3.us-west-2.amazonaws.com/uploads.dev.yugabyte.com/local-provider-test/2024.2.1.0-b116/yugabyte-2024.2.1.0-ef5232e8f428fba9d52c6cc2002d46ffd79ab999-debug-clang-darwin-arm64.tar.gz
+    </darwin_debug_arm64>
+    <darwin_release_arm64>
+      https://s3.us-west-2.amazonaws.com/uploads.dev.yugabyte.com/local-provider-test/2024.2.1.0-b116/yugabyte-2024.2.1.0-ef5232e8f428fba9d52c6cc2002d46ffd79ab999-release-clang-darwin-arm64.tar.gz
+    </darwin_release_arm64>
+  </build>
   <build version="2.25.0.0">
     <build_number>b340</build_number>
     <linux_debug_x86>

src/yb/integration-tests/upgrade-tests/pg15_upgrade-test.cc

@@ -780,7 +780,8 @@ TEST_F(Pg15UpgradeTest, Matviews) {
   ASSERT_VECTORS_EQ(result, (decltype(result){1, 2, 3, 4, 5, 6, 7}));
 }
 
-TEST_F(Pg15UpgradeTest, PartitionedTables) {
+// Blocked by #24226
+TEST_F(Pg15UpgradeTest, YB_DISABLE_TEST(PartitionedTables)) {
   // Set up partitioned tables
   ASSERT_OK(ExecuteStatements({
     "CREATE TABLE t_r (v INT, z TEXT, PRIMARY KEY(v ASC)) PARTITION BY RANGE (v)",

src/yb/integration-tests/upgrade-tests/pg15_upgrade_test_base.h

@@ -19,7 +19,7 @@ namespace yb {
 
 class Pg15UpgradeTestBase : public UpgradeTestBase {
  public:
-  Pg15UpgradeTestBase() : UpgradeTestBase(kBuild_2024_2_0_0) {}
+  Pg15UpgradeTestBase() : UpgradeTestBase(kBuild_2024_2_1_0) {}
   virtual ~Pg15UpgradeTestBase() override = default;
 
   void SetUp() override;

src/yb/integration-tests/upgrade-tests/upgrade_test_base.cc

@@ -420,6 +420,14 @@ Status UpgradeTestBase::PerformYsqlMajorCatalogUpgrade() {
     return Status::OK();
   }
 
+  RETURN_NOT_OK(StartYsqlMajorCatalogUpgrade());
+
+  return WaitForYsqlMajorCatalogUpgradeToFinish();
+}
+
+Status UpgradeTestBase::StartYsqlMajorCatalogUpgrade() {
+  LOG_WITH_FUNC(INFO) << "Starting ysql major upgrade";
+
   LOG(INFO) << "Running ysql major catalog version upgrade";
 
   master::StartYsqlMajorCatalogUpgradeRequestPB req;
@@ -432,7 +440,7 @@ Status UpgradeTestBase::PerformYsqlMajorCatalogUpgrade() {
     return StatusFromPB(resp.error().status());
   }
 
-  return WaitForYsqlMajorCatalogUpgradeToFinish();
+  return Status::OK();
 }
 
 Status UpgradeTestBase::WaitForYsqlMajorCatalogUpgradeToFinish() {

src/yb/integration-tests/upgrade-tests/upgrade_test_base.h

@@ -54,6 +54,10 @@ class UpgradeTestBase : public ExternalMiniClusterITestBase {
       ExternalTabletServer& ts, bool wait_for_cluster_to_stabilize = true);
 
   virtual Status PerformYsqlMajorCatalogUpgrade();
+
+  // Only starts the upgrade. WaitForYsqlMajorCatalogUpgradeToFinish should be called to wait for
+  // the upgrade to finish.
+  Status StartYsqlMajorCatalogUpgrade();
   Status WaitForYsqlMajorCatalogUpgradeToFinish();
 
   Status FinalizeUpgrade();
@@ -110,6 +114,7 @@ class UpgradeTestBase : public ExternalMiniClusterITestBase {
 static constexpr auto kBuild_2_20_2_4 = "2.20.2.4";
 static constexpr auto kBuild_2024_1_0_1 = "2024.1.0.1";
 static constexpr auto kBuild_2024_2_0_0 = "2024.2.0.0";
+static constexpr auto kBuild_2024_2_1_0 = "2024.2.1.0";
 static constexpr auto kBuild_2_25_0_0 = "2.25.0.0";
 
 }  // namespace yb