Replace the backup disk with a soft_delete flag on object storage disks

src/CMakeLists.txt

@@ -181,7 +181,7 @@ endif()
 
 add_headers_and_sources(dbms Databases/DataLake)
 
-add_headers_and_sources(dbms Disks/DiskObjectStorage/ObjectStorages/Backup)
+add_headers_and_sources(dbms Disks/DiskObjectStorage/ObjectStorages/SoftDelete)
 add_headers_and_sources(dbms Disks/DiskObjectStorage/ObjectStorages/Cached)
 add_headers_and_sources(dbms Disks/DiskObjectStorage/ObjectStorages/Local)
 add_headers_and_sources(dbms Disks/DiskObjectStorage/ObjectStorages/Web)

src/Disks/DiskFactory.cpp

@@ -4,6 +4,7 @@ namespace DB
 {
 namespace ErrorCodes
 {
+    extern const int BAD_ARGUMENTS;
     extern const int LOGICAL_ERROR;
     extern const int UNKNOWN_ELEMENT_IN_CONFIG;
 }
@@ -20,6 +21,11 @@ void DiskFactory::registerDiskType(const String & disk_type, Creator creator)
         throw Exception(ErrorCodes::LOGICAL_ERROR, "DiskFactory: the disk type '{}' is not unique", disk_type);
 }
 
+void DiskFactory::markSoftDeleteCapable(const String & disk_type)
+{
+    soft_delete_capable_types.insert(disk_type);
+}
+
 DiskPtr DiskFactory::create(
     const String & name,
     const Poco::Util::AbstractConfiguration & config,
@@ -44,12 +50,26 @@ DiskPtr DiskFactory::create(
         return nullptr;
     }
 
+    /// `soft_delete` is honoured only by the disk that owns the blobs. A layer above it delegates the
+    /// removal downwards, so the blob would be physically unlinked despite the flag; reject such a
+    /// config rather than let it silently do nothing. The check lives here, at the single point every
+    /// disk is created, so it cannot fall out of step as disk types are added.
+    if (config.getBool(config_prefix + ".soft_delete", false) && !soft_delete_capable_types.contains(disk_type))
+    {
+        throw Exception(
+            ErrorCodes::BAD_ARGUMENTS,
+            "Disk `{}` of type `{}` does not support `soft_delete`. Set it on the object storage disk "
+            "that holds the blobs, not on a layer above it",
+            name, disk_type);
+    }
+
     const auto & disk_creator = found->second;
     return disk_creator(name, config, config_prefix, context, map, attach, custom_disk);
 }
 
 void DiskFactory::clearRegistry()
 {
     registry.clear();
+    soft_delete_capable_types.clear();
 }
 }

src/Disks/DiskFactory.h

@@ -10,6 +10,7 @@
 #include <functional>
 #include <map>
 #include <unordered_map>
+#include <unordered_set>
 
 
 namespace DB
@@ -35,6 +36,10 @@ class DiskFactory final : private boost::noncopyable
 
     void registerDiskType(const String & disk_type, Creator creator);
 
+    /// Declares that this disk type's creator honours `soft_delete`; every other type is rejected
+    /// when the flag is set. Call it wherever the type is registered, so the two cannot drift apart.
+    void markSoftDeleteCapable(const String & disk_type);
+
     DiskPtr create(
         const String & name,
         const Poco::Util::AbstractConfiguration & config,
@@ -50,6 +55,7 @@ class DiskFactory final : private boost::noncopyable
 private:
     using DiskTypeRegistry = std::unordered_map<String, Creator>;
     DiskTypeRegistry registry;
+    std::unordered_set<String> soft_delete_capable_types;
 };
 
 }

src/Disks/DiskObjectStorage/DiskObjectStorage.cpp

@@ -16,9 +16,6 @@
 #include <Disks/IO/ReadBufferFromRemoteFSGather.h>
 #include <Disks/IO/AsynchronousBoundedReadBuffer.h>
 #include <Disks/DiskObjectStorage/DiskObjectStorageTransaction.h>
-#include <Disks/DiskObjectStorage/ObjectStorages/Backup/BackupObjectStorage.h>
-#include <Disks/DiskObjectStorage/MetadataStorages/Local/MetadataStorageFromDisk.h>
-#include <Disks/DiskObjectStorage/MetadataStorages/Cache/MetadataStorageFromCacheObjectStorage.h>
 #include <Disks/DiskObjectStorage/Replication/BlobKillerThread.h>
 #include <Disks/DiskObjectStorage/Replication/BlobCopierThread.h>
 #include <Disks/FakeDiskTransaction.h>
