Backup disk by AlexNsf · Pull Request #64222 · ClickHouse/ClickHouse

AlexNsf · 2024-05-22T13:45:28Z

Changelog category (leave one):

New Feature

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Add backup disk, that reads metadata from ".backup" file to add some optimizations for working with backup. Closes #42154.

Documentation entry for user-facing changes

Documentation is written (mandatory for new features)

Information about CI checks: https://clickhouse.com/docs/en/development/continuous-integration/

Modify your CI run

NOTE: If your merge the PR with modified CI you MUST KNOW what you are doing
NOTE: Checked options will be applied if set before CI RunConfig/PrepareRunConfig step

Include tests (required builds will be added automatically):

Exclude tests:

Extra options:

do not test (only style check)
disable merge-commit (no merge from master before tests)
disable CI cache (job reuse)

Only specified batches in multi-batch jobs:

1
2
3
4

CLAassistant · 2024-05-22T13:45:34Z

All committers have signed the CLA.

robot-ch-test-poll4 · 2024-05-22T13:59:58Z

This is an automated comment for commit c209657 with description of existing statuses. It's updated for the latest CI running

❌ Click here to open a full report in a separate page

Check name	Description	Status
A Sync	There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS	⏳ pending
CI running	A meta-check that indicates the running CI. Normally, it's in success or pending state. The failed status indicates some problems with the PR	⏳ pending
Integration tests	The integration tests report. In parenthesis the package type is given, and in square brackets are the optional part/total tests	❌ failure
Mergeable Check	Checks if all other necessary checks are successful	❌ failure
Stress test	Runs stateless functional tests concurrently from several clients to detect concurrency-related errors	❌ failure

Successful checks

Check name	Description	Status
AST fuzzer	Runs randomly generated queries to catch program errors. The build type is optionally given in parenthesis. If it fails, ask a maintainer for help	✅ success
ClickBench	Runs [ClickBench](https://github.com/ClickHouse/ClickBench/) with instant-attach table	✅ success
ClickHouse build check	Builds ClickHouse in various configurations for use in further steps. You have to fix the builds that fail. Build logs often has enough information to fix the error, but you might have to reproduce the failure locally. The cmake options can be found in the build log, grepping for cmake. Use these options and follow the general build process	✅ success
Compatibility check	Checks that clickhouse binary runs on distributions with old libc versions. If it fails, ask a maintainer for help	✅ success
Docker keeper image	The check to build and optionally push the mentioned image to docker hub	✅ success
Docker server image	The check to build and optionally push the mentioned image to docker hub	✅ success
Docs check	Builds and tests the documentation	✅ success
Fast test	Normally this is the first check that is ran for a PR. It builds ClickHouse and runs most of stateless functional tests, omitting some. If it fails, further checks are not started until it is fixed. Look at the report to see which tests fail, then reproduce the failure locally as described here	✅ success
Flaky tests	Checks if new added or modified tests are flaky by running them repeatedly, in parallel, with more randomization. Functional tests are run 100 times with address sanitizer, and additional randomization of thread scheduling. Integrational tests are run up to 10 times. If at least once a new test has failed, or was too long, this check will be red. We don't allow flaky tests, read the doc	✅ success
Install packages	Checks that the built packages are installable in a clear environment	✅ success
PR Check	There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS	✅ success
Performance Comparison	Measure changes in query performance. The performance test report is described in detail here. In square brackets are the optional part/total tests	✅ success
Stateful tests	Runs stateful functional tests for ClickHouse binaries built in various configurations -- release, debug, with sanitizers, etc	✅ success
Stateless tests	Runs stateless functional tests for ClickHouse binaries built in various configurations -- release, debug, with sanitizers, etc	✅ success
Style check	Runs a set of checks to keep the code style clean. If some of tests failed, see the related log from the report	✅ success
Unit tests	Runs the unit tests for different release types	✅ success
Upgrade check	Runs stress tests on server version from last release and then tries to upgrade it to the version from the PR. It checks if the new server can successfully startup without any errors, crashes or sanitizer asserts	✅ success

kssenii · 2024-06-11T10:31:39Z

src/Disks/DiskBackup.cpp

+namespace DB
+{
+
+DiskBackup::DiskBackup(const String & name_, DiskPtr delegate_, MetadataStoragePtr metadata_) : IDisk(name_), delegate(delegate_), metadata(metadata_)


Could you please add some documentation to https://github.com/ClickHouse/ClickHouse/blob/master/docs/en/operations/storing-data.md about this disk? What its usecase, how to configure it and maybe some small usage example.

kssenii · 2024-06-11T10:34:58Z

src/Disks/DiskBackup.h

+}
+
+public:
+    DiskBackup(const String & name_, DiskPtr delegate_, MetadataStoragePtr metadata_);


As I see this constructor is used only in unit test, because it was not clear how to create a config object from there? If so, take a look here

ClickHouse/src/Common/tests/gtest_named_collections.cpp

Lines 40 to 58 in e493856

std::string xml(R"CONFIG(<clickhouse>

<named_collections>

<collection1>

<key1>value1</key1>

<key2>2</key2>

<key3>3.3</key3>

<key4>-4</key4>

</collection1>

<collection2>

<key4>value4</key4>

<key5>5</key5>

<key6>6.6</key6>

</collection2>

</named_collections>

</clickhouse>)CONFIG");

Poco::XML::DOMParser dom_parser;

Poco::AutoPtr<Poco::XML::Document> document = dom_parser.parseString(xml);

Poco::AutoPtr<Poco::Util::XMLConfiguration> config = new Poco::Util::XMLConfiguration(document);

may be you can do the same, so this constructor could be removed

kssenii · 2024-06-11T10:36:15Z

src/Disks/DiskBackup.h

+    extern const int NOT_IMPLEMENTED;
+}
+
+class DiskBackup : public IDisk


