[#23428] docdb: Remove non-transactional snapshot code

Summary:
Transactional snapshots have been supported (and the default) since D8100 and D8349. The non-transactional snapshot flow is untested and clutters the transactional snapshots. To the best of our knowledge, it is unused. This diff removes the non-transactional snapshot code.

**Upgrade/Rollback safety:**
This diff deprecates the `transaction_aware` field in `CreateSnapshotRequestPB` and the `current_snapshot_id` field in `ListSnapshotsResponsePB`. Since masters are upgraded before tservers, the `transaction_aware` aware field will always be ignored (the new behavior) on an upgrade / downgrade. The `current_snapshot_id` field is only set for non-transactional snapshots (which are not being used), so that field's behavior does not change.
Jira: DB-12349

Test Plan: Existing tests

Reviewers: zdrudi, hsunder, loginov

Reviewed By: zdrudi, hsunder, loginov

Subscribers: loginov, ybase

Differential Revision: https://phorge.dev.yugabyte.com/D37010

src/yb/client/client-internal.cc

@@ -1819,7 +1819,6 @@ class CreateSnapshotRpc
       table.SetIntoTableIdentifierPB(&id);
       req_.mutable_tables()->Add()->Swap(&id);
     }
-    req_.set_transaction_aware(true);
     return Status::OK();
   }
 

src/yb/client/snapshot_test_util.cc

