Make cache composable, allow not to evict certain files (regarding .idx, .mrk, ..), delete old cache version by kssenii · Pull Request #36171 · ClickHouse/ClickHouse

kssenii · 2022-04-12T13:50:08Z

Changelog category (leave one):

Backward Incompatible Change

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

Make cache composable, allow not to evict certain files (regarding idx, mrk, ..), delete old cache version. Now it is possible to configure cache over Azure blob storage disk, over Local disk, over StaticWeb disk, etc. This PR is marked backward incompatible because cache configuration changes and in order for cache to work need to update the config file. Old cache will still be used with new configuration. The server will startup fine with the old cache configuration. Closes #36140. Closes #37889.

…-cache-composable

alesapin

Huge amount of work. It's hard to do really detailed review. But in general, it's a big improvement in flexibility.

src/Common/FileSegment.h

src/Disks/IO/CachedOnDiskReadBufferFromFile.cpp

alesapin · 2022-08-15T14:13:05Z

src/Disks/IO/CachedOnDiskReadBufferFromFile.cpp

-    return offset;
+    if (file_segments_holder && current_file_segment_it != file_segments_holder->file_segments.end())
+    {
+        // auto & file_segments = file_segments_holder->file_segments;


I want to leave this commented code, because it should be a good improvement to reduce number of seeks, but it is very complex and need to be done very carefully and I will not do it right now, but having it there will remind that it needs to be done.
Or I can remove this commented code and add a TODO if it is better.

alesapin · 2022-08-15T14:13:23Z

src/Disks/IO/CachedOnDiskReadBufferFromFile.cpp

+
+    if (allow_seeks)
+    {
+        if (whence != SEEK_SET && whence != SEEK_CUR)


Move this check out of if.

But when !allow_seeks only SEEK_SET is allowed. So current way seems more correct.

src/Disks/IO/CachedOnDiskWriteBufferFromFile.cpp

src/Disks/IO/CachedOnDiskReadBufferFromFile.cpp

alesapin · 2022-08-15T18:11:30Z

src/Disks/IO/ReadBufferFromWebServer.cpp

+{
+    return Range{
+        .left = static_cast<size_t>(offset),
+        .right = read_until_position ? std::optional{read_until_position - 1} : std::nullopt


Why - 1? Included?

Yes. A little inconsistent, but it happened. Because used as included in http read buffer.

src/Disks/ObjectStorages/Cached/CachedObjectStorage.cpp

alesapin · 2022-08-15T18:13:40Z

src/Disks/ObjectStorages/Cached/CachedObjectStorage.cpp

+        };
+
+        if (objects.size() != 1)
+            throw Exception(ErrorCodes::CANNOT_USE_CACHE, "Unable to read multiple objects, support not added");


Suggested change

throw Exception(ErrorCodes::CANNOT_USE_CACHE, "Unable to read multiple objects, support not added");

throw Exception(ErrorCodes::CANNOT_USE_CACHE, "Read of multiple objects is unsupported");

But does it mean that *Log tables (which do appends) will not work?

Well, I think I can remove this exception and multiple objects will work fine, just cache key will be deduces from the first path only. Anyway this logic is not applied for s3 and for it we have buffer->isIntegratedWithFIlesystemCache() == 1 and another implementation is used. For non-s3 I should add a test for *Log.

src/Disks/ObjectStorages/Cached/CachedObjectStorage.cpp

tavplubix · 2022-08-16T15:15:15Z

Changelog entry should explain what exactly is incompatible and how to do upgrade

kssenii · 2022-08-18T10:36:41Z

Stress test (debug) — Killed by signal (in clickhouse-server.log)

Logical error: 'Cannot find column in(status, _subquery8) in ActionsDAG result' -- not related to changes #40336

Stress test (memory) — Killed by signal (in clickhouse-server.log)

the same <Fatal> : Logical error: 'Cannot find column _subquery499 in ActionsDAG result'.

tavplubix · 2022-08-19T08:22:17Z

@kssenii, Stateless tests flaky check (address) failure was related, you merged PR with flaky test

tavplubix · 2022-08-19T12:44:41Z

src/IO/ReadBufferFromFileDescriptor.cpp

    /// This is a workaround of a read pass EOF bug in linux kernel with pread()
    if (file_size.has_value() && file_offset_of_buffer_end >= *file_size)
-        return false;
+         return false;


We should add a check for indentation to Style Check

Composable cache

9804c39

robot-ch-test-poll1 added the pr-backward-incompatible Pull request with backwards incompatible changes label Apr 12, 2022

kssenii assigned alesapin Apr 12, 2022

Allow non-evictable file segments based on predicate

37242a0

kssenii force-pushed the make-cache-composable branch from e41e30c to 37242a0 Compare April 12, 2022 14:55

kssenii added 2 commits April 13, 2022 15:11

Update config

7f78196

Merge with master

7cfc0ac

azat mentioned this pull request Apr 20, 2022

Strict taskstats parser #36351

Merged

kssenii added 6 commits April 21, 2022 12:33

Merge with master

24e3763

Fix test

1b008b0

Add support for exact right bounded reads for some other buffers

a395532

Save changes

8d741f1

Revert

8d3dfc6

Merge master

0c43b7b

kssenii force-pushed the make-cache-composable branch from 6bbed28 to 0c43b7b Compare April 26, 2022 14:11

This was referenced Apr 28, 2022

Update 00170_s3_cache.sql #36743

Merged

Update s3 tests config #36790

Closed

kssenii added 8 commits May 13, 2022 18:51

Merge master

58faaf0

Fixes after merge

47ff7b6

Better

646cf4c

Some fixes

f3c1f37

Better

5e3410f

Merge master

6a51dea

Merge master

e4e6e44

Update tests config

e501a5a

kssenii force-pushed the make-cache-composable branch from 58a709c to e501a5a Compare May 19, 2022 13:55

kssenii added 2 commits May 22, 2022 00:21

Continuation

ffd8d97

Merge master

9aa2019

kssenii force-pushed the make-cache-composable branch 2 times, most recently from 9fa9d02 to 3a087f5 Compare May 23, 2022 20:06

Finish support for persistent files

0ff5609

kssenii force-pushed the make-cache-composable branch from 688eab1 to 0ff5609 Compare August 10, 2022 14:55

kssenii added 2 commits August 10, 2022 21:09

Fix checks

3d61baf

Fix test

8faaa6d

kssenii force-pushed the make-cache-composable branch from 2aabe80 to 8faaa6d Compare August 11, 2022 09:00

kssenii and others added 5 commits August 11, 2022 14:13

Update 02382_filesystem_cache_persistent_files.sql

73e12ac

Merge master

768a207

Merge remote-tracking branch 'origin/make-cache-composable' into make…

b7c91b9

…-cache-composable

Update test

e203414

Merge master

fd6362d

alesapin approved these changes Aug 15, 2022

View reviewed changes

kssenii added 2 commits August 16, 2022 00:56

Merge master

eb26b21

May be fix database replicated check

52832ea

kssenii force-pushed the make-cache-composable branch from 3b4a042 to 52832ea Compare August 16, 2022 09:52

Review fixes

93816e7

kssenii force-pushed the make-cache-composable branch from f10b9a0 to 93816e7 Compare August 16, 2022 13:12

Fix

4399172

Better test

d193eee

kssenii force-pushed the make-cache-composable branch from 54b68f7 to d193eee Compare August 16, 2022 22:16

Update .reference

f9b6091

kssenii force-pushed the make-cache-composable branch from bab6dd1 to f9b6091 Compare August 17, 2022 13:34

Merge master

d63d34f

kssenii merged commit d8fd1f9 into ClickHouse:master Aug 18, 2022

tavplubix mentioned this pull request Aug 19, 2022

Disable flaky 02382_filesystem_cache_persistent_files #40386

Merged

tavplubix reviewed Aug 19, 2022

View reviewed changes

tavplubix mentioned this pull request Sep 12, 2022

BC check: Part was found on disk local_cache which is not defined in the storage policy #41218

Closed

UnamedRus mentioned this pull request Jul 17, 2023

In local storage, Is it possible to save the *.bin files into hdd when other files into ssd? #52093

Closed

	throw Exception(ErrorCodes::CANNOT_USE_CACHE, "Unable to read multiple objects, support not added");
	throw Exception(ErrorCodes::CANNOT_USE_CACHE, "Read of multiple objects is unsupported");

Conversation

kssenii commented Apr 12, 2022 • 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!

alesapin left a comment

Choose a reason for hiding this comment

Uh oh!

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!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tavplubix commented Aug 16, 2022

Uh oh!

kssenii commented Aug 18, 2022

Uh oh!

tavplubix commented Aug 19, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kssenii commented Apr 12, 2022 •

edited

Loading