It will be good to also add a comment before this class definition with a small explanation of the purpose of this class. A sentence about its purpose from #42154 will suffice.

kssenii · 2024-06-11T10:41:43Z

src/Disks/tests/gtest_disk_backup.cpp

+    delegate->createFile("test_folder/test.txt");
+
+    EXPECT_EQ(backup->isFile("test_folder/test.txt"), true);
+    EXPECT_EQ(backup->isDirectory("test_folder"), true);


Need to also check that write operations are not allowed, you can use ASSERT_THROW for this

kssenii · 2024-06-11T10:43:23Z

src/Disks/DiskBackup.cpp

+
+DiskBackup::DiskBackup(const String & name_, const Poco::Util::AbstractConfiguration & config_, const String & config_prefix_, const DisksMap & map_) : IDisk(name_)
+{
+    String disk_delegate_name = config_.getString(config_prefix_ + ".disk_delegate");


May be we can rename disk_delegate to just disk, because in some similar cases we use just "disk", for example for disk type "cache".

kssenii · 2024-06-11T10:45:25Z

src/Disks/tests/gtest_disk_backup.cpp

@@ -0,0 +1,105 @@
+#include <filesystem>


Let's also add a stateless test with disk created from SQL, see example here

ClickHouse/tests/queries/0_stateless/02808_custom_disk_with_user_defined_name.sh

Line 15 in e493856

SETTINGS disk = disk(name = 's3_disk', type = cache, max_size = '100Ki', path = ${CLICKHOUSE_TEST_UNIQUE_NAME}, disk = s3_disk);

kssenii · 2024-06-11T10:47:45Z

src/Disks/tests/gtest_disk_backup.cpp

@@ -0,0 +1,105 @@
+#include <filesystem>
+#include <fstream>
+#include <memory>


Also need an integration test which creates the disk with type backup and checks that we can read from it and that an attempt to write to it fails. Also check that server can be restarted (via node.restart_clickhouse()). Also check the same for customly created disk from SQL (mentioned in the comment above).

kssenii · 2024-06-11T10:49:38Z

src/Disks/MetadataStorageFromBackupFile.h

+namespace DB
+{
+
+class MetadataStorageFromBackupFile final : public IMetadataStorage


A small comment in code about this class here will also be good to have - its purpose and how it works - shortly.

kssenii · 2024-06-11T10:51:16Z

src/Disks/MetadataStorageFromBackupFile.cpp

+        return std::make_unique<StaticDirectoryIterator>(std::move(dir_file_paths));
+    }
+
+    for (const auto& listed_path: nodes.at(path).children)


Suggested change

for (const auto& listed_path: nodes.at(path).children)

for (const auto & listed_path: nodes.at(path).children)

kssenii · 2024-06-11T10:54:02Z

src/Disks/MetadataStorageFromBackupFile.h

+    std::unordered_map<String /* path */, MetadataStorageFromBackupFilePseudoFileSystemNode> nodes;
+
+public:
+    explicit MetadataStorageFromBackupFile(const String & path_to_backup_file);


I do not see an object of this class being created anywhere, or I missed it somewhere?

clickhouse-gh · 2024-08-20T13:10:37Z

Dear @AlexNsf, this PR hasn't been updated for a while. Will you continue working on it? If not, please close it. Otherwise, ignore this message.

clickhouse-gh · 2024-10-22T13:12:48Z

Dear @AlexNsf, this PR hasn't been updated for a while. Will you continue working on it? If not, please close it. Otherwise, ignore this message.

clickhouse-gh · 2024-12-24T13:10:49Z

Dear @AlexNsf, this PR hasn't been updated for a while. Will you continue working on it? If not, please close it. Otherwise, ignore this message.

clickhouse-gh · 2025-02-25T13:12:34Z

Dear @AlexNsf, this PR hasn't been updated for a while. Will you continue working on it? If not, please close it. Otherwise, ignore this message.

azat · 2025-03-18T12:50:04Z

@AlexNsf thanks for your contribution!

This has been superseded by #75725, see #75725 (comment) this comment on "Database vs Disk" pros and cons.

Backup disks init

3d9e5cc

alexey-milovidov added the can be tested Allows running workflows for external contributors label May 22, 2024

robot-ch-test-poll4 added the pr-feature Pull request with new product feature label May 22, 2024

Remove unused imports

c209657

kssenii self-assigned this Jun 11, 2024

kssenii reviewed Jun 11, 2024

View reviewed changes

alexey-milovidov mentioned this pull request Dec 31, 2024

Roadmap 2025 #74046

Closed

76 tasks

nikitamikhaylov mentioned this pull request Feb 7, 2025

Add DatabaseBackup #75725

Merged

azat closed this Mar 18, 2025

	std::string xml(R"CONFIG(<clickhouse>
	<named_collections>
	<collection1>
	<key1>value1</key1>
	<key2>2</key2>
	<key3>3.3</key3>
	<key4>-4</key4>
	</collection1>
	<collection2>
	<key4>value4</key4>
	<key5>5</key5>
	<key6>6.6</key6>
	</collection2>
	</named_collections>
	</clickhouse>)CONFIG");

	Poco::XML::DOMParser dom_parser;
	Poco::AutoPtr<Poco::XML::Document> document = dom_parser.parseString(xml);
	Poco::AutoPtr<Poco::Util::XMLConfiguration> config = new Poco::Util::XMLConfiguration(document);

	for (const auto& listed_path: nodes.at(path).children)
	for (const auto & listed_path: nodes.at(path).children)

Conversation

AlexNsf commented May 22, 2024 • edited by kssenii Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelog category (leave one):

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Documentation entry for user-facing changes

Include tests (required builds will be added automatically):

Exclude tests:

Extra options:

Only specified batches in multi-batch jobs:

Uh oh!

CLAassistant commented May 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robot-ch-test-poll4 commented May 22, 2024 • edited by robot-ch-test-poll Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clickhouse-gh bot commented Aug 20, 2024

Uh oh!

clickhouse-gh bot commented Oct 22, 2024

Uh oh!

clickhouse-gh bot commented Dec 24, 2024

Uh oh!

clickhouse-gh bot commented Feb 25, 2025

Uh oh!

azat commented Mar 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

AlexNsf commented May 22, 2024 •

edited by kssenii

Loading

CLAassistant commented May 22, 2024 •

edited

Loading

robot-ch-test-poll4 commented May 22, 2024 •

edited by robot-ch-test-poll

Loading