Merge pull request #72858 from ClickHouse/fix-backup-resotre-of-empty-file-with-plain-rewr

Fix backup/restore with plain rewritable disk in case there are empty files in backup

src/Disks/ObjectStorages/DiskObjectStorageTransaction.cpp

@@ -911,8 +911,11 @@ void DiskObjectStorageTransaction::chmod(const String & path, mode_t mode)
 void DiskObjectStorageTransaction::createFile(const std::string & path)
 {
     operations_to_execute.emplace_back(
-        std::make_unique<PureMetadataObjectStorageOperation>(object_storage, metadata_storage, [path](MetadataTransactionPtr tx)
+        std::make_unique<PureMetadataObjectStorageOperation>(object_storage, metadata_storage, [path, this](MetadataTransactionPtr tx)
         {
+            if (object_storage.isPlain() && !object_storage.isWriteOnce())
+                tx->createEmptyFile(path);
+
             tx->createEmptyMetadataFile(path);
         }));
 }

src/Disks/ObjectStorages/IMetadataStorage.h

@@ -135,6 +135,8 @@ public:
     /// Create empty file in metadata storage
     virtual void createEmptyMetadataFile(const std::string & path) = 0;
 
+    virtual void createEmptyFile(const std::string & /* path */) {}
+
     /// Create metadata file on paths with content (blob_name, size_in_bytes)
     virtual void createMetadataFile(const std::string & path, ObjectStorageKey key, uint64_t size_in_bytes) = 0;
 

src/Disks/ObjectStorages/MetadataStorageFromPlainObjectStorage.h

@@ -98,9 +98,9 @@ protected:
     ObjectMetadataEntryPtr getObjectMetadataEntryWithCache(const std::string & path) const;
 };
 
-class MetadataStorageFromPlainObjectStorageTransaction final : public IMetadataTransaction, private MetadataOperationsHolder
+class MetadataStorageFromPlainObjectStorageTransaction : public IMetadataTransaction, private MetadataOperationsHolder
 {
-private:
+protected:
     MetadataStorageFromPlainObjectStorage & metadata_storage;
     ObjectStoragePtr object_storage;
 

src/Disks/ObjectStorages/MetadataStorageFromPlainRewritableObjectStorage.h

@@ -4,11 +4,31 @@
 
 #include <memory>
 #include <unordered_set>
+#include <IO/WriteHelpers.h>
 
 
 namespace DB
 {
 
+class MetadataStorageFromPlainRewritableObjectStorageTransaction final : public MetadataStorageFromPlainObjectStorageTransaction
+{
+public:
+    explicit MetadataStorageFromPlainRewritableObjectStorageTransaction(
+        MetadataStorageFromPlainObjectStorage & metadata_storage_, ObjectStoragePtr object_storage_)
+        : DB::MetadataStorageFromPlainObjectStorageTransaction(metadata_storage_, object_storage_)
+    {
+    }
+
+    void createEmptyFile(const std::string & path) override
+    {
+        const auto key = object_storage->generateObjectKeyForPath(path, std::nullopt);
+        StoredObject object(key.serialize(), "", /* file_size */0);
+        auto buf = object_storage->writeObject(object, WriteMode::Rewrite);
+        buf->finalize();
+    }
+};
+
+
 class MetadataStorageFromPlainRewritableObjectStorage final : public MetadataStorageFromPlainObjectStorage
 {
 private:
@@ -22,6 +42,11 @@ public:
 
     MetadataStorageType getType() const override { return MetadataStorageType::PlainRewritable; }
 
+    MetadataTransactionPtr createTransaction() override
+    {
+        return std::make_shared<MetadataStorageFromPlainRewritableObjectStorageTransaction>(*this, object_storage);
+    }
+
     bool existsFile(const std::string & path) const override;
 
     bool existsDirectory(const std::string & path) const override;

tests/integration/test_backup_restore_s3/configs/query_log.xml

@@ -0,0 +1,8 @@
+<?xml version="1.0"?>
+<clickhouse>
+    <query_log replace="replace">
+        <database>system</database>
+        <table>query_log</table>
+        <engine>Engine = MergeTree PARTITION BY event_date ORDER BY event_time TTL event_date + INTERVAL 30 day SETTINGS storage_policy='policy_s3_plain_rewritable', ttl_only_drop_parts=1, min_rows_for_wide_part=0, min_bytes_for_wide_part=0</engine>
+    </query_log>
+</clickhouse>

tests/integration/test_backup_restore_s3/test.py

@@ -18,6 +18,7 @@ node = cluster.add_instance(
         "configs/s3_settings.xml",
         "configs/blob_log.xml",
         "configs/remote_servers.xml",
+        "configs/query_log.xml",
     ],
     user_configs=[
         "configs/zookeeper_retries.xml",
@@ -776,3 +777,28 @@ def test_backup_to_s3_different_credentials():
 
     check_backup_restore(False)
     check_backup_restore(True)
+
+
+def test_backup_restore_system_tables_with_plain_rewritable_disk():
+    instance = cluster.instances["node"]
+    backup_name = new_backup_name()
+    backup_destination = (
+        f"S3('http://minio1:9001/root/data/backups/{backup_name}', 'minio', 'minio123')"
+    )
+
+    instance.query("SYSTEM FLUSH LOGS")
+
+    backup_query_id = uuid.uuid4().hex
+    instance.query(
+        f"BACKUP TABLE system.query_log TO {backup_destination}",
+        query_id=backup_query_id,
+    )
+    restore_query_id = uuid.uuid4().hex
+    instance.query("DROP TABLE IF EXISTS data_restored SYNC")
+    instance.query(
+        f"""
+        RESTORE TABLE system.query_log AS data_restored FROM {backup_destination};
+        """,
+        query_id=restore_query_id,
+    )
+    instance.query("DROP TABLE data_restored SYNC")