From 87d1d5ce7cd6582b138d6c856b6a988f17f95e38 Mon Sep 17 00:00:00 2001 From: Sergei Politov Date: Fri, 14 May 2021 07:34:36 +0300 Subject: [PATCH] [Backport 2.6] [#7126] PITR: Handle master failover Summary: This diff adds handling of hidden tablets during master failover. We introduce a new persistent hide_state on the Table objects in the master. - When deleting a table covered by PITR, we leave it in RUNNING state, but change the hide_state to HIDING. - Once all tablets are also hidden, we transition the table's hide_state to HIDDEN - Once the table goes out of PITR scope, we then change it from RUNNING to DELETED This also buttons up all callsites that use GetTables, to ensure we don't display hidden tables to clients that do not care about them. This is relevant for YCQL system tables, for example. In the master UIs, we can keep displaying hidden tables as well. Original commit: D11563 / c221319413b3b922abba5f0bb873fa712911e6f2 Test Plan: ybd --gtest_filter YbAdminSnapshotScheduleTest.UndeleteTableWithRestart Jenkins: rebase: 2.6 Reviewers: bogdan Reviewed By: bogdan Subscribers: ybase Differential Revision: https://phabricator.dev.yugabyte.com/D11697 --- ent/src/yb/integration-tests/snapshot-test.cc | 2 - ent/src/yb/master/catalog_manager.h | 7 +- ent/src/yb/master/catalog_manager_ent.cc | 63 ++++++- src/yb/client/ql-tablet-test.cc | 6 +- src/yb/gutil/stl_util.h | 6 + .../create-table-stress-test.cc | 4 +- .../external_mini_cluster.cc | 11 ++ .../integration-tests/external_mini_cluster.h | 7 + src/yb/master/async_rpc_tasks.cc | 2 +- src/yb/master/catalog_entity_info.cc | 20 ++- src/yb/master/catalog_entity_info.h | 21 +++ src/yb/master/catalog_loaders.cc | 23 +-- src/yb/master/catalog_manager.cc | 163 ++++++++++-------- src/yb/master/catalog_manager.h | 29 ++-- src/yb/master/master-path-handlers.cc | 22 +-- src/yb/master/master.proto | 11 ++ src/yb/master/master_service.cc | 3 +- src/yb/master/master_snapshot_coordinator.cc | 2 +- src/yb/master/snapshot_coordinator_context.h | 2 +- src/yb/master/sys_catalog-internal.h | 12 +- src/yb/master/sys_catalog.h | 12 +- src/yb/master/sys_catalog_writer.h | 6 + src/yb/master/yql_columns_vtable.cc | 3 +- src/yb/master/yql_indexes_vtable.cc | 5 +- src/yb/master/yql_partitions_vtable.cc | 3 +- src/yb/master/yql_size_estimates_vtable.cc | 5 +- src/yb/master/yql_tables_vtable.cc | 5 +- .../tools/yb-admin-snapshot-schedule-test.cc | 47 ++++- 28 files changed, 346 insertions(+), 156 deletions(-) diff --git a/ent/src/yb/integration-tests/snapshot-test.cc b/ent/src/yb/integration-tests/snapshot-test.cc index cf45b5509272..d2b0a3746794 100644 --- a/ent/src/yb/integration-tests/snapshot-test.cc +++ b/ent/src/yb/integration-tests/snapshot-test.cc @@ -80,8 +80,6 @@ using master::ImportSnapshotMetaResponsePB; using master::ImportSnapshotMetaResponsePB_TableMetaPB; using master::IsCreateTableDoneRequestPB; using master::IsCreateTableDoneResponsePB; -using master::ListTablesRequestPB; -using master::ListTablesResponsePB; using master::ListSnapshotsRequestPB; using master::ListSnapshotsResponsePB; using master::MasterServiceProxy; diff --git a/ent/src/yb/master/catalog_manager.h b/ent/src/yb/master/catalog_manager.h index 42869a44d8e3..e5b68e23cc95 100644 --- a/ent/src/yb/master/catalog_manager.h +++ b/ent/src/yb/master/catalog_manager.h @@ -278,7 +278,12 @@ class CatalogManager : public yb::master::CatalogManager, SnapshotCoordinatorCon CHECKED_STATUS RestoreSysCatalog(SnapshotScheduleRestoration* restoration) override; CHECKED_STATUS VerifyRestoredObjects(const SnapshotScheduleRestoration& restoration) override; - void CleanupHiddenTablets(const ScheduleMinRestoreTime& schedule_min_restore_time) override; + void CleanupHiddenObjects(const ScheduleMinRestoreTime& schedule_min_restore_time) override; + void CleanupHiddenTablets( + const std::vector& hidden_tablets, + const ScheduleMinRestoreTime& schedule_min_restore_time); + // Will filter tables content, so pass it by value here. + void CleanupHiddenTables(std::vector tables); rpc::Scheduler& Scheduler() override; diff --git a/ent/src/yb/master/catalog_manager_ent.cc b/ent/src/yb/master/catalog_manager_ent.cc index d2a073613587..c305fe20a5ab 100644 --- a/ent/src/yb/master/catalog_manager_ent.cc +++ b/ent/src/yb/master/catalog_manager_ent.cc @@ -1103,7 +1103,7 @@ Status CatalogManager::ImportTableEntry(const NamespaceMap& namespace_map, // Check table is active, table name and table schema are equal to backed up ones. if (table != nullptr) { auto table_lock = table->LockForRead(); - if (table->is_running() && table->name() == meta.name()) { + if (table_lock->visible_to_client() && table->name() == meta.name()) { VLOG_WITH_PREFIX(1) << __func__ << " found existing table: " << table->ToString(); // Check the found table schema. Schema persisted_schema; @@ -1120,7 +1120,8 @@ Status CatalogManager::ImportTableEntry(const NamespaceMap& namespace_map, } else { VLOG_WITH_PREFIX(1) << __func__ << " existing table " << table->ToString() << " not suitable: " - << table->is_running() << ", name: " << table->name() << " vs " << meta.name(); + << table_lock->pb.ShortDebugString() + << ", name: " << table->name() << " vs " << meta.name(); table.reset(); } } @@ -1149,7 +1150,7 @@ Status CatalogManager::ImportTableEntry(const NamespaceMap& namespace_map, table = entry.second; auto ltm = table->LockForRead(); - if (table->is_running() && + if (ltm->visible_to_client() && new_namespace_id == table->namespace_id() && meta.name() == ltm->name() && (table_data->is_index() ? IsUserIndexUnlocked(*table) @@ -1511,16 +1512,30 @@ Status CatalogManager::VerifyRestoredObjects(const SnapshotScheduleRestoration& return Status::OK(); } -void CatalogManager::CleanupHiddenTablets(const ScheduleMinRestoreTime& schedule_min_restore_time) { +void CatalogManager::CleanupHiddenObjects(const ScheduleMinRestoreTime& schedule_min_restore_time) { VLOG_WITH_PREFIX_AND_FUNC(4) << AsString(schedule_min_restore_time); std::vector hidden_tablets; + std::vector tables; { SharedLock l(lock_); - if (hidden_tablets_.empty()) { - return; - } hidden_tablets = hidden_tablets_; + tables.reserve(table_ids_map_->size()); + for (const auto& p : *table_ids_map_) { + if (!p.second->is_system()) { + tables.push_back(p.second); + } + } + } + CleanupHiddenTablets(hidden_tablets, schedule_min_restore_time); + CleanupHiddenTables(std::move(tables)); +} + +void CatalogManager::CleanupHiddenTablets( + const std::vector& hidden_tablets, + const ScheduleMinRestoreTime& schedule_min_restore_time) { + if (hidden_tablets.empty()) { + return; } std::vector tablets_to_delete; std::vector tablets_to_remove_from_hidden; @@ -1556,6 +1571,7 @@ void CatalogManager::CleanupHiddenTablets(const ScheduleMinRestoreTime& schedule WARN_NOT_OK(DeleteTabletListAndSendRequests(tablets_to_delete, "Cleanup hidden tablets", {}), "Failed to cleanup hidden tablets"); } + if (!tablets_to_remove_from_hidden.empty()) { auto it = tablets_to_remove_from_hidden.begin(); std::lock_guard l(lock_); @@ -1573,6 +1589,39 @@ void CatalogManager::CleanupHiddenTablets(const ScheduleMinRestoreTime& schedule } } +void CatalogManager::CleanupHiddenTables(std::vector tables) { + std::vector locks; + EraseIf([this, &locks](const TableInfoPtr& table) { + { + auto lock = table->LockForRead(); + if (!lock->is_hidden() || lock->started_deleting() || !table->AreAllTabletsDeleted()) { + return true; + } + } + auto lock = table->LockForWrite(); + if (lock->started_deleting()) { + return true; + } + LOG_WITH_PREFIX(INFO) << "Should delete table: " << AsString(table); + lock.mutable_data()->set_state( + SysTablesEntryPB::DELETED, Format("Cleanup hidden table at $0", LocalTimeAsString())); + locks.push_back(std::move(lock)); + return false; + }, &tables); + if (tables.empty()) { + return; + } + + Status s = sys_catalog_->UpdateItems(tables, leader_ready_term()); + if (!s.ok()) { + LOG_WITH_PREFIX(WARNING) << "Failed to mark tables as deleted: " << s; + return; + } + for (auto& lock : locks) { + lock.Commit(); + } +} + rpc::Scheduler& CatalogManager::Scheduler() { return master_->messenger()->scheduler(); } diff --git a/src/yb/client/ql-tablet-test.cc b/src/yb/client/ql-tablet-test.cc index f0fc42895961..7dd32d0fc5ad 100644 --- a/src/yb/client/ql-tablet-test.cc +++ b/src/yb/client/ql-tablet-test.cc @@ -325,10 +325,8 @@ class QLTabletTest : public QLDmlTestBase { scoped_refptr GetTableInfo(const YBTableName& table_name) { auto* catalog_manager = cluster_->leader_mini_master()->master()->catalog_manager(); - std::vector> all_tables; - catalog_manager->GetAllTables(&all_tables); - scoped_refptr table_info; - for (auto& table : all_tables) { + auto all_tables = catalog_manager->GetTables(master::GetTablesMode::kAll); + for (const auto& table : all_tables) { if (table->name() == table_name.table_name()) { return table; } diff --git a/src/yb/gutil/stl_util.h b/src/yb/gutil/stl_util.h index e1d0dba8f2f3..4b6bb71c10db 100644 --- a/src/yb/gutil/stl_util.h +++ b/src/yb/gutil/stl_util.h @@ -993,4 +993,10 @@ std::set VectorToSet(const std::vector& v) { return std::set(v.begin(), v.end()); } +template +void EraseIf(const Predicate& predicate, Collection* collection) { + collection->erase(std::remove_if(collection->begin(), collection->end(), predicate), + collection->end()); +} + #endif // YB_GUTIL_STL_UTIL_H diff --git a/src/yb/integration-tests/create-table-stress-test.cc b/src/yb/integration-tests/create-table-stress-test.cc index bc9663d365a5..c6afdaf7d7ac 100644 --- a/src/yb/integration-tests/create-table-stress-test.cc +++ b/src/yb/integration-tests/create-table-stress-test.cc @@ -551,8 +551,8 @@ DontVerifyClusterBeforeNextTearDown(); LOG(INFO) << "========================================================"; LOG(INFO) << "Tables and tablets:"; LOG(INFO) << "========================================================"; - std::vector > tables; - cluster_->mini_master()->master()->catalog_manager()->GetAllTables(&tables); + auto tables = cluster_->mini_master()->master()->catalog_manager()->GetTables( + master::GetTablesMode::kAll); for (const scoped_refptr& table_info : tables) { LOG(INFO) << "Table: " << table_info->ToString(); std::vector > tablets; diff --git a/src/yb/integration-tests/external_mini_cluster.cc b/src/yb/integration-tests/external_mini_cluster.cc index b9e330f4a4d1..e7c91bcf9e99 100644 --- a/src/yb/integration-tests/external_mini_cluster.cc +++ b/src/yb/integration-tests/external_mini_cluster.cc @@ -2466,4 +2466,15 @@ Status ExternalTabletServer::Restart(bool start_cql_proxy) { return Start(start_cql_proxy); } +Status RestartAllMasters(ExternalMiniCluster* cluster) { + for (int i = 0; i != cluster->num_masters(); ++i) { + cluster->master(i)->Shutdown(); + } + for (int i = 0; i != cluster->num_masters(); ++i) { + RETURN_NOT_OK(cluster->master(i)->Restart()); + } + + return Status::OK(); +} + } // namespace yb diff --git a/src/yb/integration-tests/external_mini_cluster.h b/src/yb/integration-tests/external_mini_cluster.h index d727e1c27a54..2a28a2a4054b 100644 --- a/src/yb/integration-tests/external_mini_cluster.h +++ b/src/yb/integration-tests/external_mini_cluster.h @@ -359,6 +359,11 @@ class ExternalMiniCluster : public MiniClusterBase { return GetProxy(master(i)); } + template + std::shared_ptr GetLeaderMasterProxy() { + return GetProxy(GetLeaderMaster()); + } + // If the cluster is configured for a single non-distributed master, return a proxy to that // master. Requires that the single master is running. std::shared_ptr master_proxy(); @@ -820,5 +825,7 @@ std::shared_ptr ExternalMiniCluster::GetProxy(ExternalDaemon* daemon) { return std::make_shared(proxy_cache_.get(), daemon->bound_rpc_addr()); } +CHECKED_STATUS RestartAllMasters(ExternalMiniCluster* cluster); + } // namespace yb #endif // YB_INTEGRATION_TESTS_EXTERNAL_MINI_CLUSTER_H_ diff --git a/src/yb/master/async_rpc_tasks.cc b/src/yb/master/async_rpc_tasks.cc index f3a81ead6b95..b59224fed11c 100644 --- a/src/yb/master/async_rpc_tasks.cc +++ b/src/yb/master/async_rpc_tasks.cc @@ -111,7 +111,7 @@ RetryingTSRpcTask::RetryingTSRpcTask(Master *master, : master_(master), callback_pool_(callback_pool), replica_picker_(replica_picker.Pass()), - table_(table), + table_(DCHECK_NOTNULL(table)), start_ts_(MonoTime::Now()), attempt_(0), state_(MonitoredTaskState::kWaiting) { diff --git a/src/yb/master/catalog_entity_info.cc b/src/yb/master/catalog_entity_info.cc index 45129d8159cc..af5b2849446a 100644 --- a/src/yb/master/catalog_entity_info.cc +++ b/src/yb/master/catalog_entity_info.cc @@ -411,9 +411,15 @@ bool TableInfo::IsAlterInProgress(uint32_t version) const { } return false; } -bool TableInfo::AreAllTabletsDeleted() const { + +bool TableInfo::HasTablets() const { shared_lock l(lock_); - for (const TableInfo::TabletInfoMap::value_type& e : tablet_map_) { + return !tablet_map_.empty(); +} + +bool TableInfo::AreAllTabletsDeletedOrHidden() const { + shared_lock l(lock_); + for (const auto& e : tablet_map_) { auto tablet_lock = e.second->LockForRead(); if (!tablet_lock->is_deleted() && !tablet_lock->is_hidden()) { return false; @@ -422,6 +428,16 @@ bool TableInfo::AreAllTabletsDeleted() const { return true; } +bool TableInfo::AreAllTabletsDeleted() const { + shared_lock l(lock_); + for (const auto& e : tablet_map_) { + if (!e.second->LockForRead()->is_deleted()) { + return false; + } + } + return true; +} + bool TableInfo::IsCreateInProgress() const { shared_lock l(lock_); for (const TableInfo::TabletInfoMap::value_type& e : tablet_map_) { diff --git a/src/yb/master/catalog_entity_info.h b/src/yb/master/catalog_entity_info.h index e499890a13ec..0b5d33d9b840 100644 --- a/src/yb/master/catalog_entity_info.h +++ b/src/yb/master/catalog_entity_info.h @@ -312,6 +312,22 @@ struct PersistentTableInfo : public Persistent, // Get all tablets of the table. void GetAllTablets(TabletInfos *ret) const; + bool HasTablets() const; + // Get the tablet of the table. The table must be colocated. TabletInfoPtr GetColocatedTablet() const; @@ -421,6 +439,9 @@ class TableInfo : public RefCountedThreadSafe, // Returns true if all tablets of the table are deleted. bool AreAllTabletsDeleted() const; + // Returns true if all tablets of the table are deleted or hidden. + bool AreAllTabletsDeletedOrHidden() const; + // Returns true if the table creation is in-progress. bool IsCreateInProgress() const; diff --git a/src/yb/master/catalog_loaders.cc b/src/yb/master/catalog_loaders.cc index 3f84db9d67f2..1295999e8b12 100644 --- a/src/yb/master/catalog_loaders.cc +++ b/src/yb/master/catalog_loaders.cc @@ -89,7 +89,8 @@ Status TableLoader::Visit(const TableId& table_id, const SysTablesEntryPB& metad "Could not submit VerifyTransaction to thread pool"); } - LOG(INFO) << "Loaded metadata for table " << table->ToString(); + LOG(INFO) << "Loaded metadata for table " << table->ToString() << ", state: " + << SysTablesEntryPB::State_Name(metadata.state()); VLOG(1) << "Metadata for table " << table->ToString() << ": " << metadata.ShortDebugString(); return Status::OK(); @@ -101,14 +102,14 @@ Status TableLoader::Visit(const TableId& table_id, const SysTablesEntryPB& metad Status TabletLoader::Visit(const TabletId& tablet_id, const SysTabletsEntryPB& metadata) { // Lookup the table. - scoped_refptr first_table(FindPtrOrNull( - *catalog_manager_->table_ids_map_, metadata.table_id())); + TableInfoPtr first_table = FindPtrOrNull(*catalog_manager_->table_ids_map_, metadata.table_id()); // TODO: We need to properly remove deleted tablets. This can happen async of master loading. if (!first_table) { if (metadata.state() != SysTabletsEntryPB::DELETED) { - LOG(ERROR) << "Unexpected Tablet state for " << tablet_id << ": " - << SysTabletsEntryPB::State_Name(metadata.state()); + LOG(DFATAL) << "Unexpected Tablet state for " << tablet_id << ": " + << SysTabletsEntryPB::State_Name(metadata.state()) + << ", unknown table for this tablet: " << metadata.table_id(); } return Status::OK(); } @@ -117,7 +118,7 @@ Status TabletLoader::Visit(const TabletId& tablet_id, const SysTabletsEntryPB& m std::vector table_ids; bool tablet_deleted; bool listed_as_hidden; - TabletInfo* tablet = new TabletInfo(first_table, tablet_id); + TabletInfoPtr tablet(new TabletInfo(first_table, tablet_id)); { auto l = tablet->LockForWrite(); l.mutable_data()->pb.CopyFrom(metadata); @@ -140,7 +141,7 @@ Status TabletLoader::Visit(const TabletId& tablet_id, const SysTabletsEntryPB& m if (metadata.table_ids_size() == 0) { l.mutable_data()->pb.add_table_ids(metadata.table_id()); Status s = catalog_manager_->sys_catalog_->UpdateItem( - tablet, catalog_manager_->leader_ready_term()); + tablet.get(), catalog_manager_->leader_ready_term()); if (PREDICT_FALSE(!s.ok())) { return STATUS_FORMAT( IllegalState, "An error occurred while inserting to sys-tablets: $0", s); @@ -154,8 +155,8 @@ Status TabletLoader::Visit(const TabletId& tablet_id, const SysTabletsEntryPB& m // Assume we need to delete this tablet until we find an active table using this tablet. bool should_delete_tablet = !tablet_deleted; - for (auto table_id : table_ids) { - scoped_refptr table(FindPtrOrNull(*catalog_manager_->table_ids_map_, table_id)); + for (const auto& table_id : table_ids) { + TableInfoPtr table = FindPtrOrNull(*catalog_manager_->table_ids_map_, table_id); if (table == nullptr) { // If the table is missing and the tablet is in "preparing" state @@ -188,7 +189,7 @@ Status TabletLoader::Visit(const TabletId& tablet_id, const SysTabletsEntryPB& m if (tablet_id == kSysCatalogTabletId) { table->set_is_system(); } - table->AddTablet(tablet); + table->AddTablet(tablet.get()); } auto tl = table->LockForRead(); @@ -204,7 +205,7 @@ Status TabletLoader::Visit(const TabletId& tablet_id, const SysTabletsEntryPB& m << "Deleting tablet " << tablet->id() << " for table " << first_table->ToString(); string deletion_msg = "Tablet deleted at " + LocalTimeAsString(); l.mutable_data()->set_state(SysTabletsEntryPB::DELETED, deletion_msg); - RETURN_NOT_OK_PREPEND(catalog_manager_->sys_catalog()->UpdateItem(tablet, term_), + RETURN_NOT_OK_PREPEND(catalog_manager_->sys_catalog()->UpdateItem(tablet.get(), term_), strings::Substitute("Error deleting tablet $0", tablet->id())); } diff --git a/src/yb/master/catalog_manager.cc b/src/yb/master/catalog_manager.cc index 1115a2b6cd01..e3ba330f1f11 100644 --- a/src/yb/master/catalog_manager.cc +++ b/src/yb/master/catalog_manager.cc @@ -516,14 +516,14 @@ class IndexInfoBuilder { }; template -Status CheckIfTableDeletedOrNotRunning(const Lock& lock, RespClass* resp) { +Status CheckIfTableDeletedOrNotVisibleToClient(const Lock& lock, RespClass* resp) { // This covers both in progress and fully deleted objects. if (lock->started_deleting()) { Status s = STATUS_SUBSTITUTE(NotFound, "The object '$0.$1' does not exist", lock->namespace_id(), lock->name()); return SetupError(resp->mutable_error(), MasterErrorPB::OBJECT_NOT_FOUND, s); } - if (!lock->is_running()) { + if (!lock->visible_to_client()) { Status s = STATUS_SUBSTITUTE(ServiceUnavailable, "The object '$0.$1' is not running", lock->namespace_id(), lock->name()); return SetupError(resp->mutable_error(), MasterErrorPB::OBJECT_NOT_FOUND, s); @@ -1875,7 +1875,7 @@ Status CatalogManager::AddIndexInfoToTable(const scoped_refptr& index << yb::ToString(index_info); TRACE("Locking indexed table"); auto l = DCHECK_NOTNULL(indexed_table)->LockForWrite(); - RETURN_NOT_OK(CheckIfTableDeletedOrNotRunning(l, resp)); + RETURN_NOT_OK(CheckIfTableDeletedOrNotVisibleToClient(l, resp)); // Make sure that the index appears to not have been added to the table until the tservers apply // the alter and respond back. @@ -2490,7 +2490,7 @@ Status CatalogManager::CreateTable(const CreateTableRequestPB* orig_req, } TRACE("Locking indexed table"); - RETURN_NOT_OK(CheckIfTableDeletedOrNotRunning(indexed_table->LockForRead(), resp)); + RETURN_NOT_OK(CheckIfTableDeletedOrNotVisibleToClient(indexed_table->LockForRead(), resp)); } // Determine if this table should be colocated. If not specified, the table should be colocated if @@ -3225,7 +3225,7 @@ Status CatalogManager::IsCreateTableDone(const IsCreateTableDoneRequestPB* req, TRACE("Locking table"); auto l = table->LockForRead(); - RETURN_NOT_OK(CheckIfTableDeletedOrNotRunning(l, resp)); + RETURN_NOT_OK(CheckIfTableDeletedOrNotVisibleToClient(l, resp)); const auto& pb = l->pb; // 2. Verify if the create is in-progress. @@ -3658,7 +3658,7 @@ Status CatalogManager::TruncateTable(const TableId& table_id, TRACE(Substitute("Locking object with id $0", table_id)); auto l = table->LockForRead(); - RETURN_NOT_OK(CheckIfTableDeletedOrNotRunning(l, resp)); + RETURN_NOT_OK(CheckIfTableDeletedOrNotVisibleToClient(l, resp)); // Truncate on a colocated table should not hit master because it should be handled by a write // DML that creates a table-level tombstone. @@ -3716,7 +3716,7 @@ Status CatalogManager::IsTruncateTableDone(const IsTruncateTableDoneRequestPB* r } TRACE("Locking table"); - RETURN_NOT_OK(CheckIfTableDeletedOrNotRunning(table->LockForRead(), resp)); + RETURN_NOT_OK(CheckIfTableDeletedOrNotVisibleToClient(table->LockForRead(), resp)); resp->set_done(!table->HasTasks(MonitoredTask::Type::ASYNC_TRUNCATE_TABLET)); return Status::OK(); @@ -3897,15 +3897,14 @@ Status CatalogManager::DeleteTable( // IMPORTANT: If modifying, consider updating DeleteYsqlDBTables(), the bulk deletion API. Status CatalogManager::DeleteTableInternal( const DeleteTableRequestPB* req, DeleteTableResponsePB* resp, rpc::RpcContext* rpc) { - vector> tables; - vector table_locks; auto schedules_to_tables_map = VERIFY_RESULT( MakeSnapshotSchedulesToObjectIdsMap(SysRowEntry::TABLE)); + vector tables; RETURN_NOT_OK(DeleteTableInMemory(req->table(), req->is_index_table(), true /* update_indexed_table */, schedules_to_tables_map, - &tables, &table_locks, resp, rpc)); + &tables, resp, rpc)); // Delete any CDC streams that are set up on this table. TRACE("Deleting CDC streams on table"); @@ -3913,8 +3912,8 @@ Status CatalogManager::DeleteTableInternal( // Update the in-memory state. TRACE("Committing in-memory state"); - for (int i = 0; i < table_locks.size(); i++) { - table_locks[i].Commit(); + for (auto& table : tables) { + table.write_lock.Commit(); } if (PREDICT_FALSE(FLAGS_catalog_manager_inject_latency_in_delete_table_ms > 0)) { @@ -3924,22 +3923,14 @@ Status CatalogManager::DeleteTableInternal( } for (const auto& table : tables) { - RepeatedBytes retained_by_snapshot_schedules; - for (const auto& entry : schedules_to_tables_map) { - if (std::binary_search(entry.second.begin(), entry.second.end(), table->id())) { - retained_by_snapshot_schedules.Add()->assign( - entry.first.AsSlice().cdata(), entry.first.size()); - } - } - // Send a DeleteTablet() request to each tablet replica in the table. - RETURN_NOT_OK(DeleteTabletsAndSendRequests(table, retained_by_snapshot_schedules)); + RETURN_NOT_OK(DeleteTabletsAndSendRequests(table.info, table.retained_by_snapshot_schedules)); // Send a RemoveTableFromTablet() request to each colocated parent tablet replica in the table. // TODO(pitr) handle YSQL colocated tables. - if (IsColocatedUserTable(*table)) { + if (IsColocatedUserTable(*table.info)) { auto call = std::make_shared( - master_, AsyncTaskPool(), table->GetColocatedTablet(), table); - table->AddTask(call); + master_, AsyncTaskPool(), table.info->GetColocatedTablet(), table.info); + table.info->AddTask(call); WARN_NOT_OK(ScheduleTask(call), "Failed to send RemoveTableFromTablet request"); } } @@ -3967,8 +3958,7 @@ Status CatalogManager::DeleteTableInMemory( const bool is_index_table, const bool update_indexed_table, const SnapshotSchedulesToObjectIdsMap& schedules_to_tables_map, - vector>* tables, - vector* table_lcks, + vector* tables, DeleteTableResponsePB* resp, rpc::RpcContext* rpc) { // TODO(NIC): How to handle a DeleteTable request when the namespace is being deleted? @@ -3989,15 +3979,19 @@ Status CatalogManager::DeleteTableInMemory( auto table = std::move(*table_result); TRACE(Substitute("Locking $0", object_type)); - auto l = table->LockForWrite(); + auto data = DeletingTableData { + .info = table, + .write_lock = table->LockForWrite(), + }; + auto& l = data.write_lock; resp->set_table_id(table->id()); - if (is_index_table == IsTable(l.data().pb)) { + if (is_index_table == IsTable(l->pb)) { Status s = STATUS(NotFound, "The object does not exist"); return SetupError(resp->mutable_error(), MasterErrorPB::OBJECT_NOT_FOUND, s); } - if (l.data().started_deleting()) { + if (l->started_deleting()) { if (cascade_delete_index) { LOG(WARNING) << "Index " << table_identifier.DebugString() << " was deleted"; return Status::OK(); @@ -4007,10 +4001,21 @@ Status CatalogManager::DeleteTableInMemory( } } + for (const auto& entry : schedules_to_tables_map) { + if (std::binary_search(entry.second.begin(), entry.second.end(), table->id())) { + data.retained_by_snapshot_schedules.Add()->assign( + entry.first.AsSlice().cdata(), entry.first.size()); + } + } + TRACE("Updating metadata on disk"); // Update the metadata for the on-disk state. - l.mutable_data()->set_state(SysTablesEntryPB::DELETING, - Substitute("Started deleting at $0", LocalTimeAsString())); + if (data.retained_by_snapshot_schedules.empty()) { + l.mutable_data()->set_state(SysTablesEntryPB::DELETING, + Substitute("Started deleting at $0", LocalTimeAsString())); + } else { + l.mutable_data()->pb.set_hide_state(SysTablesEntryPB::HIDING); + } // Update sys-catalog with the removed table state. Status s = sys_catalog_->UpdateItem(table.get(), leader_ready_term()); @@ -4042,11 +4047,11 @@ Status CatalogManager::DeleteTableInMemory( // index info from the indexed table. if (!is_index_table) { TableIdentifierPB index_identifier; - for (auto index : l->pb.indexes()) { + for (const auto& index : l->pb.indexes()) { index_identifier.set_table_id(index.table_id()); RETURN_NOT_OK(DeleteTableInMemory(index_identifier, true /* is_index_table */, false /* update_indexed_table */, schedules_to_tables_map, - tables, table_lcks, resp, rpc)); + tables, resp, rpc)); } } else if (update_indexed_table) { s = MarkIndexInfoFromTableForDeletion( @@ -4064,18 +4069,12 @@ Status CatalogManager::DeleteTableInMemory( // For regular (indexed) table, insert table info and lock in the front of the list. Else for // index table, append them to the end. We do so so that we will commit and delete the indexed // table first before its indexes. - if (!is_index_table) { - tables->insert(tables->begin(), table); - table_lcks->insert(table_lcks->begin(), std::move(l)); - } else { - tables->push_back(table); - table_lcks->push_back(std::move(l)); - } + tables->insert(is_index_table ? tables->end() : tables->begin(), std::move(data)); return Status::OK(); } -TableInfo::WriteLock CatalogManager::MaybeTransitionTableToDeleted(TableInfoPtr table) { +TableInfo::WriteLock CatalogManager::MaybeTransitionTableToDeleted(const TableInfoPtr& table) { if (table->HasTasks()) { return TableInfo::WriteLock(); } @@ -4084,7 +4083,7 @@ TableInfo::WriteLock CatalogManager::MaybeTransitionTableToDeleted(TableInfoPtr // For any table in DELETING state, we will want to mark it as DELETED once all its respective // tablets have been successfully removed from tservers. - if (!lock->is_deleting()) { + if (!lock->is_deleting() && !lock->is_hiding()) { return TableInfo::WriteLock(); } } @@ -4096,14 +4095,19 @@ TableInfo::WriteLock CatalogManager::MaybeTransitionTableToDeleted(TableInfoPtr // This creates a race, wherein, after 2, HasTasks can be false, but we still have not // gotten to point 3, which would add further tasks for the deletes. // - // However, HasTasks is cheaper than AreAllTabletsDeleted... - if (!table->AreAllTabletsDeleted() && + // However, HasTasks is cheaper than AreAllTabletsDeletedOrHidden... + if (!table->AreAllTabletsDeletedOrHidden() && !IsSystemTable(*table) && !IsColocatedUserTable(*table)) { return TableInfo::WriteLock(); } auto lock = table->LockForWrite(); + if (lock->is_hiding()) { + LOG(INFO) << "Marking table as HIDDEN: " << table->ToString(); + lock.mutable_data()->pb.set_hide_state(SysTablesEntryPB::HIDDEN); + return lock; + } if (lock->is_deleting()) { // Update the metadata for the on-disk state. LOG(INFO) << "Marking table as DELETED: " << table->ToString(); @@ -4170,7 +4174,7 @@ Status CatalogManager::IsDeleteTableDone(const IsDeleteTableDoneRequestPB* req, TRACE("Locking table"); auto l = table->LockForRead(); - if (!l->started_deleting()) { + if (!l->started_deleting() && !l->started_hiding()) { LOG(WARNING) << "Servicing IsDeleteTableDone request for table id " << req->table_id() << ": NOT deleted"; Status s = STATUS(IllegalState, "The object was NOT deleted", l->pb.state_msg()); @@ -4186,10 +4190,10 @@ Status CatalogManager::IsDeleteTableDone(const IsDeleteTableDoneRequestPB* req, return Status::OK(); } - if (l->is_deleted()) { + if (l->is_deleted() || l->is_hidden()) { LOG(INFO) << "Servicing IsDeleteTableDone request for table id " - << req->table_id() << ": totally deleted"; - resp->set_done(true); + << req->table_id() << ": totally " << (l->is_hidden() ? "hidden" : "deleted"); + resp->set_done(true); } else { LOG(INFO) << "Servicing IsDeleteTableDone request for table id " << req->table_id() << ((!IsColocatedUserTable(*table)) ? ": deleting tablets" : ""); @@ -4332,7 +4336,7 @@ Status CatalogManager::AlterTable(const AlterTableRequestPB* req, TRACE("Locking table"); auto l = table->LockForWrite(); - RETURN_NOT_OK(CheckIfTableDeletedOrNotRunning(l, resp)); + RETURN_NOT_OK(CheckIfTableDeletedOrNotVisibleToClient(l, resp)); bool has_changes = false; auto& table_pb = l.mutable_data()->pb; @@ -4509,7 +4513,7 @@ Status CatalogManager::IsAlterTableDone(const IsAlterTableDoneRequestPB* req, TRACE("Locking table"); auto l = table->LockForRead(); - RETURN_NOT_OK(CheckIfTableDeletedOrNotRunning(l, resp)); + RETURN_NOT_OK(CheckIfTableDeletedOrNotVisibleToClient(l, resp)); // 2. Verify if the alter is in-progress. TRACE("Verify if there is an alter operation in progress for $0", table->ToString()); @@ -4605,7 +4609,7 @@ Status CatalogManager::GetTableSchemaInternal(const GetTableSchemaRequestPB* req TRACE("Locking table"); auto l = table->LockForRead(); - RETURN_NOT_OK(CheckIfTableDeletedOrNotRunning(l, resp)); + RETURN_NOT_OK(CheckIfTableDeletedOrNotVisibleToClient(l, resp)); if (l->pb.has_fully_applied_schema()) { // An AlterTable is in progress; fully_applied_schema is the last @@ -4693,7 +4697,7 @@ Status CatalogManager::GetColocatedTabletSchema(const GetColocatedTabletSchemaRe { TRACE("Locking table"); auto l = parent_colocated_table->LockForRead(); - RETURN_NOT_OK(CheckIfTableDeletedOrNotRunning(l, resp)); + RETURN_NOT_OK(CheckIfTableDeletedOrNotVisibleToClient(l, resp)); } if (!parent_colocated_table->colocated() || !IsColocatedParentTable(*parent_colocated_table)) { @@ -4782,7 +4786,9 @@ Status CatalogManager::ListTables(const ListTablesRequestPB* req, auto& table_info = *entry.second; auto ltm = table_info.LockForRead(); - if (!ltm->is_running()) continue; + if (!ltm->visible_to_client() && !req->include_not_running()) { + continue; + } if (!namespace_id.empty() && namespace_id != table_info.namespace_id()) { continue; // Skip tables from other namespaces. @@ -4826,15 +4832,16 @@ Status CatalogManager::ListTables(const ListTablesRequestPB* req, ListTablesResponsePB::TableInfo *table = resp->add_tables(); { - auto l = (**ns).LockForRead(); + auto namespace_lock = (**ns).LockForRead(); table->mutable_namespace_()->set_id((**ns).id()); - table->mutable_namespace_()->set_name((**ns).name()); - table->mutable_namespace_()->set_database_type((**ns).database_type()); + table->mutable_namespace_()->set_name(namespace_lock->name()); + table->mutable_namespace_()->set_database_type(namespace_lock->pb.database_type()); } table->set_id(entry.second->id()); table->set_name(ltm->name()); table->set_table_type(ltm->table_type()); table->set_relation_type(relation_type); + table->set_state(ltm->pb.state()); } return Status::OK(); } @@ -4859,16 +4866,32 @@ scoped_refptr CatalogManager::GetTableInfoUnlocked(const TableId& tab return FindPtrOrNull(*table_ids_map_, table_id); } -void CatalogManager::GetAllTables(std::vector> *tables, - bool includeOnlyRunningTables) { - tables->clear(); - SharedLock l(lock_); - for (const auto& e : *table_ids_map_) { - if (includeOnlyRunningTables && !e.second->is_running()) { - continue; +std::vector CatalogManager::GetTables(GetTablesMode mode) { + std::vector result; + { + SharedLock l(lock_); + result.reserve(table_ids_map_->size()); + for (const auto& e : *table_ids_map_) { + result.push_back(e.second); + } + } + switch (mode) { + case GetTablesMode::kAll: + return result; + case GetTablesMode::kRunning: { + auto filter = [](const TableInfoPtr& table_info) { return !table_info->is_running(); }; + EraseIf(filter, &result); + return result; + } + case GetTablesMode::kVisibleToClient: { + auto filter = [](const TableInfoPtr& table_info) { + return !table_info->LockForRead()->visible_to_client(); + }; + EraseIf(filter, &result); + return result; } - tables->push_back(e.second); } + FATAL_INVALID_ENUM_VALUE(GetTablesMode, mode); } void CatalogManager::GetAllNamespaces(std::vector>* namespaces, @@ -5004,7 +5027,7 @@ bool CatalogManager::IsSequencesSystemTable(const TableInfo& table) const { void CatalogManager::NotifyTabletDeleteFinished(const TabletServerId& tserver_uuid, const TabletId& tablet_id, - scoped_refptr table) { + const TableInfoPtr& table) { shared_ptr ts_desc; if (!master_->ts_manager()->LookupTSByUUID(tserver_uuid, &ts_desc)) { LOG(WARNING) << "Unable to find tablet server " << tserver_uuid; @@ -5107,7 +5130,7 @@ Status CatalogManager::ProcessTabletReport(TSDescriptor* ts_desc, report.tablet_data_state() == TABLET_DATA_DELETED) { VLOG(1) << "Ignoring report from unknown tablet " << tablet_id; } else { - LOG(WARNING) << "Ignoring report from unknown tablet " << tablet_id; + LOG(DFATAL) << "Ignoring report from unknown tablet " << tablet_id; } // Every tablet in the report that is processed gets a heartbeat response entry. ReportedTabletUpdatesPB* update = full_report_update->add_tablets(); @@ -5175,7 +5198,7 @@ Status CatalogManager::ProcessTabletReport(TSDescriptor* ts_desc, ++i, ++tablet_iter) { const string& tablet_id = tablet_iter->first; const scoped_refptr& tablet = tablet_iter->second; - const scoped_refptr& table = tablet->table(); + const TableInfoPtr& table = tablet->table(); const ReportedTabletPB& report = *FindOrDie(reports, tablet_id); // Prepare an heartbeat response entry for this tablet, now that we're going to process it. @@ -8406,7 +8429,7 @@ Status CatalogManager::GetTableLocations( } auto l = table->LockForRead(); - RETURN_NOT_OK(CheckIfTableDeletedOrNotRunning(l, resp)); + RETURN_NOT_OK(CheckIfTableDeletedOrNotVisibleToClient(l, resp)); vector> tablets_in_range; table->GetTabletsInRange(req, &tablets_in_range); @@ -8897,9 +8920,9 @@ Status CatalogManager::AreLeadersOnPreferredOnly(const AreLeadersOnPreferredOnly RETURN_NOT_OK(GetClusterConfig(&config)); // Only need to fetch if txn tables are not using preferred zones. - vector> tables; + vector tables; if (!FLAGS_transaction_tables_use_preferred_zones) { - master_->catalog_manager()->GetAllTables(&tables, true /* include only running tables */); + tables = master_->catalog_manager()->GetTables(GetTablesMode::kRunning); } Status s = CatalogManagerUtil::AreLeadersOnPreferredOnly(ts_descs, diff --git a/src/yb/master/catalog_manager.h b/src/yb/master/catalog_manager.h index 544e4efcfee4..0c6782fc8df6 100644 --- a/src/yb/master/catalog_manager.h +++ b/src/yb/master/catalog_manager.h @@ -156,6 +156,11 @@ class BlacklistState { YB_STRONGLY_TYPED_BOOL(HideOnly); +YB_DEFINE_ENUM(GetTablesMode, (kAll) // All tables + (kRunning) // All running tables + (kVisibleToClient) // All tables visible to the client + ); + // The component of the master which tracks the state and location // of tables/tablets in the cluster. // @@ -462,19 +467,16 @@ class CatalogManager : public tserver::TabletPeerLookupIf { bool IsLoadBalancerEnabled(); // Return the table info for the table with the specified UUID, if it exists. - scoped_refptr GetTableInfo(const TableId& table_id); - scoped_refptr GetTableInfoUnlocked(const TableId& table_id) REQUIRES_SHARED(lock_); + TableInfoPtr GetTableInfo(const TableId& table_id); + TableInfoPtr GetTableInfoUnlocked(const TableId& table_id) REQUIRES_SHARED(lock_); // Get Table info given namespace id and table name. // Does not work for YSQL tables because of possible ambiguity. scoped_refptr GetTableInfoFromNamespaceNameAndTableName( YQLDatabase db_type, const NamespaceName& namespace_name, const TableName& table_name); - // Return all the available TableInfo. The flag 'includeOnlyRunningTables' determines whether - // to retrieve all Tables irrespective of their state or just 'RUNNING' tables. - // To retrieve all live tables in the system, you should set this flag to true. - void GetAllTables(std::vector > *tables, - bool includeOnlyRunningTables = false); + // Return TableInfos according to specified mode. + std::vector GetTables(GetTablesMode mode); // Return all the available NamespaceInfo. The flag 'includeOnlyRunningNamespaces' determines // whether to retrieve all Namespaces irrespective of their state or just 'RUNNING' namespaces. @@ -535,7 +537,7 @@ class CatalogManager : public tserver::TabletPeerLookupIf { // Async tasks should call this when they finish. The last such tablet peer notification will // trigger trying to transition the table from DELETING to DELETED state. void NotifyTabletDeleteFinished( - const TabletServerId& tserver_uuid, const TableId& table_id, scoped_refptr table); + const TabletServerId& tserver_uuid, const TableId& table_id, const TableInfoPtr& table); // For a DeleteTable, we first mark tables as DELETING then move them to DELETED once all // outstanding tasks are complete and the TS side tablets are deleted. @@ -543,7 +545,7 @@ class CatalogManager : public tserver::TabletPeerLookupIf { // // If all conditions are met, returns a locked write lock on this table. // Otherwise lock is default constructed, i.e. not locked. - TableInfo::WriteLock MaybeTransitionTableToDeleted(TableInfoPtr table); + TableInfo::WriteLock MaybeTransitionTableToDeleted(const TableInfoPtr& table); // Used by ConsensusService to retrieve the TabletPeer for a system // table specified by 'tablet_id'. @@ -1086,6 +1088,12 @@ class CatalogManager : public tserver::TabletPeerLookupIf { TruncateTableResponsePB* resp, rpc::RpcContext* rpc); + struct DeletingTableData { + TableInfoPtr info; + TableInfo::WriteLock write_lock; + RepeatedBytes retained_by_snapshot_schedules; + }; + // Delete the specified table in memory. The TableInfo, DeletedTableInfo and lock of the deleted // table are appended to the lists. The caller will be responsible for committing the change and // deleting the actual table and tablets. @@ -1094,8 +1102,7 @@ class CatalogManager : public tserver::TabletPeerLookupIf { bool is_index_table, bool update_indexed_table, const SnapshotSchedulesToObjectIdsMap& schedules_to_tables_map, - std::vector>* tables, - std::vector* table_lcks, + std::vector* tables, DeleteTableResponsePB* resp, rpc::RpcContext* rpc); diff --git a/src/yb/master/master-path-handlers.cc b/src/yb/master/master-path-handlers.cc index bf02c2a81321..1fae17b44007 100644 --- a/src/yb/master/master-path-handlers.cc +++ b/src/yb/master/master-path-handlers.cc @@ -788,8 +788,7 @@ void MasterPathHandlers::HandleHealthCheck( jw.String("under_replicated_tablets"); jw.StartArray(); - vector> tables; - master_->catalog_manager()->GetAllTables(&tables, true /* include only running tables */); + auto tables = master_->catalog_manager()->GetTables(GetTablesMode::kRunning); for (const auto& table : tables) { // Ignore tables that are neither user tables nor user indexes. // However there are a bunch of system tables that still need to be investigated: @@ -867,8 +866,7 @@ void MasterPathHandlers::HandleCatalogManager(const Webserver::WebRequest& req, std::stringstream *output = &resp->output; master_->catalog_manager()->AssertLeaderLockAcquiredForReading(); - vector > tables; - master_->catalog_manager()->GetAllTables(&tables); + auto tables = master_->catalog_manager()->GetTables(GetTablesMode::kAll); bool has_tablegroups = master_->catalog_manager()->HasTablegroups(); @@ -880,8 +878,7 @@ void MasterPathHandlers::HandleCatalogManager(const Webserver::WebRequest& req, ordered_tables[i] = std::make_unique(); } - TableType table_cat; - for (const scoped_refptr& table : tables) { + for (const auto& table : tables) { auto l = table->LockForRead(); if (!l->is_running()) { continue; @@ -890,6 +887,7 @@ void MasterPathHandlers::HandleCatalogManager(const Webserver::WebRequest& req, string keyspace = master_->catalog_manager()->GetNamespaceName(table->namespace_id()); bool is_platform = keyspace.compare(kSystemPlatformNamespace) == 0; + TableType table_cat; // Determine the table category. YugaWare tables should be displayed as system tables. if (is_platform) { table_cat = kSystemTable; @@ -1162,8 +1160,7 @@ void MasterPathHandlers::HandleTablePage(const Webserver::WebRequest& req, void MasterPathHandlers::HandleTasksPage(const Webserver::WebRequest& req, Webserver::WebResponse* resp) { std::stringstream *output = &resp->output; - vector > tables; - master_->catalog_manager()->GetAllTables(&tables); + auto tables = master_->catalog_manager()->GetTables(GetTablesMode::kAll); *output << "

Active Tasks

\n"; *output << "\n"; *output << "
Task NameStateStart " @@ -1219,8 +1216,7 @@ std::vector MasterPathHandlers::GetNonSystemTablets() { master_->catalog_manager()->AssertLeaderLockAcquiredForReading(); - std::vector tables; - master_->catalog_manager()->GetAllTables(&tables, /* includeOnlyRunningTables */ true); + auto tables = master_->catalog_manager()->GetTables(GetTablesMode::kRunning); for (const auto& table : tables) { if (master_->catalog_manager()->IsSystemTable(*table.get())) { @@ -1403,8 +1399,7 @@ void MasterPathHandlers::RootHandler(const Webserver::WebRequest& req, } // Get all the tables. - vector > tables; - master_->catalog_manager()->GetAllTables(&tables, true /* includeOnlyRunningTables */); + auto tables = master_->catalog_manager()->GetTables(GetTablesMode::kRunning); // Get the list of user tables. vector > user_tables; @@ -1980,8 +1975,7 @@ string MasterPathHandlers::RegistrationToHtml( } void MasterPathHandlers::CalculateTabletMap(TabletCountMap* tablet_map) { - vector> tables; - master_->catalog_manager()->GetAllTables(&tables, true /* include only running tables */); + auto tables = master_->catalog_manager()->GetTables(GetTablesMode::kRunning); for (const auto& table : tables) { if (master_->catalog_manager()->IsColocatedUserTable(*table)) { // will be taken care of by colocated parent table diff --git a/src/yb/master/master.proto b/src/yb/master/master.proto index 3c81ca6ae585..44e82683e0da 100644 --- a/src/yb/master/master.proto +++ b/src/yb/master/master.proto @@ -355,6 +355,12 @@ message SysTablesEntryPB { DELETED = 5; } + enum HideState { + VISIBLE = 0; + HIDING = 1; + HIDDEN = 2; + } + // Table name required bytes name = 1; @@ -430,6 +436,8 @@ message SysTablesEntryPB { // This is typically set during alters that are caused due to index // permissions being changed, when the table is backfilled. optional bool updates_only_index_permissions = 31 [ default = false ]; + + optional HideState hide_state = 33; } // The data part of a SysRowEntry in the sys.catalog table for a namespace. @@ -1099,6 +1107,8 @@ message ListTablesRequestPB { // Can be used to filter tables based on RelationType repeated RelationType relation_type_filter = 4; + + optional bool include_not_running = 5; } message ListTablesResponsePB { @@ -1111,6 +1121,7 @@ message ListTablesResponsePB { optional TableType table_type = 3; optional NamespaceIdentifierPB namespace = 4; optional RelationType relation_type = 5 [default = USER_TABLE_RELATION]; + optional SysTablesEntryPB.State state = 6; } repeated TableInfo tables = 2; diff --git a/src/yb/master/master_service.cc b/src/yb/master/master_service.cc index 846f4f26ab2d..2115a476c0e5 100644 --- a/src/yb/master/master_service.cc +++ b/src/yb/master/master_service.cc @@ -280,8 +280,7 @@ void MasterServiceImpl::GetTabletLocations(const GetTabletLocationsRequestPB* re SleepFor(MonoDelta::FromMilliseconds(FLAGS_master_inject_latency_on_tablet_lookups_ms)); } if (PREDICT_FALSE(FLAGS_TEST_master_fail_transactional_tablet_lookups)) { - std::vector> tables; - server_->catalog_manager()->GetAllTables(&tables); + auto tables = server_->catalog_manager()->GetTables(GetTablesMode::kAll); const auto& tablet_id = req->tablet_ids(0); for (const auto& table : tables) { TabletInfos tablets; diff --git a/src/yb/master/master_snapshot_coordinator.cc b/src/yb/master/master_snapshot_coordinator.cc index bf3101446afb..999f0eaa8d87 100644 --- a/src/yb/master/master_snapshot_coordinator.cc +++ b/src/yb/master/master_snapshot_coordinator.cc @@ -693,7 +693,7 @@ class MasterSnapshotCoordinator::Impl { WARN_NOT_OK(ExecuteScheduleOperation(operation, leader_term), Format("Failed to execute operation on $0", operation.schedule_id)); } - context_.CleanupHiddenTablets(data.schedule_min_restore_time); + context_.CleanupHiddenObjects(data.schedule_min_restore_time); } SnapshotState* BoundingSnapshot(const SnapshotScheduleId& schedule_id, Bound bound) diff --git a/src/yb/master/snapshot_coordinator_context.h b/src/yb/master/snapshot_coordinator_context.h index def236e9a2ee..10d6780af1ac 100644 --- a/src/yb/master/snapshot_coordinator_context.h +++ b/src/yb/master/snapshot_coordinator_context.h @@ -73,7 +73,7 @@ class SnapshotCoordinatorContext { virtual CHECKED_STATUS RestoreSysCatalog(SnapshotScheduleRestoration* restoration) = 0; virtual CHECKED_STATUS VerifyRestoredObjects(const SnapshotScheduleRestoration& restoration) = 0; - virtual void CleanupHiddenTablets(const ScheduleMinRestoreTime& schedule_min_restore_time) = 0; + virtual void CleanupHiddenObjects(const ScheduleMinRestoreTime& schedule_min_restore_time) = 0; virtual const Schema& schema() = 0; diff --git a/src/yb/master/sys_catalog-internal.h b/src/yb/master/sys_catalog-internal.h index 9ad642b2a363..ee0901cf8e12 100644 --- a/src/yb/master/sys_catalog-internal.h +++ b/src/yb/master/sys_catalog-internal.h @@ -73,14 +73,14 @@ CHECKED_STATUS SysCatalogTable::AddItem(Item* item, int64_t leader_term) { } template -CHECKED_STATUS SysCatalogTable::AddItems(const vector& items, int64_t leader_term) { +CHECKED_STATUS SysCatalogTable::AddItems(const vector& items, int64_t leader_term) { return MutateItems(items, QLWriteRequestPB::QL_STMT_INSERT, leader_term); } template CHECKED_STATUS SysCatalogTable::AddAndUpdateItems( - const vector& added_items, - const vector& updated_items, + const vector& added_items, + const vector& updated_items, int64_t leader_term) { auto w = NewWriter(leader_term); for (const auto& item : added_items) { @@ -100,7 +100,7 @@ CHECKED_STATUS SysCatalogTable::UpdateItem(Item* item, int64_t leader_term) { } template -CHECKED_STATUS SysCatalogTable::UpdateItems(const vector& items, int64_t leader_term) { +CHECKED_STATUS SysCatalogTable::UpdateItems(const vector& items, int64_t leader_term) { return MutateItems(items, QLWriteRequestPB::QL_STMT_UPDATE, leader_term); } @@ -112,13 +112,13 @@ CHECKED_STATUS SysCatalogTable::DeleteItem(Item* item, int64_t leader_term) { } template -CHECKED_STATUS SysCatalogTable::DeleteItems(const vector& items, int64_t leader_term) { +CHECKED_STATUS SysCatalogTable::DeleteItems(const vector& items, int64_t leader_term) { return MutateItems(items, QLWriteRequestPB::QL_STMT_DELETE, leader_term); } template CHECKED_STATUS SysCatalogTable::MutateItems( - const vector& items, const QLWriteRequestPB::QLStmtType& op_type, int64_t leader_term) { + const vector& items, const QLWriteRequestPB::QLStmtType& op_type, int64_t leader_term) { auto w = NewWriter(leader_term); for (const auto& item : items) { RETURN_NOT_OK(w->MutateItem(item, op_type)); diff --git a/src/yb/master/sys_catalog.h b/src/yb/master/sys_catalog.h index af95a0033bc0..28711f1f78c4 100644 --- a/src/yb/master/sys_catalog.h +++ b/src/yb/master/sys_catalog.h @@ -106,25 +106,25 @@ class SysCatalogTable { template CHECKED_STATUS AddItem(Item* item, int64_t leader_term); template - CHECKED_STATUS AddItems(const vector& items, int64_t leader_term); + CHECKED_STATUS AddItems(const vector& items, int64_t leader_term); template CHECKED_STATUS UpdateItem(Item* item, int64_t leader_term); template - CHECKED_STATUS UpdateItems(const vector& items, int64_t leader_term); + CHECKED_STATUS UpdateItems(const vector& items, int64_t leader_term); template - CHECKED_STATUS AddAndUpdateItems(const vector& added_items, - const vector& updated_items, + CHECKED_STATUS AddAndUpdateItems(const vector& added_items, + const vector& updated_items, int64_t leader_term); template CHECKED_STATUS DeleteItem(Item* item, int64_t leader_term); template - CHECKED_STATUS DeleteItems(const vector& items, int64_t leader_term); + CHECKED_STATUS DeleteItems(const vector& items, int64_t leader_term); template - CHECKED_STATUS MutateItems(const vector& items, + CHECKED_STATUS MutateItems(const vector& items, const QLWriteRequestPB::QLStmtType& op_type, int64_t leader_term); diff --git a/src/yb/master/sys_catalog_writer.h b/src/yb/master/sys_catalog_writer.h index 0c5bc9185eed..e5e9adae80a4 100644 --- a/src/yb/master/sys_catalog_writer.h +++ b/src/yb/master/sys_catalog_writer.h @@ -43,6 +43,12 @@ class SysCatalogWriter { PersistentDataEntryClass::type(), item->id(), old_pb, new_pb, op_type); } + template + CHECKED_STATUS MutateItem(const scoped_refptr& item, + QLWriteRequestPB::QLStmtType op_type) { + return MutateItem(item.get(), op_type); + } + // Insert a row into a Postgres sys catalog table. CHECKED_STATUS InsertPgsqlTableRow(const Schema& source_schema, const QLTableRow& source_row, diff --git a/src/yb/master/yql_columns_vtable.cc b/src/yb/master/yql_columns_vtable.cc index afec58046cb0..1a922377ff96 100644 --- a/src/yb/master/yql_columns_vtable.cc +++ b/src/yb/master/yql_columns_vtable.cc @@ -51,8 +51,7 @@ Status YQLColumnsVTable::PopulateColumnInformation(const Schema& schema, Result> YQLColumnsVTable::RetrieveData( const QLReadRequestPB& request) const { auto vtable = std::make_shared(schema_); - std::vector > tables; - master_->catalog_manager()->GetAllTables(&tables, true); + auto tables = master_->catalog_manager()->GetTables(GetTablesMode::kVisibleToClient); for (scoped_refptr table : tables) { // Skip non-YQL tables. diff --git a/src/yb/master/yql_indexes_vtable.cc b/src/yb/master/yql_indexes_vtable.cc index 199779cdaa4e..46b6929fe010 100644 --- a/src/yb/master/yql_indexes_vtable.cc +++ b/src/yb/master/yql_indexes_vtable.cc @@ -39,11 +39,10 @@ const string& ColumnName(const Schema& schema, const ColumnId id) { Result> YQLIndexesVTable::RetrieveData( const QLReadRequestPB& request) const { auto vtable = std::make_shared(schema_); - std::vector> tables; CatalogManager* catalog_manager = master_->catalog_manager(); - catalog_manager->GetAllTables(&tables, true); - for (scoped_refptr table : tables) { + auto tables = master_->catalog_manager()->GetTables(GetTablesMode::kVisibleToClient); + for (const auto& table : tables) { const auto indexed_table_id = table->indexed_table_id(); if (indexed_table_id.empty()) { continue; diff --git a/src/yb/master/yql_partitions_vtable.cc b/src/yb/master/yql_partitions_vtable.cc index 224baa31d7aa..1b90268092cf 100644 --- a/src/yb/master/yql_partitions_vtable.cc +++ b/src/yb/master/yql_partitions_vtable.cc @@ -85,8 +85,7 @@ Status YQLPartitionsVTable::GenerateAndCacheData() const { } auto vtable = std::make_shared(schema_); - std::vector > tables; - catalog_manager->GetAllTables(&tables, true /* includeOnlyRunningTables */); + auto tables = master_->catalog_manager()->GetTables(GetTablesMode::kVisibleToClient); auto& resolver = master_->messenger()->resolver(); std::unordered_map>> dns_lookups; diff --git a/src/yb/master/yql_size_estimates_vtable.cc b/src/yb/master/yql_size_estimates_vtable.cc index a2a7478bddec..ff4b4c4e7e27 100644 --- a/src/yb/master/yql_size_estimates_vtable.cc +++ b/src/yb/master/yql_size_estimates_vtable.cc @@ -31,11 +31,10 @@ YQLSizeEstimatesVTable::YQLSizeEstimatesVTable(const TableName& table_name, Result> YQLSizeEstimatesVTable::RetrieveData( const QLReadRequestPB& request) const { auto vtable = std::make_shared(schema_); - std::vector > tables; CatalogManager* catalog_manager = master_->catalog_manager(); - catalog_manager->GetAllTables(&tables, true); - for (scoped_refptr table : tables) { + auto tables = master_->catalog_manager()->GetTables(GetTablesMode::kVisibleToClient); + for (const auto& table : tables) { Schema schema; RETURN_NOT_OK(table->GetSchema(&schema)); diff --git a/src/yb/master/yql_tables_vtable.cc b/src/yb/master/yql_tables_vtable.cc index bf9a89af0310..916135ad5fa4 100644 --- a/src/yb/master/yql_tables_vtable.cc +++ b/src/yb/master/yql_tables_vtable.cc @@ -28,10 +28,9 @@ YQLTablesVTable::YQLTablesVTable(const TableName& table_name, Result> YQLTablesVTable::RetrieveData( const QLReadRequestPB& request) const { auto vtable = std::make_shared(schema_); - std::vector > tables; - master_->catalog_manager()->GetAllTables(&tables, true); - for (scoped_refptr table : tables) { + auto tables = master_->catalog_manager()->GetTables(GetTablesMode::kVisibleToClient); + for (const auto& table : tables) { // Skip non-YQL tables. if (!CatalogManager::IsYcqlTable(*table)) { continue; diff --git a/src/yb/tools/yb-admin-snapshot-schedule-test.cc b/src/yb/tools/yb-admin-snapshot-schedule-test.cc index 4c566bf0317d..423e7c8d4284 100644 --- a/src/yb/tools/yb-admin-snapshot-schedule-test.cc +++ b/src/yb/tools/yb-admin-snapshot-schedule-test.cc @@ -210,6 +210,8 @@ class YbAdminSnapshotScheduleTest : public AdminTestBase { return schedule_id; } + void TestUndeleteTable(bool restart_masters); + std::unique_ptr cql_driver_; }; @@ -261,7 +263,7 @@ TEST_F(YbAdminSnapshotScheduleTest, Basic) { ASSERT_OK(RestoreSnapshotSchedule(schedule_id, last_snapshot_time)); } -TEST_F(YbAdminSnapshotScheduleTest, UndeleteTable) { +void YbAdminSnapshotScheduleTest::TestUndeleteTable(bool restart_masters) { auto schedule_id = ASSERT_RESULT(PrepareQl()); auto session = client_->NewSession(); @@ -283,9 +285,20 @@ TEST_F(YbAdminSnapshotScheduleTest, UndeleteTable) { ASSERT_NOK(client::kv_table_test::WriteRow(&table_, session, kMinKey, 0)); + ASSERT_NO_FATALS(client::kv_table_test::CreateTable( + client::Transactional::kTrue, 3, client_.get(), &table_)); + + ASSERT_OK(client::kv_table_test::WriteRow(&table_, session, kMinKey, 0)); + + if (restart_masters) { + ASSERT_OK(RestartAllMasters(cluster_.get())); + } + LOG(INFO) << "Restore schedule"; ASSERT_OK(RestoreSnapshotSchedule(schedule_id, time)); + ASSERT_OK(table_.Open(client::kTableName, client_.get())); + LOG(INFO) << "Reading rows"; auto rows = ASSERT_RESULT(client::kv_table_test::SelectAllRows(&table_, session)); LOG(INFO) << "Rows: " << AsString(rows); @@ -300,6 +313,14 @@ TEST_F(YbAdminSnapshotScheduleTest, UndeleteTable) { ASSERT_EQ(extra_value, -kExtraKey); } +TEST_F(YbAdminSnapshotScheduleTest, UndeleteTable) { + TestUndeleteTable(false); +} + +TEST_F(YbAdminSnapshotScheduleTest, UndeleteTableWithRestart) { + TestUndeleteTable(true); +} + TEST_F(YbAdminSnapshotScheduleTest, CleanupDeletedTablets) { auto schedule_id = ASSERT_RESULT(PrepareQl(kInterval)); @@ -319,6 +340,8 @@ TEST_F(YbAdminSnapshotScheduleTest, CleanupDeletedTablets) { ASSERT_OK(client_->DeleteTable(client::kTableName)); auto deadline = CoarseMonoClock::now() + kInterval + 10s; + + // Wait tablets deleted from tservers. ASSERT_OK(Wait([this, deadline]() -> Result { for (int i = 0; i != cluster_->num_tablet_servers(); ++i) { auto proxy = cluster_->GetTServerProxy(i); @@ -329,13 +352,33 @@ TEST_F(YbAdminSnapshotScheduleTest, CleanupDeletedTablets) { RETURN_NOT_OK(proxy->ListTablets(req, &resp, &controller)); for (const auto& tablet : resp.status_and_schema()) { if (tablet.tablet_status().table_type() != TableType::TRANSACTION_STATUS_TABLE_TYPE) { - LOG(INFO) << "Not yet deleted: " << tablet.ShortDebugString(); + LOG(INFO) << "Not yet deleted tablet: " << tablet.ShortDebugString(); return false; } } } return true; }, deadline, "Deleted tablet cleanup")); + + // Wait table marked as deleted. + ASSERT_OK(Wait([this, deadline]() -> Result { + auto proxy = cluster_->GetLeaderMasterProxy(); + master::ListTablesRequestPB req; + master::ListTablesResponsePB resp; + rpc::RpcController controller; + controller.set_deadline(deadline); + req.set_include_not_running(true); + RETURN_NOT_OK(proxy->ListTables(req, &resp, &controller)); + for (const auto& table : resp.tables()) { + if (table.table_type() != TableType::TRANSACTION_STATUS_TABLE_TYPE + && table.relation_type() != master::RelationType::SYSTEM_TABLE_RELATION + && table.state() != master::SysTablesEntryPB::DELETED) { + LOG(INFO) << "Not yet deleted table: " << table.ShortDebugString(); + return false; + } + } + return true; + }, deadline, "Deleted table cleanup")); } TEST_F_EX(YbAdminSnapshotScheduleTest, YB_DISABLE_TEST_IN_TSAN(Pgsql),