Skip to content

Commit

Permalink
[#22184] YSQL: Fix DDL Atomicity drop table
Browse files Browse the repository at this point in the history
Summary:
The the fix for #22095 (c8c2616/D34431) has caused the drop table DDL to return to the user as soon as the table is marked as DELETING. Prior to the change we used to wait till the table reached the state DELETED. This causes a bunch of test failures when atomic DDL is enabled.

The fix removed the DdlTransactionState early causing the early exit from drop table DDL.

The primary issue is from `CatalogManager::ShouldDeleteTable`. This function returns false when the table has any tasks. DDL atomicity runs the `TableSchemaVerificationTask` which causes this function to return false, causing the table to be lazily deleted in the async background task. The check is only meant to wait for DeleteReplica RPCs to complete, so a better fix is to change this check to be more specific and keep the original cleanup logic.
Jira: DB-11111

Test Plan:
Jenkins
Verified by merging with #D30471

Reviewers: myang

Reviewed By: myang

Subscribers: ybase

Differential Revision: https://phorge.dev.yugabyte.com/D34601
  • Loading branch information
hari90 committed Apr 29, 2024
1 parent 584f3e3 commit c86ec8d
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 7 deletions.
5 changes: 3 additions & 2 deletions src/yb/master/catalog_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6718,8 +6718,9 @@ bool CatalogManager::ShouldDeleteTable(const TableInfoPtr& table) {
<< "Table Information is null. Skipping updating its state to DELETED.";
return false;
}
if (table->HasTasks()) {
VLOG_WITH_PREFIX_AND_FUNC(2) << table->ToString() << " has tasks";
// Wait for all the replica delete tasks that were sent to all peers to complete.
if (table->HasTasks(server::MonitoredTaskType::kDeleteReplica)) {
VLOG_WITH_PREFIX_AND_FUNC(2) << table->ToString() << " has more replica delete tasks";
return false;
}
bool hide_only;
Expand Down
6 changes: 1 addition & 5 deletions src/yb/master/ysql_ddl_handler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -406,11 +406,7 @@ Status CatalogManager::YsqlDdlTxnDropTableHelper(const YsqlTableDdlTxnState txn_
dtreq.mutable_table()->set_table_name(table->name());
dtreq.mutable_table()->set_table_id(table->id());
dtreq.set_is_index_table(table->is_index());
RETURN_NOT_OK(DeleteTableInternal(&dtreq, &dtresp, nullptr /* rpc */, txn_data.epoch));

RemoveDdlTransactionState(table->id(), {txn_data.ddl_txn_id});

return Status::OK();
return DeleteTableInternal(&dtreq, &dtresp, nullptr /* rpc */, txn_data.epoch);
}

Status CatalogManager::WaitForDdlVerificationToFinish(
Expand Down

0 comments on commit c86ec8d

Please sign in to comment.