[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 / c221319

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

@@ -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;

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<TabletInfoPtr>& hidden_tablets,
+      const ScheduleMinRestoreTime& schedule_min_restore_time);
+  // Will filter tables content, so pass it by value here.
+  void CleanupHiddenTables(std::vector<TableInfoPtr> tables);
 
   rpc::Scheduler& Scheduler() override;
 

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<TabletInfoPtr> hidden_tablets;
+  std::vector<TableInfoPtr> tables;
   {
     SharedLock<LockType> 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<TabletInfoPtr>& hidden_tablets,
+    const ScheduleMinRestoreTime& schedule_min_restore_time) {
+  if (hidden_tablets.empty()) {
+    return;
   }
   std::vector<TabletInfoPtr> tablets_to_delete;
   std::vector<TabletInfoPtr> 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<LockType> l(lock_);
@@ -1573,6 +1589,39 @@ void CatalogManager::CleanupHiddenTablets(const ScheduleMinRestoreTime& schedule
   }
 }
 
+void CatalogManager::CleanupHiddenTables(std::vector<TableInfoPtr> tables) {
+  std::vector<TableInfo::WriteLock> 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();
 }

src/yb/client/ql-tablet-test.cc

@@ -325,10 +325,8 @@ class QLTabletTest : public QLDmlTestBase<MiniCluster> {
 
   scoped_refptr<master::TableInfo> GetTableInfo(const YBTableName& table_name) {
     auto* catalog_manager = cluster_->leader_mini_master()->master()->catalog_manager();
-    std::vector<scoped_refptr<master::TableInfo>> all_tables;
-    catalog_manager->GetAllTables(&all_tables);
-    scoped_refptr<master::TableInfo> 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;
       }

src/yb/gutil/stl_util.h

@@ -993,4 +993,10 @@ std::set<T> VectorToSet(const std::vector<T>& v) {
   return std::set<T>(v.begin(), v.end());
 }
 
+template <class Predicate, class Collection>
+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

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<scoped_refptr<master::TableInfo> > 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<master::TableInfo>& table_info : tables) {
     LOG(INFO) << "Table: " << table_info->ToString();
     std::vector<scoped_refptr<master::TabletInfo> > tablets;

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

src/yb/integration-tests/external_mini_cluster.h

@@ -359,6 +359,11 @@ class ExternalMiniCluster : public MiniClusterBase {
     return GetProxy<T>(master(i));
   }
 
+  template <class T>
+  std::shared_ptr<T> GetLeaderMasterProxy() {
+    return GetProxy<T>(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::MasterServiceProxy> master_proxy();
@@ -820,5 +825,7 @@ std::shared_ptr<T> ExternalMiniCluster::GetProxy(ExternalDaemon* daemon) {
   return std::make_shared<T>(proxy_cache_.get(), daemon->bound_rpc_addr());
 }
 
+CHECKED_STATUS RestartAllMasters(ExternalMiniCluster* cluster);
+
 }  // namespace yb
 #endif  // YB_INTEGRATION_TESTS_EXTERNAL_MINI_CLUSTER_H_

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) {

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<decltype(lock_)> l(lock_);
-  for (const TableInfo::TabletInfoMap::value_type& e : tablet_map_) {
+  return !tablet_map_.empty();
+}
+
+bool TableInfo::AreAllTabletsDeletedOrHidden() const {
+  shared_lock<decltype(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<decltype(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<decltype(lock_)> l(lock_);
   for (const TableInfo::TabletInfoMap::value_type& e : tablet_map_) {

src/yb/master/catalog_entity_info.h

@@ -312,6 +312,22 @@ struct PersistentTableInfo : public Persistent<SysTablesEntryPB, SysRowEntry::TA
            pb.state() == SysTablesEntryPB::ALTERING;
   }
 
+  bool visible_to_client() const {
+    return is_running() && !is_hidden();
+  }
+
+  bool is_hiding() const {
+    return pb.hide_state() == SysTablesEntryPB::HIDING;
+  }
+
+  bool is_hidden() const {
+    return pb.hide_state() == SysTablesEntryPB::HIDDEN;
+  }
+
+  bool started_hiding() const {
+    return is_hiding() || is_hidden();
+  }
+
   // Return the table's name.
   const TableName& name() const {
     return pb.name();
@@ -412,6 +428,8 @@ class TableInfo : public RefCountedThreadSafe<TableInfo>,
   // 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<TableInfo>,
   // 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;
 

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<TableInfo> 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<TableId> 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<TableInfo> 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()));
     }