[#20300] YSQL: Fix CREATE INDEX failure in upgrading to perdb catalog…

… version mode

Summary:
To reproduce the bug, create a universe with `ysql_enable_db_catalog_version_mode=true`
and `allowed_preview_flags_csv=ysql_enable_db_catalog_version_mode`.

Do NOT execute the following
```
SET yb_non_ddl_txn_for_sys_tables_allowed=true;
SELECT yb_fix_catalog_version_table(true);
```
Run the next test
```
yugabyte=# create table t(id int);
CREATE TABLE

yugabyte=# insert into t values(1);
INSERT 0 1

yugabyte=# CREATE TABLE tempTable AS SELECT * FROM t limit 1000000;
SELECT 1

yugabyte=# CREATE INDEX idx2 ON tempTable(id);
NOTICE:  Retrying wait for backends catalog version: Requested catalog version is too high: req
version 2, master version 0
NOTICE:  Retrying wait for backends catalog version: Requested catalog version is too high: req
version 2, master version 0
NOTICE:  Retrying wait for backends catalog version: Requested catalog version is too high: req
version 2, master version 0
NOTICE:  Retrying wait for backends catalog version: Requested catalog version is too high: req
version 2, master version 0
ERROR:  Requested catalog version is too high: req version 2, master version 0
```

This configuration has an mismatch between --ysql_enable_db_catalog_version_mode
and pg_yb_catalog_version: --ysql_enable_db_catalog_version_mode=true but
pg_yb_catalog_version is still in global-catalog version mode (only has one
row for database template1). This is a special configuration that can legitimately
happen during cluster upgrade to a new release that has the per-database catalog
version mode on by default. The new binary executables are upgraded first which
will have --ysql_enable_db_catalog_version_mode=true, but the YSQL migration script
will be executed at a later time in the finalization phase of the upgrade
process. There is a tryout phase before the finalization phase to allow possible
rolling back of the upgrade. The tryout phase can last for an extended period of
time (e.g., 1 month).

Therefore we need to support DDL statements during the tryout phase when the
gflag is on but the table pg_yb_catalog_version isn't upgraded yet. However we
should still advise customers not to run any DDL statements during the
finalization phase which includes running the YSQL migration scripts (at present
this is not enforced).

I added a variable `catalog_version_table_in_perdb_mode_` that is used to cache
the state of pg_yb_catalog_version: whether it has been upgraded to have one row
per database. The intention is to only support rolling upgrade to per-database
catalog version mode (not downgrade back to global catalog version mode).
Therefore once set to true, the variable `catalog_version_table_in_perdb_mode_`
is never reset back to false. This variable is introduced in both yb-master and
yb-tserver.

Normally PG request carries the catalog version mode as part of the RPC
arguments (`ysql_catalog_version` vs `ysql_db_catalog_version`) and yb-tserver
will respect that.

Before this fix, when no PG request is available, yb-tserver only uses
--ysql_enable_db_catalog_version_mode to tell which catalog version mode to use.
After this fix, only when both the gflag --ysql_enable_db_catalog_version_mode
and `catalog_version_table_in_perdb_mode_` are true, yb-tserver operates in
per-database catalog version mode. This fixes the CREATE INDEX failure.

The variable `catalog_version_table_in_perdb_mode_` is also stored in shared
memory to allow fast PG access to improve the performance of
`YBIsDBCatalogVersionMode()` by replacing `YbGetNumberOfDatabases() > 1)` so we
avoid a local RPC call during the tryout phase.
Jira: DB-9266

Test Plan: ./yb_build.sh --cxx-test pg_catalog_version-test

Reviewers: jason, tverona

Reviewed By: jason

Subscribers: ybase, yql, bogdan

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

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

@@ -639,7 +639,7 @@ YBCanEnableDBCatalogVersionMode()
 	 * upgrade, the pg_yb_catalog_version is transactionally updated
 	 * to have one row per database.
 	 */
-	return (YbGetNumberOfDatabases() > 1);
+	return YbCatalogVersionTableInPerdbMode();
 }
 
 /*
@@ -4020,6 +4020,13 @@ uint32_t YbGetNumberOfDatabases()
 	return num_databases;
 }
 
+bool YbCatalogVersionTableInPerdbMode()
+{
+	bool perdb_mode = false;
+	HandleYBStatus(YBCCatalogVersionTableInPerdbMode(&perdb_mode));
+	return perdb_mode;
+}
+
 static bool yb_is_batched_execution = false;
 
 bool YbIsBatchedExecution()

src/postgres/src/include/pg_yb_utils.h

@@ -907,6 +907,7 @@ void YbSetCatalogCacheVersion(YBCPgStatement handle, uint64_t version);
 
 uint64_t YbGetSharedCatalogVersion();
 uint32_t YbGetNumberOfDatabases();
+bool YbCatalogVersionTableInPerdbMode();
 
 /*
  * This function maps the user intended row-level lock policy i.e., "pg_wait_policy" of

src/yb/master/catalog_manager.cc

@@ -10355,7 +10355,7 @@ Status CatalogManager::GetYsqlAllDBCatalogVersionsImpl(DbOidToCatalogVersionMap*
 Status CatalogManager::GetYsqlAllDBCatalogVersions(
     bool use_cache, DbOidToCatalogVersionMap* versions, uint64_t* fingerprint) {
   DCHECK(FLAGS_ysql_enable_db_catalog_version_mode);
-  if (use_cache) {
+  if (use_cache && catalog_version_table_in_perdb_mode_) {
     SharedLock lock(heartbeat_pg_catalog_versions_cache_mutex_);
     // We expect that the only caller uses this cache is the heartbeat service.
     // It is ok for heartbeat_pg_catalog_versions_cache_ to be empty: the
@@ -10381,6 +10381,9 @@ Status CatalogManager::GetYsqlAllDBCatalogVersions(
     *fingerprint = FingerprintCatalogVersions<DbOidToCatalogVersionMap>(*versions);
     VLOG_WITH_FUNC(2) << "databases: " << versions->size() << ", fingerprint: " << *fingerprint;
   }
+  if (versions->size() > 1) {
+    catalog_version_table_in_perdb_mode_ = true;
+  }
   return Status::OK();
 }
 

src/yb/master/catalog_manager.h

@@ -689,6 +689,9 @@ class CatalogManager : public tserver::TabletPeerLookupIf,
       EXCLUDES(heartbeat_pg_catalog_versions_cache_mutex_);
   Status GetYsqlDBCatalogVersion(
       uint32_t db_oid, uint64_t* catalog_version, uint64_t* last_breaking_version) override;
+  bool catalog_version_table_in_perdb_mode() const override {
+    return catalog_version_table_in_perdb_mode_;
+  }
 
   Status InitializeTransactionTablesConfig(int64_t term);
 
@@ -3215,6 +3218,20 @@ class CatalogManager : public tserver::TabletPeerLookupIf,
   std::atomic<bool> pg_catalog_versions_bg_task_running_ = {false};
   rpc::ScheduledTaskTracker refresh_ysql_pg_catalog_versions_task_;
 
+  // For per-database catalog version mode upgrade support: when the gflag
+  // --ysql_enable_db_catalog_version_mode is true, whether the table
+  // pg_yb_catalog_version has been upgraded to have one row per database.
+  // During upgrade the binaries are installed first but before YSQL migration
+  // script is run pg_yb_catalog_version only has one row for template1.
+  // YB Note:
+  // (1) Each time we read the entire pg_yb_catalog_version table if the number
+  // of rows is > 1 we assume that the table has exactly one row per database.
+  // (2) This is only used to support per-database catalog version mode upgrade.
+  // Once set it is never reset back to false. It is an error to change
+  // pg_yb_catalog_version back to global catalog version mode when
+  // --ysql_enable_db_catalog_version_mode=true.
+  std::atomic<bool> catalog_version_table_in_perdb_mode_ = false;
+
   // mutex on heartbeat_pg_catalog_versions_cache_
   mutable MutexType heartbeat_pg_catalog_versions_cache_mutex_;
   std::optional<DbOidToCatalogVersionMap> heartbeat_pg_catalog_versions_cache_

src/yb/master/catalog_manager_if.h

@@ -126,6 +126,7 @@ class CatalogManagerIf {
       uint64_t* fingerprint) = 0;
   virtual Status GetYsqlDBCatalogVersion(
       uint32_t db_oid, uint64_t* catalog_version, uint64_t* last_breaking_version) = 0;
+  virtual bool catalog_version_table_in_perdb_mode() const = 0;
 
   virtual Status GetClusterConfig(GetMasterClusterConfigResponsePB* resp) = 0;
   virtual Status GetClusterConfig(SysClusterConfigEntryPB* config) = 0;

src/yb/master/master_tserver.cc

@@ -148,6 +148,10 @@ void MasterTabletServer::get_ysql_db_catalog_version(uint32_t db_oid,
   }
 }
 
+bool MasterTabletServer::catalog_version_table_in_perdb_mode() const {
+  return master_->catalog_manager()->catalog_version_table_in_perdb_mode();
+}
+
 tserver::TServerSharedData& MasterTabletServer::SharedObject() {
   return master_->shared_object();
 }

src/yb/master/master_tserver.h

@@ -58,6 +58,7 @@ class MasterTabletServer : public tserver::TabletServerIf,
   void get_ysql_db_catalog_version(uint32_t db_oid,
                                    uint64_t* current_version,
                                    uint64_t* last_breaking_version) const override;
+  bool catalog_version_table_in_perdb_mode() const override;
 
   Status get_ysql_db_oid_to_cat_version_info_map(
       const tserver::GetTserverCatalogVersionInfoRequestPB& req,

src/yb/master/ysql_backends_manager.cc

@@ -132,7 +132,8 @@ Status YsqlBackendsManager::WaitForYsqlBackendsCatalogVersion(
     Status s;
     // TODO(jason): using the gflag to determine per-db mode may not work for initdb, so make sure
     // to handle that case if initdb ever goes through this codepath.
-    if (FLAGS_ysql_enable_db_catalog_version_mode) {
+    if (FLAGS_ysql_enable_db_catalog_version_mode &&
+        master_->catalog_manager_impl()->catalog_version_table_in_perdb_mode()) {
       s = master_->catalog_manager_impl()->GetYsqlDBCatalogVersion(
           db_oid, &master_version, nullptr /* last_breaking_version */);
     } else {

src/yb/tserver/tablet_server.cc

@@ -912,6 +912,10 @@ void TabletServer::SetYsqlDBCatalogVersions(
       std::make_pair(db_oid, CatalogVersionInfo({.current_version = new_version,
                                                  .last_breaking_version = new_breaking_version,
                                                  .shm_index = -1})));
+    if (ysql_db_catalog_version_map_.size() > 1) {
+      catalog_version_table_in_perdb_mode_ = true;
+      shared_object().SetCatalogVersionTableInPerdbMode();
+    }
     bool row_inserted = it.second;
     bool row_updated = false;
     int shm_index = -1;

src/yb/tserver/tablet_server.h

@@ -243,6 +243,11 @@ class TabletServer : public DbServerBase, public TabletServerIf {
     }
   }
 