@@ -47,25 +44,6 @@ namespace ErrorCodes
 {
     extern const int INCORRECT_DISK_INDEX;
     extern const int CANNOT_RMDIR;
-    extern const int BAD_ARGUMENTS;
-}
-
-namespace
-{
-    /// Turn off the in-memory removal queue of a metadata storage below a backup layer (see
-    /// `wrapWithBackup`). Reached by concrete type rather than a virtual on `IMetadataStorage` on
-    /// purpose: adding a virtual would shift every subclass's vtable slots and force a full rebuild;
-    /// only these two concrete storages own a removal queue. An unrecognized type simply keeps
-    /// recording (correct, just not optimized) — assert in debug so a new queue-holder is not missed.
-    void stopRecordingRemovals(IMetadataStorage & metadata_storage)
-    {
-        if (auto * from_disk = dynamic_cast<MetadataStorageFromDisk *>(&metadata_storage))
-            from_disk->setRecordRemovals(false);
-        else if (auto * from_cache = dynamic_cast<MetadataStorageFromCacheObjectStorage *>(&metadata_storage))
-            from_cache->setRecordRemovals(false);
-        else
-            chassert(false && "backup layer wraps a metadata storage with an undrained removal queue");
-    }
 }
 
 DiskTransactionPtr DiskObjectStorage::createTransaction()
@@ -80,78 +58,6 @@ ObjectStoragePtr DiskObjectStorage::getObjectStorage()
     return object_storages->takePointingTo(cluster->getLocalLocation());
 }
 
