Small improvements for tables on write-once (s3_plain)/read-only (web) disks #59170

azat · 2024-01-24T16:25:25Z

Changelog category (leave one):

Improvement

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

Prohibit mutable operations (INSERT/ALTER/OPTIMIZE/...) on read-only/write-once storages with a proper TABLE_IS_READ_ONLY error (to avoid leftovers). Avoid leaving left-overs on write-once disks (format_version.txt) on CREATE/ATTACH. Ignore DROP for ReplicatedMergeTree (so as for MergeTree). Fix iterating over s3_plain (MetadataStorageFromPlainObjectStorage::iterateDirectory). Note read-only is web disk, and write-once is s3_plain.

robot-ch-test-poll2 · 2024-01-24T16:27:16Z

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

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

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
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	✅ 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 server and keeper images	There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS	✅ success
Docs check	There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS	✅ success
Fast tests	There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS	✅ 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
Integration tests	The integration tests report. In parenthesis the package type is given, and in square brackets are the optional part/total tests	✅ success
Mergeable Check	Checks if all other necessary checks are successful	✅ success
SQLancer	Fuzzing tests that detect logical bugs with SQLancer tool	✅ success
Sqllogic	Run clickhouse on the sqllogic test set against sqlite and checks that all statements are passed	✅ 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
Stress test	Runs stateless functional tests concurrently from several clients to detect concurrency-related errors	✅ success
Style check	There's no description for the check yet, please add it to tests/ci/ci_config.py:CHECK_DESCRIPTIONS	✅ 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

Check name	Description	Status
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	❌ failure

azat · 2024-01-26T13:43:15Z

Fast tests — fail: 5, passed: 4969, skipped: 1074

Fix fasttest by pinning pip dependencies #59256

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

…engine Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

…actions Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

…orage Mutable operations includes: - INSERT - OPTIMIZE - parts manipulation - ... But does not include DROP, since you need an ability to DROP it. Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

…ectory (for DROP TABLE) This is required to remove format_version.txt for MergeTree after DROP (lldb) bt * thread 192, name = 'BgSchPool', stop reason = breakpoint 1.1 frame 0: 0x0000000016c7f452 clickhouse`__cxa_throw(thrown_object=0x00007ffd7a53c480, tinfo=0x00000000011602a0, dest=(clickhouse`DB::Exception::~Exception() at Exception.h:40)) + 18 at cxa_exception.cpp:258 frame 1: 0x00000000109bf709 clickhouse`DB::IMetadataTransaction::throwNotImplemented() + 137 at IMetadataStorage.h:151 frame 2: 0x00000000109bf126 clickhouse`DB::IMetadataTransaction::removeDirectory(this=<unavailable>, (null)=<unavailable>) + 6 at IMetadataStorage.h:96 frame 3: 0x0000000010968978 clickhouse`DB::(anonymous namespace)::RemoveRecursiveObjectStorageOperation::removeMetadataRecursive(this=0x00007ffd7a7327a0, tx=std::__1::shared_ptr<DB::IMetadataTransaction>::element_type @ 0x00007ffd7a6f6018 strong=4 weak=1, path_to_remove="tmp_insert_all_2_2_0") + 376 at DiskObjectStorageTransaction.cpp:364 frame 4: 0x000000001096893d clickhouse`DB::(anonymous namespace)::RemoveRecursiveObjectStorageOperation::removeMetadataRecursive(this=0x00007ffd7a7327a0, tx=std::__1::shared_ptr<DB::IMetadataTransaction>::element_type @ 0x00007ffd7a6f6018 strong=4 weak=1, path_to_remove="store/27f/27f6f9d2-ab2e-4817-b771-0e706e485b15/") + 317 at DiskObjectStorageTransaction.cpp:362 frame 5: 0x0000000010965a7f clickhouse`DB::(anonymous namespace)::RemoveRecursiveObjectStorageOperation::execute(this=0x00007ffd7a7327a0, tx=std::__1::shared_ptr<DB::IMetadataTransaction>::element_type @ 0x00007ffd7a6f6018 strong=4 weak=1) + 95 at DiskObjectStorageTransaction.cpp:372 frame 6: 0x0000000010963a2f clickhouse`DB::DiskObjectStorageTransaction::commit(this=0x00007ffea22b6168) + 79 at DiskObjectStorageTransaction.cpp:903 frame 7: 0x00000000109577e5 clickhouse`DB::DiskObjectStorage::removeSharedRecursive(this=<unavailable>, path="store/27f/27f6f9d2-ab2e-4817-b771-0e706e485b15/", keep_all_batch_data=false, file_names_remove_metadata_only=size=0) + 133 at DiskObjectStorage.cpp:426 frame 8: 0x000000001095af79 clickhouse`DB::DiskObjectStorage::removeRecursive(this=<unavailable>, path=<unavailable>) + 57 at DiskObjectStorage.h:79 * frame 9: 0x0000000010b5c3d5 clickhouse`DB::DatabaseCatalog::dropTableFinally(this=0x00007ffff7096800, table=0x00007ffe853e1bd0) + 1365 at DatabaseCatalog.cpp:1317 frame 10: 0x0000000010b5b7e7 clickhouse`DB::DatabaseCatalog::dropTableDataTask(this=0x00007ffff7096800) + 2375 at DatabaseCatalog.cpp:1270 Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

…efix) Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

