Provide better partitions hint for merge selecting task by tavplubix · Pull Request #49637 · ClickHouse/ClickHouse

tavplubix · 2023-05-07T22:23:02Z

Changelog category (leave one):

Improvement

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

Reduced number of List ZooKeeper requests when selecting parts to merge and a lot of partitions do not have anything to merge

Also fixes #49201

robot-clickhouse-ci-2 · 2023-05-07T22:24:11Z

This is an automated comment for commit c2b1e8c with description of existing statuses. It's updated for the latest CI running
The full report is available here
The overall status of the commit is 🔴 failure

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	🟡 pending
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 image for servers	The check to build and optionally push the mentioned image to docker hub	🟢 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
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	🟢 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
Push to Dockerhub	The check for building and pushing the CI related docker images to docker hub	🟢 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	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

tavplubix · 2023-05-10T15:42:22Z

Integration tests (asan) [4/6] - #45404
Stateless tests (release, DatabaseReplicated) [3/4] - 02439_merge_selecting_partitions - something weird, will check later
Stateless tests (release, analyzer) - 02458_relax_too_many_parts is broken in master

tavplubix · 2023-05-17T11:52:37Z

Stateless tests (release, analyzer) - #46835 (comment)
Unit tests (asan) - IOResourceDynamicResourceManager was broken in master

antonio2368 · 2023-05-22T08:13:19Z

src/Storages/MergeTree/MergeTreeDataMergerMutator.cpp

+    auto new_end_it = std::remove_if(res.begin(), res.end(), [partitions_hint](const auto & part)
+    {
+        return !partitions_hint->contains(part->info.partition_id);
+    });
+    res.erase(new_end_it, res.end());


antonio2368 · 2023-05-22T08:18:23Z

src/Storages/MergeTree/ReplicatedMergeTreeQueue.h


+/// Lightweight version of ReplicatedMergeTreeMergePredicate that do not make any ZooKeeper requests,
+/// but may return false-positive results. Checks only a subset of required conditions.
+class TrivialMergePredicate


LocalMergePredicate maybe?

antonio2368 · 2023-05-22T08:24:23Z

tests/queries/0_stateless/02439_merge_selecting_partitions.sql

@@ -0,0 +1,24 @@
+


drop table if exists rmt

I don't think these drop queries are really useful since we create a new database for each test, but okay

tavplubix · 2023-05-23T09:40:21Z

Integration tests (release) [2/4] - #48691

boldakovds · 2023-11-01T06:45:35Z

@tavplubix Hi! Can you backport this to 23.3 ? It will be really helpful for us.

Algunenano · 2023-11-02T12:22:16Z

Hi! Can you backport this to 23.3 ? It will be really helpful for us.

Improvements, and specially ones with critical changes like this, aren't backported. That's the whole point of stable releases. Consider upgrading.

tavplubix added 2 commits May 7, 2023 22:06

split selectPartsToMerge into smaller functions

29bbde8

provide better partitions hint for merge selecting task

9148924

robot-clickhouse-ci-2 added the pr-improvement Pull request with some product improvements label May 7, 2023

better partitions hint in mutations finalization

aa829c4

tavplubix force-pushed the less_zookeeper_requests branch from 3bc16e2 to aa829c4 Compare May 8, 2023 22:23

fix another issue, add a test

6f53ee2

tavplubix marked this pull request as ready for review May 9, 2023 21:30

tavplubix added 2 commits May 16, 2023 18:24

Merge branch 'master' into less_zookeeper_requests

242a3fc

fix test

2648818

antonio2368 self-assigned this May 19, 2023

antonio2368 reviewed May 22, 2023

View reviewed changes

tavplubix added 3 commits May 22, 2023 13:59

Merge branch 'master' into less_zookeeper_requests

487e510

apply review suggestions

821b64b

fix

c2b1e8c

tavplubix force-pushed the less_zookeeper_requests branch from 087dfba to c2b1e8c Compare May 22, 2023 17:30

antonio2368 approved these changes May 23, 2023

View reviewed changes

tavplubix merged commit 141a72d into master May 23, 2023

tavplubix deleted the less_zookeeper_requests branch May 23, 2023 09:40

This was referenced Jun 12, 2023

High amount of ZooKeeper requests/CPU Load after upgrade to 22.11.1 from 22.3 #43647

Closed

Some fixes for ReplicatedMergeTree #42878

Merged

LuPan92 mentioned this pull request Jul 22, 2023

Zookeeper time wait metric is too high. #52447

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Provide better partitions hint for merge selecting task#49637

Provide better partitions hint for merge selecting task#49637
tavplubix merged 9 commits intomasterfrom
less_zookeeper_requests

tavplubix commented May 7, 2023 •

edited

Loading

Uh oh!

robot-clickhouse-ci-2 commented May 7, 2023 •

edited by robot-ch-test-poll1

Loading

Uh oh!

tavplubix commented May 10, 2023

Uh oh!

tavplubix commented May 17, 2023

Uh oh!

antonio2368 May 22, 2023

Uh oh!

antonio2368 May 22, 2023

Uh oh!

antonio2368 May 22, 2023

Uh oh!

tavplubix May 22, 2023

Uh oh!

tavplubix commented May 23, 2023

Uh oh!

boldakovds commented Nov 1, 2023

Uh oh!

Algunenano commented Nov 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

tavplubix commented May 7, 2023 • edited 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):

Uh oh!

robot-clickhouse-ci-2 commented May 7, 2023 • edited by robot-ch-test-poll1 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tavplubix commented May 10, 2023

Uh oh!

tavplubix commented May 17, 2023

Uh oh!

antonio2368 May 22, 2023

Choose a reason for hiding this comment

Uh oh!

antonio2368 May 22, 2023

Choose a reason for hiding this comment

Uh oh!

antonio2368 May 22, 2023

Choose a reason for hiding this comment

Uh oh!

tavplubix May 22, 2023

Choose a reason for hiding this comment

Uh oh!

tavplubix commented May 23, 2023

Uh oh!

boldakovds commented Nov 1, 2023

Uh oh!

Algunenano commented Nov 2, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tavplubix commented May 7, 2023 •

edited

Loading

robot-clickhouse-ci-2 commented May 7, 2023 •

edited by robot-ch-test-poll1

Loading