-DiskObjectStoragePtr DiskObjectStorage::wrapWithBackup(const String & layer_name, const String & backup_base_path) const
-{
-    /// The backup layer soft-deletes objects only at the local location's object storage. On a
-    /// multi-location (replicated) disk the background blob replication/GC would not observe those
-    /// soft-deletes, so the external-GC contract would be unsound. We only support single-location
-    /// object storage disks (tiered storage); reject the unsupported multi-location case explicitly.
-    if (cluster->getConfiguration().size() > 1)
-        throw Exception(
-            ErrorCodes::BAD_ARGUMENTS,
-            "Cannot wrap disk with backup layer `{}`: backup is only supported on single-location "
-            "object storage disks, but the wrapped disk has {} locations",
-            layer_name, cluster->getConfiguration().size());
-
-    auto registry = object_storages->getRegistry();
-    auto local_location = cluster->getLocalLocation();
-    registry[local_location] = std::make_shared<BackupObjectStorage>(registry[local_location], backup_base_path, layer_name);
-
-    /// Backup only intercepts object deletions, not metadata, so metadata_storage is passed through unchanged.
-    auto backup_disk = std::make_shared<DiskObjectStorage>(
-        layer_name,
-        std::make_shared<ClusterConfiguration>(layer_name, cluster->getConfiguration()),
-        metadata_storage,
-        std::make_shared<ObjectStorageRouter>(std::move(registry)),
-        std::dynamic_pointer_cast<const DiskObjectStorage>(shared_from_this()),
-        Context::getGlobalContextInstance()->getConfigRef(),
-        "storage_configuration.disks." + layer_name,
-        use_fake_transaction);
-
-    /// Deferred-delete correctness.
-    ///
-    /// On 26.3 blob deletion is deferred: a DROP/merge enqueues orphaned blobs into an in-memory
-    /// removal queue owned by each disk's `metadata_storage`, and that disk's background
-    /// `BlobKillerThread` later drains it via
-    /// `object_storages->takePointingTo(location)->removeObjectsIfExist(...)`. In a stack
-    /// (`cache -> object_storage`) EACH layer owns its own queue: a commit fills the base queue and,
-    /// as it unwinds, each wrapper copies the transaction-local removal list into its own queue.
-    /// The backup layer reuses THIS disk's `metadata_storage` (only the local-location object
-    /// storage is swapped for `BackupObjectStorage`), so the backup killer drains THIS disk's queue
-    /// and writes deletion markers (soft delete) instead of unlinking. If the wrapped killers stayed
-    /// live they would drain THEIR queues through the RAW object storage and physically unlink the
-    /// blob — defeating the markers and the external-GC contract.
-    ///
-    /// Fix, two complementary halves:
-    ///
-    /// 1. Only the backup disk's killer may run on the queue it drains. The backup disk reuses
-    ///    THIS disk's `metadata_storage`, so the backup killer drains THIS disk's removal queue and
-    ///    writes soft-delete markers. Disable this disk's own killer (it would drain the same queue
-    ///    through the RAW object storage and physically unlink the marked blobs) and detach the
-    ///    wrapped chain from the backup killer so `triggerAndWait` won't fire it. `disable` is
-    ///    sticky across `SYSTEM RELOAD CONFIG` (see `BlobKillerThread::disable`).
-    ///
-    /// 2. Disks BELOW this one (e.g. `cache -> object_storage`) each own a SEPARATE removal queue
-    ///    filled at commit, but the backup killer only drains THIS disk's queue — nobody drains
-    ///    theirs. Disable their killers too (defense-in-depth against a physical delete) AND stop
-    ///    them recording removals at the source (`stopRecordingRemovals`): otherwise their queues
-    ///    grow unbounded for the process lifetime (memory leak / eventual OOM). Suppressing at the
-    ///    source keeps the transaction-local removal list intact, so this disk's queue is still
-    ///    populated and the backup killer still soft-deletes every removed blob.
-    ///
-    /// This relies on the wrapped stack being private to the backup wrapper, which holds for our
-    /// single-location tiered-storage deployment.
-    backup_disk->blob_killer->detachWrapped();
-    blob_killer->disable();
-    for (DiskObjectStorageConstPtr layer = wrapped_disk; layer; layer = layer->wrapped_disk)
-    {
-        layer->blob_killer->disable();
-        stopRecordingRemovals(*layer->metadata_storage);
-    }
-
-    return backup_disk;
-}
-
 DiskTransactionPtr DiskObjectStorage::createObjectStorageTransaction()
 {
     return std::make_shared<DiskObjectStorageTransaction>(cluster, metadata_storage, object_storages, blob_killer, wait_blob_removal, getReadResourceName(), getWriteResourceName());

src/Disks/DiskObjectStorage/DiskObjectStorage.h

@@ -221,11 +221,6 @@ friend class DiskObjectStorageReservation;
     /// DiskObjectStorage(CachedObjectStorage(...CacheObjectStorage(S3ObjectStorage)...))
     DiskObjectStoragePtr wrapWithCache(FileCachePtr cache, const FileCacheSettings & cache_settings, const String & layer_name) const;
 
-    /// Add a backup layer that turns object deletions into local deletion-marker files.
-    /// Like wrapWithCache, this returns a new DiskObjectStorage whose local object storage
-    /// is wrapped in a BackupObjectStorage and which links back to this disk via wrapped_disk.
-    DiskObjectStoragePtr wrapWithBackup(const String & layer_name, const String & backup_base_path) const;
-
     bool supportsLayers() const override { return true; }
 
     /// Get names of all cache layers. Name is how cache is defined in configuration file.

src/Disks/DiskObjectStorage/MetadataStorages/Cache/MetadataStorageFromCacheObjectStorage.cpp

@@ -265,9 +265,6 @@ void MetadataStorageFromCacheObjectStorageTransaction::commit(const TransactionC
 {
     underlying->commit(options);
 
-    /// See `setRecordRemovals`: skip enqueuing when a backup layer below owns deletion for this
-    /// cache. The transaction-local list still flows up via `getSubmittedForRemovalBlobs`.
-    if (metadata_storage.record_removals.load(std::memory_order_relaxed))
     {
         std::lock_guard guard(metadata_storage.removed_objects_mutex);
         metadata_storage.objects_to_remove.submitForRemoval(underlying->getSubmittedForRemovalBlobs());
@@ -278,7 +275,7 @@ TransactionCommitOutcomeVariant MetadataStorageFromCacheObjectStorageTransaction
 {
     auto result = underlying->tryCommit(options);
 
-    if (isSuccessfulOutcome(result) && metadata_storage.record_removals.load(std::memory_order_relaxed))
+    if (isSuccessfulOutcome(result))
     {
         std::lock_guard guard(metadata_storage.removed_objects_mutex);
         metadata_storage.objects_to_remove.submitForRemoval(underlying->getSubmittedForRemovalBlobs());

src/Disks/DiskObjectStorage/MetadataStorages/Cache/MetadataStorageFromCacheObjectStorage.h

@@ -72,10 +72,6 @@ class MetadataStorageFromCacheObjectStorage : public IMetadataStorage
     int64_t recordAsRemoved(const StoredObjects & blobs) override;
     bool hasPendingRemovalBlobs(const StoredObjects & blobs) const override;
 
-    /// Non-virtual (reached via `dynamic_cast` from `DiskObjectStorage::wrapWithBackup`), see that
-    /// call site and `record_removals`. Adds no slot to the `IMetadataStorage` vtable.
-    void setRecordRemovals(bool value) { record_removals.store(value, std::memory_order_relaxed); }
-
     BlobsToReplicate getBlobsToReplicate(const ClusterConfigurationPtr & cluster, int64_t max_count) override;
     int64_t recordAsReplicated(const BlobsToReplicate & blobs) override;
     bool hasUnreplicatedBlobs(const Location & location_to_check) override;
@@ -92,11 +88,6 @@ class MetadataStorageFromCacheObjectStorage : public IMetadataStorage
 
     mutable std::mutex removed_objects_mutex;
     InMemoryRemovalQueue objects_to_remove TSA_GUARDED_BY(removed_objects_mutex);
-
-    /// When false, commits skip enqueuing orphaned blobs into `objects_to_remove` (see
-    /// `setRecordRemovals`). Set by a backup layer only for a cache that sits BELOW the wrapped
-    /// disk; the directly-wrapped disk keeps recording (its queue is drained by the backup killer).
-    std::atomic_bool record_removals{true};
 };
 
 class MetadataStorageFromCacheObjectStorageTransaction : public IMetadataTransaction

src/Disks/DiskObjectStorage/MetadataStorages/Local/MetadataStorageFromDisk.cpp

@@ -185,10 +185,6 @@ void MetadataStorageFromDiskTransaction::commit(const TransactionCommitOptionsVa
 
     operations.finalize();
 
-    /// Skip enqueuing when a backup layer owns deletion for this disk (see `setRecordRemovals`):
-    /// the queue would have no drainer and grow unbounded. `objects_to_remove` (transaction-local)
-    /// is still returned by `getSubmittedForRemovalBlobs`, so the layer above still soft-deletes.
-    if (metadata_storage.record_removals.load(std::memory_order_relaxed))
     {
         std::lock_guard guard(metadata_storage.removed_objects_mutex);
         metadata_storage.objects_to_remove.submitForRemoval(objects_to_remove);

src/Disks/DiskObjectStorage/MetadataStorages/Local/MetadataStorageFromDisk.h

@@ -33,10 +33,6 @@ class MetadataStorageFromDisk final : public IMetadataStorage
     mutable std::mutex removed_objects_mutex;
     InMemoryRemovalQueue objects_to_remove TSA_GUARDED_BY(removed_objects_mutex);
 
-    /// When false, transaction commits skip enqueuing orphaned blobs into `objects_to_remove`
-    /// (see `setRecordRemovals`). Set once by a backup layer at wrap time; never re-enabled.
-    std::atomic_bool record_removals{true};
-
 public:
     MetadataStorageFromDisk(DiskPtr disk_, String compatible_key_prefix_, ObjectStorageKeyGeneratorPtr key_generator_);
 
@@ -94,11 +90,6 @@ class MetadataStorageFromDisk final : public IMetadataStorage
     BlobsToRemove getBlobsToRemove(const ClusterConfigurationPtr & cluster, int64_t max_count) override;
     int64_t recordAsRemoved(const StoredObjects & blobs) override;
     bool hasPendingRemovalBlobs(const StoredObjects & blobs) const override;
-
-    /// Stop enqueuing orphaned blobs into `objects_to_remove` at commit time. Non-virtual on purpose
-    /// (reached via `dynamic_cast` from `DiskObjectStorage::wrapWithBackup`) so it adds no slot to
-    /// the `IMetadataStorage` vtable. See that call site and `record_removals` for the rationale.
-    void setRecordRemovals(bool value) { record_removals.store(value, std::memory_order_relaxed); }
 };
 
 class MetadataStorageFromDiskTransaction final : public IMetadataTransaction