Skip to content

Commit

Permalink
[#25223] DocDB: Fix table exists for deleted table
Browse files Browse the repository at this point in the history
Summary:
After recent commit c81b852/D40502 TableExists started to return true for recently deleted table.
Fixed by adding check to master.
Jira: DB-14415

Test Plan: YBBackupTest.TestYCQLKeyspaceBackupWithoutIndexes

Reviewers: hsunder

Reviewed By: hsunder

Subscribers: ybase

Tags: #jenkins-ready

Differential Revision: https://phorge.dev.yugabyte.com/D40576
  • Loading branch information
spolitov committed Dec 12, 2024
1 parent 572f28b commit 4593e62
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 12 deletions.
1 change: 1 addition & 0 deletions src/yb/client/client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2599,6 +2599,7 @@ Result<std::vector<YBTableName>> YBClient::ListTables(

for (int i = 0; i < resp.tables_size(); i++) {
const ListTablesResponsePB_TableInfo& table_info = resp.tables(i);
VLOG_WITH_FUNC(4) << "Table: " << AsString(table_info);
DCHECK(table_info.has_namespace_());
DCHECK(table_info.namespace_().has_name());
DCHECK(table_info.namespace_().has_id());
Expand Down
16 changes: 8 additions & 8 deletions src/yb/master/catalog_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5561,15 +5561,15 @@ Status CatalogManager::RemoveTableIdsFromTabletInfo(
}

Result<scoped_refptr<TableInfo>> CatalogManager::FindTable(
const TableIdentifierPB& table_identifier) const {
const TableIdentifierPB& table_identifier, bool include_deleted) const {
SharedLock lock(mutex_);
return FindTableUnlocked(table_identifier);
return FindTableUnlocked(table_identifier, include_deleted);
}

Result<scoped_refptr<TableInfo>> CatalogManager::FindTableUnlocked(
const TableIdentifierPB& table_identifier) const {
const TableIdentifierPB& table_identifier, bool include_deleted) const {
if (table_identifier.has_table_id()) {
return FindTableByIdUnlocked(table_identifier.table_id());
return FindTableByIdUnlocked(table_identifier.table_id(), include_deleted);
}

if (table_identifier.has_table_name()) {
Expand All @@ -5583,7 +5583,7 @@ Result<scoped_refptr<TableInfo>> CatalogManager::FindTableUnlocked(
}

auto it = table_names_map_.find({namespace_info->id(), table_identifier.table_name()});
if (it == table_names_map_.end()) {
if (it == table_names_map_.end() || (!include_deleted && it->second->is_deleted())) {
return STATUS_EC_FORMAT(
NotFound, MasterError(MasterErrorPB::OBJECT_NOT_FOUND),
"Table $0.$1 not found", namespace_info->name(), table_identifier.table_name());
Expand All @@ -5602,9 +5602,9 @@ Result<scoped_refptr<TableInfo>> CatalogManager::FindTableById(
}

Result<scoped_refptr<TableInfo>> CatalogManager::FindTableByIdUnlocked(
const TableId& table_id) const {
const TableId& table_id, bool include_deleted) const {
auto table = tables_->FindTableOrNull(table_id);
if (table == nullptr) {
if (table == nullptr || (!include_deleted && table->is_deleted())) {
return STATUS_EC_FORMAT(
NotFound, MasterError(MasterErrorPB::OBJECT_NOT_FOUND),
"Table with identifier $0 not found", table_id);
Expand Down Expand Up @@ -7658,7 +7658,7 @@ Status CatalogManager::GetTableSchemaInternal(const GetTableSchemaRequestPB* req

// Lookup the table and verify if it exists.
TRACE("Looking up table");
auto table = VERIFY_RESULT(FindTable(req->table()));
auto table = VERIFY_RESULT(FindTable(req->table(), !req->check_exists_only()));
if (req->check_exists_only()) {
return Status::OK();
}
Expand Down
8 changes: 5 additions & 3 deletions src/yb/master/catalog_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -1049,16 +1049,18 @@ class CatalogManager : public CatalogManagerIf, public SnapshotCoordinatorContex
const NamespaceId& id) const REQUIRES_SHARED(mutex_);

Result<scoped_refptr<TableInfo>> FindTableUnlocked(
const TableIdentifierPB& table_identifier) const REQUIRES_SHARED(mutex_);
const TableIdentifierPB& table_identifier, bool include_deleted = true) const
REQUIRES_SHARED(mutex_);

Result<scoped_refptr<TableInfo>> FindTable(
const TableIdentifierPB& table_identifier) const override EXCLUDES(mutex_);
const TableIdentifierPB& table_identifier, bool include_deleted = true) const override
EXCLUDES(mutex_);

Result<scoped_refptr<TableInfo>> FindTableById(
const TableId& table_id) const override EXCLUDES(mutex_);

Result<scoped_refptr<TableInfo>> FindTableByIdUnlocked(
const TableId& table_id) const REQUIRES_SHARED(mutex_);
const TableId& table_id, bool include_deleted = true) const REQUIRES_SHARED(mutex_);

Result<TableId> GetColocatedTableId(
const TablegroupId& tablegroup_id, ColocationId colocation_id) const EXCLUDES(mutex_);
Expand Down
2 changes: 1 addition & 1 deletion src/yb/master/catalog_manager_if.h
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ class CatalogManagerIf : public tserver::TabletPeerLookupIf {
const GetCDCDBStreamInfoRequestPB* req, GetCDCDBStreamInfoResponsePB* resp) = 0;

virtual Result<scoped_refptr<TableInfo>> FindTable(
const TableIdentifierPB& table_identifier) const = 0;
const TableIdentifierPB& table_identifier, bool include_deleted = true) const = 0;

virtual Status IsInitDbDone(
const IsInitDbDoneRequestPB* req, IsInitDbDoneResponsePB* resp) = 0;
Expand Down

0 comments on commit 4593e62

Please sign in to comment.