@@ -247,7 +247,6 @@ Result<TxnSnapshotId> SnapshotTestUtil::DoStartSnapshot(const F& fill_tables) {
   rpc::RpcController controller;
   controller.set_timeout(60s);
   master::CreateSnapshotRequestPB req;
-  req.set_transaction_aware(true);
   fill_tables(&req);
   master::CreateSnapshotResponsePB resp;
   RETURN_NOT_OK(VERIFY_RESULT(MakeBackupServiceProxy()).CreateSnapshot(req, &resp, &controller));

src/yb/integration-tests/snapshot-test.cc

@@ -180,9 +180,6 @@ class SnapshotTest : public YBMiniClusterTestBase<MiniCluster> {
     LOG(INFO) << "Number of snapshots: " << list_resp.snapshots_size();
     ASSERT_EQ(list_resp.snapshots_size(), snapshot_info.size());
 
-    // Current snapshot is available for non-transaction aware snapshots only.
-    ASSERT_FALSE(list_resp.has_current_snapshot_id());
-
     for (int i = 0; i < list_resp.snapshots_size(); ++i) {
       LOG(INFO) << "Snapshot " << i << ": " << list_resp.snapshots(i).DebugString();
       auto id = ASSERT_RESULT(FullyDecodeTxnSnapshotId(list_resp.snapshots(i).id()));
@@ -270,7 +267,6 @@ class SnapshotTest : public YBMiniClusterTestBase<MiniCluster> {
       std::optional<bool> imported = std::nullopt) {
     CreateSnapshotRequestPB req;
     CreateSnapshotResponsePB resp;
-    req.set_transaction_aware(true);
     TableIdentifierPB* const table = req.mutable_tables()->Add();
     table->set_table_name(kTableName.table_name());
     table->mutable_namespace_()->set_name(kTableName.namespace_name());

src/yb/master/async_snapshot_tasks.cc

@@ -140,46 +140,7 @@ void AsyncTabletSnapshotOp::HandleResponse(int attempt) {
 
   if (state() != server::MonitoredTaskState::kComplete) {
     VLOG_WITH_PREFIX(1) << "TabletSnapshotOp task is not completed";
-    return;
-  }
-
-  switch (operation_) {
-    case tserver::TabletSnapshotOpRequestPB::CREATE_ON_TABLET: {
-      // TODO: this class should not know CatalogManager API,
-      //       remove circular dependency between classes.
-      master_->catalog_manager()->HandleCreateTabletSnapshotResponse(
-          tablet_.get(), resp_.has_error());
-      return;
-    }
-    case tserver::TabletSnapshotOpRequestPB::RESTORE_ON_TABLET: {
-      // TODO: this class should not know CatalogManager API,
-      //       remove circular dependency between classes.
-      master_->catalog_manager()->HandleRestoreTabletSnapshotResponse(
-          tablet_.get(), resp_.has_error());
-      return;
-    }
-    case tserver::TabletSnapshotOpRequestPB::DELETE_ON_TABLET: {
-      // TODO: this class should not know CatalogManager API,
-      //       remove circular dependency between classes.
-      // HandleDeleteTabletSnapshotResponse handles only non transaction aware snapshots.
-      // So prevent log flooding for transaction aware snapshots.
-      if (!TryFullyDecodeTxnSnapshotId(snapshot_id_)) {
-        master_->catalog_manager()->HandleDeleteTabletSnapshotResponse(
-            snapshot_id_, tablet_.get(), resp_.has_error());
-      }
-      return;
-    }
-    case tserver::TabletSnapshotOpRequestPB::RESTORE_FINISHED:
-      return;
-    case tserver::TabletSnapshotOpRequestPB::CREATE_ON_MASTER: FALLTHROUGH_INTENDED;
-    case tserver::TabletSnapshotOpRequestPB::DELETE_ON_MASTER: FALLTHROUGH_INTENDED;
-    case tserver::TabletSnapshotOpRequestPB::RESTORE_SYS_CATALOG: FALLTHROUGH_INTENDED;
-    case google::protobuf::kint32min: FALLTHROUGH_INTENDED;
-    case google::protobuf::kint32max: FALLTHROUGH_INTENDED;
-    case tserver::TabletSnapshotOpRequestPB::UNKNOWN: break; // Not handled.
   }
-
-  FATAL_INVALID_ENUM_VALUE(tserver::TabletSnapshotOpRequestPB::Operation, operation_);
 }
 
 bool AsyncTabletSnapshotOp::SendRequest(int attempt) {

src/yb/master/catalog_manager.cc

@@ -891,47 +891,6 @@ Result<QLWriteRequestPB::QLStmtType> ToQLStmtType(
 
 }  // anonymous namespace
 
-////////////////////////////////////////////////////////////
-// Snapshot Loader
-////////////////////////////////////////////////////////////
-
-class SnapshotLoader : public Visitor<PersistentSnapshotInfo> {
- public:
-  explicit SnapshotLoader(CatalogManager* catalog_manager) : catalog_manager_(catalog_manager) {}
-
-  Status Visit(const SnapshotId& snapshot_id, const SysSnapshotEntryPB& metadata) override {
-    if (TryFullyDecodeTxnSnapshotId(snapshot_id)) {
-      // Transaction aware snapshots should be already loaded.
-      return Status::OK();
-    }
-    return VisitNonTransactionAwareSnapshot(snapshot_id, metadata);
-  }
-
-  Status VisitNonTransactionAwareSnapshot(
-      const SnapshotId& snapshot_id, const SysSnapshotEntryPB& metadata) {
-    // Setup the snapshot info.
-    auto snapshot_info = make_scoped_refptr<SnapshotInfo>(snapshot_id);
-    auto l = snapshot_info->LockForWrite();
-    l.mutable_data()->pb.CopyFrom(metadata);
-
-    // Add the snapshot to the IDs map (if the snapshot is not deleted).
-    auto emplace_result =
-        catalog_manager_->non_txn_snapshot_ids_map_.emplace(snapshot_id, std::move(snapshot_info));
-    CHECK(emplace_result.second) << "Snapshot already exists: " << snapshot_id;
-
-    LOG(INFO) << "Loaded metadata for snapshot (id=" << snapshot_id
-              << "): " << emplace_result.first->second->ToString() << ": "
-              << metadata.ShortDebugString();
-    l.Commit();
-    return Status::OK();
-  }
-
- private:
-  CatalogManager* catalog_manager_;
-
-  DISALLOW_COPY_AND_ASSIGN(SnapshotLoader);
-};
-
 ////////////////////////////////////////////////////////////
 // CatalogManager
 ////////////////////////////////////////////////////////////
@@ -1504,14 +1463,6 @@ Status CatalogManager::RunLoaders(SysCatalogLoadingState* state) {
     RETURN_NOT_OK(InitializeTransactionTablesConfig(state->epoch.leader_term));
   }
 
-  // Clear the snapshots.
-  non_txn_snapshot_ids_map_.clear();
-
-  LOG_WITH_FUNC(INFO) << "Loading snapshots into memory.";
-  unique_ptr<SnapshotLoader> snapshot_loader(new SnapshotLoader(this));
-  RETURN_NOT_OK_PREPEND(
-      sys_catalog_->Visit(snapshot_loader.get()), "Failed while visiting snapshots in sys catalog");
-
   RETURN_NOT_OK(LoadXReplStream());
   RETURN_NOT_OK(LoadUniverseReplication());
   RETURN_NOT_OK(LoadUniverseReplicationBootstrap());

src/yb/master/catalog_manager.h

@@ -1290,13 +1290,6 @@ class CatalogManager : public tserver::TabletPeerLookupIf,
       rpc::RpcContext* rpc,
       const LeaderEpoch& epoch);
 
-  void HandleCreateTabletSnapshotResponse(TabletInfo* tablet, bool error) override;
-
-  void HandleRestoreTabletSnapshotResponse(TabletInfo* tablet, bool error) override;
-
-  void HandleDeleteTabletSnapshotResponse(
-      const SnapshotId& snapshot_id, TabletInfo* tablet, bool error) override;
-
   // Is encryption at rest enabled for this cluster.
   Status IsEncryptionEnabled(
       const IsEncryptionEnabledRequestPB* req, IsEncryptionEnabledResponsePB* resp);
@@ -2395,7 +2388,6 @@ class CatalogManager : public tserver::TabletPeerLookupIf,
     const TSDescriptorVector& ts_descs);
 
  private:
-  friend class SnapshotLoader;
   friend class yb::master::ClusterLoadBalancer;
   friend class CDCStreamLoader;
   friend class UniverseReplicationLoader;
@@ -2764,14 +2756,6 @@ class CatalogManager : public tserver::TabletPeerLookupIf,
 
   std::unordered_set<xrepl::StreamId> GetCDCSDKStreamsForTable(const TableId& table_id) const;
 
-  Status CreateNonTransactionAwareSnapshot(
-      const CreateSnapshotRequestPB* req, CreateSnapshotResponsePB* resp, const LeaderEpoch& epoch);
-
-  Status RestoreNonTransactionAwareSnapshot(
-      const SnapshotId& snapshot_id, const LeaderEpoch& epoch);
-
-  Status DeleteNonTransactionAwareSnapshot(const SnapshotId& snapshot_id, const LeaderEpoch& epoch);
-
   Status AddNamespaceEntriesToPB(
       const std::vector<TableDescription>& tables,
       google::protobuf::RepeatedPtrField<SysRowEntry>* out,
@@ -2944,11 +2928,6 @@ class CatalogManager : public tserver::TabletPeerLookupIf,
   std::unordered_map<std::string, std::unique_ptr<DynamicAsyncTaskThrottler>>
     delete_replica_task_throttler_per_ts_ GUARDED_BY(delete_replica_task_throttler_per_ts_mutex_);
 
-  // Snapshot map: snapshot-id -> SnapshotInfo.
-  typedef std::unordered_map<SnapshotId, scoped_refptr<SnapshotInfo>> SnapshotInfoMap;
-  SnapshotInfoMap non_txn_snapshot_ids_map_;
-  SnapshotId current_snapshot_id_;
-
   // mutex on should_send_universe_key_registry_mutex_.
   mutable simple_spinlock should_send_universe_key_registry_mutex_;
   // Should catalog manager resend latest universe key registry to tserver.