Skip to content

Commit

Permalink
[#25237] YSQL: PG15 upgrade: Block Create DB during major upgrade
Browse files Browse the repository at this point in the history
Summary:
Block the creation of new databases during for the entire ysql major upgrade process.
Prior to this change the CreateNamespace call would only fail during the ysql major catalog upgrade phase, but the CREATE DATABASE SQL statement would fail trying to modify the system catalog tables. This happens because the ysql major catalog upgrade is just one of the steps in the upgrade, and we block the catalog for the entire upgrade. Since CREATE DATABASE is not yet an atomic DDL, this skips the cleanup of the partially created keyspace and cause further creation attempts of the same database to fail.

- Unified the error message for all the DDLs that are blocked during the major upgrade.
- Removed `VerifyDroppedColumnsForUpgrade` since it is never invoked. `CatalogManager::AlterTable` is completely skipped during the upgrade.
- Removed some duplicate checks from `CatalogManager::DeleteYsqlDatabase` since its caller `CatalogManager::DoDeleteNamespace` performs the exact same checks already.

Fixes #25237
Jira: DB-14425

Test Plan: YsqlMajorUpgradeDdlBlockingTest.CreateAndDropDBs

Reviewers: telgersma, smishra

Reviewed By: telgersma

Subscribers: yql, ybase

Differential Revision: https://phorge.dev.yugabyte.com/D41330
  • Loading branch information
hari90 committed Jan 22, 2025
1 parent b307817 commit 4f5b411
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 148 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -226,10 +226,9 @@ TEST_F(YsqlMajorUpgradeDdlBlockingTest, CreateAndDropDBs) {
ASSERT_OK(RestartAllMastersInCurrentVersion(kNoDelayBetweenNodes));

auto validate_db_ddls_fail = [this] {
// TODO: Enable after CreateDatabase blocking is properly implemented.
// ASSERT_NOK_STR_CONTAINS(
// ExecuteStatement("CREATE DATABASE new_db2"),
// "No new namespaces can be created during a major YSQL upgrade");
ASSERT_NOK_STR_CONTAINS(
ExecuteStatement("CREATE DATABASE new_db2"),
"No new namespaces can be created during a major YSQL upgrade");
ASSERT_NOK_STR_CONTAINS(ExecuteStatement("DROP DATABASE new_db1"), kExpectedDdlError);
};

Expand Down
189 changes: 59 additions & 130 deletions src/yb/master/catalog_manager.cc

Large diffs are not rendered by default.

2 changes: 0 additions & 2 deletions src/yb/master/catalog_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -2923,8 +2923,6 @@ class CatalogManager : public CatalogManagerIf, public SnapshotCoordinatorContex
bool is_colocated_via_database, const NamespaceId& namespace_id,
const NamespaceName& namespace_name, const TableInfoPtr& indexed_table) REQUIRES(mutex_);

bool IsYsqlMajorCatalogUpgradeInProgress() const;

bool SkipCatalogVersionChecks() override;

// Should be bumped up when tablet locations are changed.
Expand Down
6 changes: 1 addition & 5 deletions src/yb/master/ysql/ysql_initdb_major_upgrade_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -175,10 +175,6 @@ Status YsqlInitDBAndMajorUpgradeHandler::RollbackYsqlMajorCatalogVersion(const L
return sync.Wait();
}

bool YsqlInitDBAndMajorUpgradeHandler::IsYsqlMajorCatalogUpgradeInProgress() const {
return !IsYsqlMajorCatalogUpgradeDone().done();
}

Result<YsqlMajorCatalogUpgradeState>
YsqlInitDBAndMajorUpgradeHandler::GetYsqlMajorCatalogUpgradeState() const {
const auto state = ysql_catalog_config_.GetMajorCatalogUpgradeState();
Expand Down Expand Up @@ -207,7 +203,7 @@ YsqlInitDBAndMajorUpgradeHandler::GetYsqlMajorCatalogUpgradeState() const {
bool YsqlInitDBAndMajorUpgradeHandler::IsWriteToCatalogTableAllowed(
const TableId& table_id, bool is_forced_update) const {
// During the upgrade only allow special updates to the catalog.
if (IsYsqlMajorUpgradeInProgress()) {
if (IsMajorUpgradeInProgress()) {
return is_forced_update;
}

Expand Down
4 changes: 1 addition & 3 deletions src/yb/master/ysql/ysql_initdb_major_upgrade_handler.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,7 @@ class YsqlInitDBAndMajorUpgradeHandler {
// The upgrade is considered to have started when the yb-master leader has upgraded to a new major
// catalog version.
// The upgrade is completed after it has been finalized.
bool IsYsqlMajorUpgradeInProgress() const { return ysql_major_upgrade_in_progress_; }

bool IsYsqlMajorCatalogUpgradeInProgress() const;
bool IsMajorUpgradeInProgress() const { return ysql_major_upgrade_in_progress_; }

Result<YsqlMajorCatalogUpgradeState> GetYsqlMajorCatalogUpgradeState() const;

Expand Down
6 changes: 3 additions & 3 deletions src/yb/master/ysql/ysql_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ Status YsqlManager::SetInitDbDone(const LeaderEpoch& epoch) {
return ysql_catalog_config_.SetInitDbDone(Status::OK(), epoch);
}

bool YsqlManager::IsYsqlMajorCatalogUpgradeInProgress() const {
return ysql_initdb_and_major_upgrade_helper_->IsYsqlMajorCatalogUpgradeInProgress();
bool YsqlManager::IsMajorUpgradeInProgress() const {
return ysql_initdb_and_major_upgrade_helper_->IsMajorUpgradeInProgress();
}

uint64_t YsqlManager::GetYsqlCatalogVersion() const { return ysql_catalog_config_.GetVersion(); }
Expand Down Expand Up @@ -155,7 +155,7 @@ Result<TableId> YsqlManager::GetVersionSpecificCatalogTableId(

// Use the current version of the catalog if it is updatable, since if the current version is
// available in the MONITORING phase, it can be deleted by a Rollback.
if (!ysql_initdb_and_major_upgrade_helper_->IsYsqlMajorUpgradeInProgress()) {
if (!IsMajorUpgradeInProgress()) {
return current_table_id;
}

Expand Down
2 changes: 1 addition & 1 deletion src/yb/master/ysql/ysql_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ class YsqlManager : public YsqlManagerIf {

Status SetInitDbDone(const LeaderEpoch& epoch);

bool IsYsqlMajorCatalogUpgradeInProgress() const;
bool IsMajorUpgradeInProgress() const;

void HandleNewTableId(const TableId& table_id);

Expand Down

0 comments on commit 4f5b411

Please sign in to comment.