From eadb8801058b95ff403cc04cdcb521d47d2a4c64 Mon Sep 17 00:00:00 2001 From: Minghui Yang Date: Thu, 18 Jan 2024 18:26:31 +0000 Subject: [PATCH] [BACKPORT 2.20][#18507] YSQL: Fix bug: yb_catalog_version() does not exist Summary: A customer reported that backup failed and the reason is that the function `yb_catalog_version()` does not exist. This function is introduced in the migration script `V1__3979__pg_yb_catalog_version.sql`. After debugging I found there is a code bug in the YSQL upgrade code. Specifically: ``` Result GetMajorVersionFromSystemCatalogState(PGConn* pgconn) { int major_version = 0; // Helper macro removing boilerplate. if (VERIFY_RESULT(oneliner_with_result)) { \ ++major_version; \ } else { \ return major_version; \ } // V1: #3979 introducing pg_yb_catalog_version table. INCREMENT_VERSION_OR_RETURN_IT(SystemTableHasRows(pgconn, "pg_yb_catalog_version")) ``` This function tries to determine which version we should start from. For example, it checks the existence of a non-empty table `pg_yb_catalog_version` to decide whether we need to skip V1 migration script or not. The V1 migration script has two blocks: * The first block introduces the table `pg_catalog.pg_yb_catalog_version` * The second block introduces the function `yb_catalog_version()` In a 2.4.x or 2.6.x cluster, `pg_catalog.pg_yb_catalog_version` already exists but `yb_catalog_version()` does not. As a result, the above code will skip V1 migration script and that's why function `yb_catalog_version` isn't introduced. I have verified that any older release < 2.4 (e.g., 2.2.7.0), or any newer release > 2.6 (e.g., 2.8.0.0) do not have this bug. In an older release < 2.4, `pg_catalog.pg_yb_catalog_version` does not exist so the above code will not skip V1 migration script. In a newer release > 2.6, `yb_catalog_version()` also exists so it is correct to skip V1 migration script. The customer cluster was manually fixed by reapplying V1 migration script. I made a fix such that after the normal migration has completed, check whether `yb_catalog_version()` exists or not. If it is missing then apply V1 migration script. Original commit: 55aac07e3d4de9d5e76b16209f784f0f8f67fe41 / D31793 Jira: DB-7466 Test Plan: (1) ./yb_build.sh release --sj --java-test 'org.yb.pgsql.TestYsqlUpgrade#creatingSystemRelsByNonSuperuser' ./yb_build.sh release --sj --java-test 'org.yb.pgsql.TestYsqlUpgrade#creatingSharedRelsCreatesThemEverywhere' ./yb_build.sh release --sj --java-test 'org.yb.pgsql.TestYsqlUpgrade#creatingSharedRelsIsLikeInitdb' ./yb_build.sh release --sj --java-test 'org.yb.pgsql.TestYsqlUpgrade#creatingSystemRelsIsLikeInitdb' ./yb_build.sh release --sj --java-test 'org.yb.pgsql.TestYsqlUpgrade#creatingSystemRelsDontFireTriggers' ./yb_build.sh release --sj --java-test 'org.yb.pgsql.TestYsqlUpgrade#creatingSystemRelsAfterFailure' ./yb_build.sh release --sj --java-test 'org.yb.pgsql.TestYsqlUpgrade#sharedRelsIndexesWork' ./yb_build.sh release --sj --java-test 'org.yb.pgsql.TestYsqlUpgrade#creatingSystemViewsIsLikeInitdb' ./yb_build.sh release --sj --java-test 'org.yb.pgsql.TestYsqlUpgrade#viewReloptionsAreFilteredOnReplace' ./yb_build.sh release --sj --java-test 'org.yb.pgsql.TestYsqlUpgrade#replacingViewKeepsCacheConsistent' ./yb_build.sh release --sj --java-test 'org.yb.pgsql.TestYsqlUpgrade#insertOnConflictWithOidsWorks' ./yb_build.sh release --sj --java-test 'org.yb.pgsql.TestYsqlUpgrade#dmlsUpdatePgCache' ./yb_build.sh release --sj --java-test 'org.yb.pgsql.TestYsqlUpgrade#pinnedObjectsCacheIsUpdated' ./yb_build.sh release --sj --java-test 'org.yb.pgsql.TestYsqlUpgrade#upgradeIsIdempotent' ./yb_build.sh release --sj --java-test 'org.yb.pgsql.TestYsqlUpgrade#upgradeIsIdempotentSingleConn' ./yb_build.sh release --sj --java-test 'org.yb.pgsql.TestYsqlUpgrade#migratingIsEquivalentToReinitdb' ./yb_build.sh release --sj --java-test 'org.yb.pgsql.TestYsqlUpgrade#migrationInGeoPartitionedSetup' ./yb_build.sh release --sj --java-test 'org.yb.pgsql.TestYsqlUpgrade#migrationFilenameComment' ./yb_build.sh release --sj --java-test 'org.yb.pgsql.TestYsqlUpgrade#invalidUpgradeActions' (2) Download old release packages such as yugabyte-2.2.7.0 yugabyte-2.4.0.0 yugabyte-2.4.8.0 yugabyte-2.6.0.0, yugabyte-2.6.20.0 yugabyte-2.8.0.0 yugabyte-2.8.12.0 Run the following test manually against each of the old releases. For example: ``` ./bin/yb-ctl create --timeout-yb-admin-sec 180 --rf 1 --master_flags initial_sys_catalog_snapshot_path=$HOME/tmp/yugabyte-2.4.0.0/share/initial_sys_catalog_snapshot ./build/latest/bin/yb-admin --timeout_ms=720000 upgrade_ysql ``` Look at yb-tserver.INFO ``` W0118 17:16:55.246408 11555 ysql_upgrade.cc:438] Function yb_catalog_version is missing in template1 I0118 17:16:55.246526 11555 ysql_upgrade.cc:473] template1: applying migration 'V1__3979__pg_yb_catalog_version.sql' I0118 17:16:55.246542 11555 ysql_upgrade.cc:481] Found pg_global in migration file V1__3979__pg_yb_catalog_version.sql when applying to template1 I0118 17:16:55.341861 11555 ysql_upgrade.cc:517] template1: migration successfully applied without version bump W0118 17:16:55.478569 11555 ysql_upgrade.cc:438] Function yb_catalog_version is missing in template0 I0118 17:16:55.478693 11555 ysql_upgrade.cc:473] template0: applying migration 'V1__3979__pg_yb_catalog_version.sql' I0118 17:16:55.563812 11555 ysql_upgrade.cc:517] template0: migration successfully applied without version bump W0118 17:16:55.700487 11555 ysql_upgrade.cc:438] Function yb_catalog_version is missing in postgres I0118 17:16:55.700606 11555 ysql_upgrade.cc:473] postgres: applying migration 'V1__3979__pg_yb_catalog_version.sql' I0118 17:16:55.783318 11555 ysql_upgrade.cc:517] postgres: migration successfully applied without version bump W0118 17:16:55.802189 11555 ysql_upgrade.cc:438] Function yb_catalog_version is missing in yugabyte I0118 17:16:55.802299 11555 ysql_upgrade.cc:473] yugabyte: applying migration 'V1__3979__pg_yb_catalog_version.sql' I0118 17:16:55.826416 11555 ysql_upgrade.cc:517] yugabyte: migration successfully applied without version bump W0118 17:16:55.845325 11555 ysql_upgrade.cc:438] Function yb_catalog_version is missing in system_platform I0118 17:16:55.845443 11555 ysql_upgrade.cc:473] system_platform: applying migration 'V1__3979__pg_yb_catalog_version.sql' I0118 17:16:55.865423 11555 ysql_upgrade.cc:517] system_platform: migration successfully applied without version bump ``` Rerun the upgrade command ``` ./build/latest/bin/yb-admin --timeout_ms=720000 upgrade_ysql ``` Look at the yb-tserver.INFO again: ``` I0118 17:34:51.982327 11558 ysql_upgrade.cc:443] Found function yb_catalog_version in template1 I0118 17:34:51.985905 11558 ysql_upgrade.cc:443] Found function yb_catalog_version in template0 I0118 17:34:51.989349 11558 ysql_upgrade.cc:443] Found function yb_catalog_version in postgres I0118 17:34:51.992571 11558 ysql_upgrade.cc:443] Found function yb_catalog_version in yugabyte I0118 17:34:51.997953 11558 ysql_upgrade.cc:443] Found function yb_catalog_version in system_platform ``` Reviewers: jason, tverona Reviewed By: jason Subscribers: yql Tags: #jenkins-ready Differential Revision: https://phorge.dev.yugabyte.com/D31898 --- src/yb/yql/pgwrapper/ysql_upgrade.cc | 26 ++++++++++++++++++++++++-- src/yb/yql/pgwrapper/ysql_upgrade.h | 3 ++- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/src/yb/yql/pgwrapper/ysql_upgrade.cc b/src/yb/yql/pgwrapper/ysql_upgrade.cc index 5492c79141e4..ee5bc2fe43c9 100644 --- a/src/yb/yql/pgwrapper/ysql_upgrade.cc +++ b/src/yb/yql/pgwrapper/ysql_upgrade.cc @@ -428,12 +428,29 @@ Status YsqlUpgradeHelper::Upgrade() { } } + // Fix for https://github.com/yugabyte/yugabyte-db/issues/18507: + // This bug only shows up when upgrading from 2.4.x or 2.6.x, in these + // two releases the table pg_yb_catalog_version exists but the function + // yb_catalog_version does not exist. However we have skipped V1 because + // SystemTableHasRows(pgconn, "pg_yb_catalog_version") returns true. + for (auto& entry : databases) { + auto conn = VERIFY_RESULT(entry->GetConnection()); + if (!VERIFY_RESULT(FunctionExists(conn.get(), "yb_catalog_version"))) { + LOG(WARNING) << "Function yb_catalog_version is missing in " << entry->database_name_; + // Run V1 migration script to introduce function "yb_catalog_version". + const Version version = {0, 0}; + RETURN_NOT_OK(MigrateOnce(entry.get(), &version)); + } else { + LOG(INFO) << "Found function yb_catalog_version in " << entry->database_name_; + } + } + return Status::OK(); } -Status YsqlUpgradeHelper::MigrateOnce(DatabaseEntry* db_entry) { +Status YsqlUpgradeHelper::MigrateOnce(DatabaseEntry* db_entry, const Version* historical_version) { const auto& db_name = db_entry->database_name_; - const auto& version = db_entry->version_; + const auto& version = historical_version ? *historical_version : db_entry->version_; auto pgconn = VERIFY_RESULT(db_entry->GetConnection()); @@ -485,6 +502,11 @@ Status YsqlUpgradeHelper::MigrateOnce(DatabaseEntry* db_entry) { catalog_version_migration_applied_ = true; } + if (historical_version) { + LOG(INFO) << db_name << ": migration successfully applied without version bump"; + return Status::OK(); + } + RETURN_NOT_OK_PREPEND( pgconn->ExecuteFormat( WrapSystemDml( diff --git a/src/yb/yql/pgwrapper/ysql_upgrade.h b/src/yb/yql/pgwrapper/ysql_upgrade.h index e0068b04a9d2..ea6485101eb6 100644 --- a/src/yb/yql/pgwrapper/ysql_upgrade.h +++ b/src/yb/yql/pgwrapper/ysql_upgrade.h @@ -49,7 +49,8 @@ class YsqlUpgradeHelper { Result> MakeDatabaseEntry(std::string database_name); // Migrate a given database to the next version, updating it in the given database entry. - Status MigrateOnce(DatabaseEntry* db_entry); + // If historical_version isn't nullptr, use it to override db_entry->version_. + Status MigrateOnce(DatabaseEntry* db_entry, const Version* historical_version = nullptr); const HostPort ysql_proxy_addr_;