fs cache: small optimization by kssenii · Pull Request #57615 · ClickHouse/ClickHouse

kssenii · 2023-12-07T12:21:54Z

Changelog category (leave one):

Not for changelog (changelog entry is not required)

robot-ch-test-poll · 2023-12-07T12:22:53Z

This is an automated comment for commit 235980d 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
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

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

…to filecache-small-optimization

…imization

src/Interpreters/Cache/Metadata.cpp

kssenii · 2024-01-25T11:51:10Z

src/Interpreters/Cache/FileSegment.cpp

        const auto & entry = it->getEntry();
-        UNUSED(entry);
-        chassert(entry->size == reserved_size);
+        if (download_state != State::DOWNLOADING && entry->size != reserved_size)


This additional check for download_state fixes #55867.

just for my information, why it cannot be e.g. PARTIALLY_DOWNLOADED_NO_CONTINUATION?

In tryReserve we first update entry->size and right after reserved_size, which is done under cache lock (which is priority queue lock). But here in assertCacheCorrectness this lock is not taken, so it can see entry->size != reserved_size. As tryReserve is executing while file segment has DOWNLOADING state, therefore I added this check. PARTIALLY_DOWNLOADED_NO_CONTINUATION does not apply to the described case.

kssenii · 2024-01-25T11:53:27Z

Upgrade check (asan)
Upgrade check (debug)

#58319

src/Interpreters/Cache/FileSegment.cpp

nickitat · 2024-01-25T12:25:48Z

src/Interpreters/Cache/FileSegment.cpp

        const auto & entry = it->getEntry();
-        UNUSED(entry);
-        chassert(entry->size == reserved_size);
+        if (download_state != State::DOWNLOADING && entry->size != reserved_size)


just for my information, why it cannot be e.g. PARTIALLY_DOWNLOADED_NO_CONTINUATION?

src/Interpreters/Cache/Metadata.cpp

robot-ch-test-poll added the pr-not-for-changelog This PR should not be mentioned in the changelog label Dec 7, 2023

File cache small optimization improvement

3fd9f85

kssenii force-pushed the filecache-small-optimization branch from 2801c0f to 3fd9f85 Compare December 7, 2023 12:27

kssenii added 2 commits December 8, 2023 11:00

Fxi

629c27d

Try fxi

f80f61c

kssenii force-pushed the filecache-small-optimization branch from 060c9a1 to f80f61c Compare December 8, 2023 12:01

Debug info

002b406

kssenii marked this pull request as draft December 8, 2023 14:35

kssenii and others added 21 commits December 11, 2023 13:44

Fxi

5c5c462

Increase log level to debug

bef2736

Fxi

2d5a5c6

Safer

f2da31f

Merge branch 'master' into filecache-small-optimization

1a1fa0c

Fxi

95a0978

Merge remote-tracking branch 'origin/filecache-small-optimization' in…

618c483

…to filecache-small-optimization

Try fxi

67d3f84

Merge remote-tracking branch 'origin' into filecache-small-optimization

08920b1

Log exception

d5b138e

Fxi

c14a0e8

Merge remote-tracking branch 'origin' into filecache-small-optimization

caf267c

One more fi

18e31df

Merge branch 'master' into filecache-small-optimization

f2a31db

Merge branch 'master' into filecache-small-optimization

5be8644

Merge branch 'master' into filecache-small-optimization

b6094ac

Update install.sh

e0bf32d

Fix build

f3a1a2f

Merge remote-tracking branch 'origin/master' into filecache-small-opt…

ed607fb

…imization

Fix build

0d7710e

Update FileCache.cpp

235980d

kssenii marked this pull request as ready for review January 25, 2024 11:47

kssenii commented Jan 25, 2024

View reviewed changes

src/Interpreters/Cache/Metadata.cpp Show resolved Hide resolved

kssenii requested a review from nickitat January 25, 2024 11:49

kssenii commented Jan 25, 2024

View reviewed changes

nickitat reviewed Jan 25, 2024

View reviewed changes

nickitat self-assigned this Jan 25, 2024

nickitat approved these changes Jan 25, 2024

View reviewed changes

kssenii merged commit 70514e3 into master Jan 25, 2024

kssenii deleted the filecache-small-optimization branch January 25, 2024 14:50

robot-ch-test-poll1 added the pr-synced-to-cloud The PR is synced to the cloud repo label Jan 25, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fs cache: small optimization#57615

fs cache: small optimization#57615
kssenii merged 25 commits intomasterfrom
filecache-small-optimization

kssenii commented Dec 7, 2023 •

edited

Loading

Uh oh!

robot-ch-test-poll commented Dec 7, 2023 •

edited by robot-ch-test-poll4

Loading

Uh oh!

Uh oh!

kssenii Jan 25, 2024 •

edited

Loading

Uh oh!

nickitat Jan 25, 2024

Uh oh!

kssenii Jan 25, 2024

Uh oh!

kssenii commented Jan 25, 2024

Uh oh!

Uh oh!

nickitat Jan 25, 2024

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

kssenii commented Dec 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changelog category (leave one):

Uh oh!

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

Uh oh!

Uh oh!

Uh oh!

kssenii Jan 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickitat Jan 25, 2024

Choose a reason for hiding this comment

Uh oh!

kssenii Jan 25, 2024

Choose a reason for hiding this comment

Uh oh!

kssenii commented Jan 25, 2024

Uh oh!

Uh oh!

nickitat Jan 25, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kssenii commented Dec 7, 2023 •

edited

Loading

robot-ch-test-poll commented Dec 7, 2023 •

edited by robot-ch-test-poll4

Loading

kssenii Jan 25, 2024 •

edited

Loading