Improve checksum checks for async INSERT into Distributed on the sender#18853
Merged
alexey-milovidov merged 9 commits intoClickHouse:masterfrom Jan 15, 2021
Merged
Improve checksum checks for async INSERT into Distributed on the sender#18853alexey-milovidov merged 9 commits intoClickHouse:masterfrom
alexey-milovidov merged 9 commits intoClickHouse:masterfrom
Conversation
azat
commented
Jan 8, 2021
…e sending This is already done for distributed_directory_monitor_batch_inserts=1, so let's do the same for the non batched mode, since otherwise in case the file will be truncated the receiver will just stuck (since it will wait for the block, but the sender will not send it).
… on sending Before this patch StorageDistributedDirectoryMonitor reading .bin files in batch mode, just to calculate number of bytes/rows, this is very ineffective, let's just store them in the header (rows/bytes).
f070693 to
b7a5bd3
Compare
Buffer for reading from a compressed file with just checking checksums of the compressed blocks, without any decompression, so result can be proxied.
Before this patch the DirectoryMonitor was checking the compressed file by reading it one more time (since w/o this receiver may stuck on truncated file), while this is ineffective and we can just check the checksums before sending. But note that this may decrease batch size that is used for sending over network.
b7a5bd3 to
2565d2a
Compare
Member
|
Fuzzer has found unrelated issue #18887 |
Since now CompressedReadBufferBase read checksum and header at once it will expect 25 (16+9) bytes not 16 (only checksum)
azat
commented
Jan 13, 2021
Member
Author
|
Here are some numbers, in general, they shows that patchedcreate table data (key Int, value String) engine=Null();
create table dist (key Int, value String) engine=Distributed(test_shard_localhost, currentDatabase(), data);
system stop distributed sends dist;
-- insert into Null() directly, just foward blocks to Null engine.
insert into dist select number, reinterpretAsString(number) from system.numbers limit 1e9 settings prefer_localhost_replica=1;
-- 0 rows in set. Elapsed: 16.888 sec. Processed 1.00 billion rows, 8.00 GB (59.23 million rows/s., 473.85 MB/s.)
-- async INSERT, with storing blocks on disk
insert into dist select number, reinterpretAsString(number) from system.numbers limit 1e9 settings prefer_localhost_replica=0;
-- 0 rows in set. Elapsed: 44.010 sec. Processed 1.00 billion rows, 8.00 GB (22.73 million rows/s., 181.83 MB/s.)
-- distributed_directory_monitor_batch_inserts=0
system flush distributed dist;
-- pending *.bin size is 7.5G
-- 0 rows in set. Elapsed: 22.445 sec.
-- distributed_directory_monitor_batch_inserts=1
system flush distributed dist;
-- first run
-- 0 rows in set. Elapsed: 50.845 sec.;
-- second run
-- 0 rows in set. Elapsed: 39.750 sec.;upstream-- distributed_directory_monitor_batch_inserts=0
system flush distributed dist;
-- 0 rows in set. Elapsed: 21.459 sec.
-- distributed_directory_monitor_batch_inserts=1
system flush distributed dist;
-- 0 rows in set. Elapsed: 45.488 sec.And I believe that batching mode overhead can be eliminated by removing extra file reading. NOTE: INSERT was done using the same version as |
Member
Author
Confirmed, w/o splitting by block header it is 2x faster (since does not requires extra file reading after other patches in this series) |
Before this patch batched mode of the DirectoryMonitor is 2x slower then non-batched, after it should be more or less the same as non-batched.
Member
Author
|
alexey-milovidov
approved these changes
Jan 15, 2021
azat
added a commit
to azat/ClickHouse
that referenced
this pull request
Jan 19, 2021
- the sender will got ATTEMPT_TO_READ_AFTER_EOF (added in 946c275) when the client just go away, i.e. server had been restarted, and this is incorrect to mark the file as broken in this case. - since ClickHouse#18853 the file will be checked on the sender locally, and in case the file was truncated CANNOT_READ_ALL_DATA will be thrown. But before ClickHouse#18853 the sender will not receive ATTEMPT_TO_READ_AFTER_EOF from the client in case of file was truncated on the sender, since the client will just wait for more data, IOW just hang. - and I don't see how ATTEMPT_TO_READ_AFTER_EOF can be received while reading local file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Refs: #18369