It is actually OK to DROP at least from read-only disk, as for the write-once it should be OK as well, since CREATE is prohibited anyway, so there should be no leftovers CI: https://s3.amazonaws.com/clickhouse-test-reports/59170/c82050d1925439f0ede2b32acb5b1b8df4acae5d/stress_test__asan_.html CI: https://s3.amazonaws.com/clickhouse-test-reports/59170/c82050d1925439f0ede2b32acb5b1b8df4acae5d/integration_tests__asan__[3_4].html This reverts commit 082e627. Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

… storage Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

…tead)" CREATE is used by Replicated database even for ATTACH, and anyway ATTACH creates format_version.txt as well (without previuos patch). This reverts commit 323f1f320ebe7e588d443abf78efa2c60193e7a9. Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

azat · 2024-01-26T15:48:38Z

@kssenii the logic had been changed slightly (to make it work in all known cases), so it is better to give another look, and not simply re-approve:)

azat · 2024-01-27T14:01:53Z

Performance Comparison [1/4] — Errors while building the report.

Fix perf tests (after sumMap starts to filter out -0.) #59287

Upgrade check (debug) — New settings are not reflected in settings changes history (see new_settings.txt)

Add missing setting optimize_injective_functions_in_group_by to SettingsChangesHistory #59283

Stateless tests flaky check (asan) — fail: 1, passed: 201

71cd1ef (#59170)

In case of commit failed (likely due to fault injection in tests) part name will have different name, not all_0_0_0, so replace numbers with X. Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

robot-ch-test-poll2 added the pr-improvement Pull request with some product improvements label Jan 24, 2024

azat force-pushed the disks/s3-plain-improvements branch from 463c0f2 to 2165d45 Compare January 24, 2024 16:48

kssenii self-assigned this Jan 24, 2024

azat mentioned this pull request Jan 24, 2024

Add ability to skip read-only replicas for INSERT into Distributed engine #59176

Merged

azat force-pushed the disks/s3-plain-improvements branch 2 times, most recently from 73e096d to 091021e Compare January 25, 2024 14:48

azat marked this pull request as draft January 25, 2024 16:59

azat force-pushed the disks/s3-plain-improvements branch from 091021e to c82050d Compare January 25, 2024 18:39

azat marked this pull request as ready for review January 25, 2024 18:42

kssenii approved these changes Jan 26, 2024

View reviewed changes

azat marked this pull request as draft January 26, 2024 14:02

azat added 15 commits January 26, 2024 16:47

tests: add s3_plain disk

e56728a

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

Do not drop parts for static (ro/write-once) storages for Replicated …

6751e01

…engine Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

Add a note for MetadataStorageFromPlainObjectStorage to support trans…

41ebaef

…actions Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

Disable mutable operations into {Replicated,}MergeTree with static st…

f3b4f28

…orage Mutable operations includes: - INSERT - OPTIMIZE - parts manipulation - ... But does not include DROP, since you need an ability to DROP it. Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

Fix MetadataStorageFromPlainObjectStorage::iterateDirectory() (add pr…

d79c1f7

…efix) Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

tests: cover DROP TABLE on s3_plain disk

046a7a5

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

Prohibit DROP from static storages

1ee723d

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

Prohibit CREATE on static storages (ATTACH should be used instead)

aafc149

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

tests: rewrite test for MergeTree engines in s3_plain using ATTACH

d99fb4c

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

tests/test_disk_over_web_server: update expected assertion for static…

7cc7fb8

… storage Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

Do not create format_version.txt on write-once storages

7ba31bf

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

tests: adjust 02980_s3_plain_DROP_TABLE tests

332924a

Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

azat force-pushed the disks/s3-plain-improvements branch from a715cd2 to 332924a Compare January 26, 2024 15:47

azat marked this pull request as ready for review January 26, 2024 15:47

tests: fix 02980_s3_plain_DROP_TABLE_ReplicatedMergeTree flakiness

71cd1ef

In case of commit failed (likely due to fault injection in tests) part name will have different name, not all_0_0_0, so replace numbers with X. Signed-off-by: Azat Khuzhin <a.khuzhin@semrush.com>

kssenii approved these changes Jan 29, 2024

View reviewed changes

kssenii merged commit a29ab8f into ClickHouse:master Jan 29, 2024
249 of 253 checks passed

azat deleted the disks/s3-plain-improvements branch January 29, 2024 09:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Small improvements for tables on write-once (s3_plain)/read-only (web) disks #59170

Small improvements for tables on write-once (s3_plain)/read-only (web) disks #59170

azat commented Jan 24, 2024 •

edited

robot-ch-test-poll2 commented Jan 24, 2024 •

edited by robot-ch-test-poll1

azat commented Jan 26, 2024

azat commented Jan 26, 2024 •

edited

azat commented Jan 27, 2024 •

edited

Small improvements for tables on write-once (s3_plain)/read-only (web) disks #59170

Small improvements for tables on write-once (s3_plain)/read-only (web) disks #59170

Conversation

azat commented Jan 24, 2024 • edited

Changelog category (leave one):

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

robot-ch-test-poll2 commented Jan 24, 2024 • edited by robot-ch-test-poll1

azat commented Jan 26, 2024

azat commented Jan 26, 2024 • edited

azat commented Jan 27, 2024 • edited

azat commented Jan 24, 2024 •

edited

robot-ch-test-poll2 commented Jan 24, 2024 •

edited by robot-ch-test-poll1

azat commented Jan 26, 2024 •

edited

azat commented Jan 27, 2024 •

edited