Skip to content

Commit

Permalink
[BACKPORT 2.20][#18507] YSQL: Fix bug: yb_catalog_version() does not …
Browse files Browse the repository at this point in the history
…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<int> 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: 55aac07 / 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
  • Loading branch information
myang2021 committed Jan 24, 2024
1 parent cd9abbf commit eadb880
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 3 deletions.
26 changes: 24 additions & 2 deletions src/yb/yql/pgwrapper/ysql_upgrade.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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());

Expand Down Expand Up @@ -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(
Expand Down
3 changes: 2 additions & 1 deletion src/yb/yql/pgwrapper/ysql_upgrade.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ class YsqlUpgradeHelper {
Result<std::unique_ptr<DatabaseEntry>> 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_;

Expand Down

0 comments on commit eadb880

Please sign in to comment.