+  bool catalog_version_table_in_perdb_mode() const override {
+    std::lock_guard l(lock_);
+    return catalog_version_table_in_perdb_mode_;
+  }
+
   Status get_ysql_db_oid_to_cat_version_info_map(
       const tserver::GetTserverCatalogVersionInfoRequestPB& req,
       tserver::GetTserverCatalogVersionInfoResponsePB* resp) const override;
@@ -403,6 +408,8 @@ class TabletServer : public DbServerBase, public TabletServerIf {
   uint64_t ysql_catalog_version_ = 0;
   uint64_t ysql_last_breaking_catalog_version_ = 0;
   tserver::DbOidToCatalogVersionInfoMap ysql_db_catalog_version_map_;
+  // See same variable comments in CatalogManager.
+  bool catalog_version_table_in_perdb_mode_ = false;
 
   // Fingerprint of the catalog versions map.
   std::atomic<std::optional<uint64_t>> catalog_versions_fingerprint_;

src/yb/tserver/tablet_server_interface.h

@@ -55,6 +55,7 @@ class TabletServerIf : public LocalTabletServer {
   virtual void get_ysql_db_catalog_version(uint32_t db_oid,
                                            uint64_t* current_version,
                                            uint64_t* last_breaking_version) const = 0;
+  virtual bool catalog_version_table_in_perdb_mode() const = 0;
 
   virtual Status get_ysql_db_oid_to_cat_version_info_map(
       const tserver::GetTserverCatalogVersionInfoRequestPB& req,

src/yb/tserver/tablet_service.cc

@@ -1977,7 +1977,8 @@ void TabletServiceAdminImpl::WaitForYsqlBackendsCatalogVersion(
       [catalog_version, database_oid, this, &ts_catalog_version]() -> Result<bool> {
         // TODO(jason): using the gflag to determine per-db mode may not work for initdb, so make
         // sure to handle that case if initdb ever goes through this codepath.
-        if (FLAGS_ysql_enable_db_catalog_version_mode) {
+        if (FLAGS_ysql_enable_db_catalog_version_mode &&
+            server_->catalog_version_table_in_perdb_mode()) {
           server_->get_ysql_db_catalog_version(
               database_oid, &ts_catalog_version, nullptr /* last_breaking_catalog_version */);
         } else {

src/yb/tserver/tserver_shared_mem.h

@@ -85,6 +85,14 @@ class TServerSharedData {
     return db_catalog_versions_[index].load(std::memory_order_acquire);
   }
 
+  void SetCatalogVersionTableInPerdbMode() {
+    catalog_version_table_in_perdb_mode_.store(true, std::memory_order_release);
+  }
+
+  bool catalog_version_table_in_perdb_mode() const {
+    return catalog_version_table_in_perdb_mode_.load(std::memory_order_acquire);
+  }
+
   void SetPostgresAuthKey(uint64_t auth_key) {
     postgres_auth_key_ = auth_key;
   }
@@ -112,6 +120,8 @@ class TServerSharedData {
   unsigned char tserver_uuid_[16]; // Tserver UUID is stored as raw bytes.
 
   std::atomic<uint64_t> db_catalog_versions_[kMaxNumDbCatalogVersions] = {0};
+  // See same variable comments in CatalogManager.
+  std::atomic<bool> catalog_version_table_in_perdb_mode_{false};
 };
 
 YB_STRONGLY_TYPED_BOOL(Create);

src/yb/yql/pggate/pggate.cc

@@ -1906,6 +1906,10 @@ Result<uint32_t> PgApiImpl::GetNumberOfDatabases() {
   return info.num_entries();
 }
 
+Result<bool> PgApiImpl::CatalogVersionTableInPerdbMode() {
+  return tserver_shared_object_->catalog_version_table_in_perdb_mode();
+}
+
 uint64_t PgApiImpl::GetSharedAuthKey() const {
   return tserver_shared_object_->postgres_auth_key();
 }

src/yb/yql/pggate/pggate.h

@@ -166,6 +166,7 @@ class PgApiImpl {
 
   Result<uint64_t> GetSharedCatalogVersion(std::optional<PgOid> db_oid = std::nullopt);
   Result<uint32_t> GetNumberOfDatabases();
+  Result<bool> CatalogVersionTableInPerdbMode();
   uint64_t GetSharedAuthKey() const;
   const unsigned char *GetLocalTserverUuid() const;
 

src/yb/yql/pggate/ybc_pggate.cc

@@ -1633,6 +1633,10 @@ YBCStatus YBCGetNumberOfDatabases(uint32_t* num_databases) {
   return ExtractValueFromResult(pgapi->GetNumberOfDatabases(), num_databases);
 }
 
+YBCStatus YBCCatalogVersionTableInPerdbMode(bool* perdb_mode) {
+  return ExtractValueFromResult(pgapi->CatalogVersionTableInPerdbMode(), perdb_mode);
+}
+
 uint64_t YBCGetSharedAuthKey() {
   return pgapi->GetSharedAuthKey();
 }

src/yb/yql/pggate/ybc_pggate.h

@@ -77,6 +77,10 @@ YBCStatus YBCGetSharedDBCatalogVersion(
 // catalog version mode is enabled.
 YBCStatus YBCGetNumberOfDatabases(uint32_t* num_databases);
 
+// Return true if the pg_yb_catalog_version table has been updated to
+// have one row per database.
+YBCStatus YBCCatalogVersionTableInPerdbMode(bool* perdb_mode);
+
 // Return auth_key to the local tserver's postgres authentication key stored in shared memory.
 uint64_t YBCGetSharedAuthKey();
 

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

@@ -831,15 +831,20 @@ TEST_F(PgCatalogVersionTest, FixCatalogVersionTable) {
   CHECK_EQ(versions.size(), 1);
   ASSERT_OK(CheckMatch(versions.begin()->second, kNewCatalogVersion));
 
-  // For a new connection, although --ysql_enable_db_catalog_version_mode is still
-  // true, the fact that the table pg_yb_catalog_version has only one row prevents
-  // a new connection to enter per-database catalog version mode. Verify that we
-  // can make a new connection to database "yugabyte".
-  ASSERT_RESULT(ConnectToDB("yugabyte"));
+  // Once a tserver enters per-database catalog version mode it remains so.
+  // It is an error to change pg_yb_catalog_version back to global catalog
+  // version mode when --ysql_enable_db_catalog_version_mode=true.
+  // Verify that we can not make a new connection to database "yugabyte"
+  // in this error state.
+  const auto yugabyte_db_oid = ASSERT_RESULT(GetDatabaseOid(&conn_yugabyte, kYugabyteDatabase));
+  auto status = ResultToStatus(ConnectToDB("yugabyte"));
+  ASSERT_TRUE(status.IsNetworkError()) << status;
+  ASSERT_STR_CONTAINS(status.ToString(),
+                      Format("catalog version for database $0 was not found", yugabyte_db_oid));
+  ASSERT_STR_CONTAINS(status.ToString(), "Database might have been dropped by another user");
 
-  // We can also make a new connection to database "template1" but the fact that
-  // now it is the only database that has a row in pg_yb_catalog_version table is
-  // not relevant.
+  // We can only make a new connection to database "template1" because now it
+  // is the only database that has a row in pg_yb_catalog_version table.
   conn_template1 = ASSERT_RESULT(ConnectToDB("template1"));
 
   // Sync up pg_yb_catalog_version with pg_database.
@@ -1206,6 +1211,19 @@ TEST_F(PgCatalogVersionTest, RemoveRelCacheInitFiles) {
   RemoveRelCacheInitFilesHelper(false /* per_database_mode */);
 }
 
+// This test that YSQL can execute DDL statements when the gflag
+// --ysql_enable_db_catalog_version_mode is on but the pg_yb_catalog_version
+// table isn't updated to have one row per database.
+TEST_F(PgCatalogVersionTest, SimulateTryoutPhaseInUpgrade) {
+  auto conn_yugabyte = ASSERT_RESULT(Connect());
+  ASSERT_OK(PrepareDBCatalogVersion(&conn_yugabyte, false /* per_database_mode */));
+  RestartClusterWithDBCatalogVersionMode();
+  conn_yugabyte = ASSERT_RESULT(Connect());
+  ASSERT_OK(conn_yugabyte.Execute("CREATE TABLE t(id INT)"));
+  ASSERT_OK(conn_yugabyte.ExecuteFormat("CREATE INDEX idx ON t(id)"));
+  ASSERT_OK(conn_yugabyte.Execute("ALTER ROLE yugabyte SUPERUSER"));
+}
+
 TEST_F(PgCatalogVersionTest, SqlCrossDBLoadWithDDL) {
 
   const std::vector<std::vector<string>> ddlLists = {