Fix reading of sparse columns from s3

[CurtizJ]

Changelog category (leave one):

• Bug Fix (user-visible misbehavior in official stable or prestable release)

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

Fix reading of sparse columns from MergeTree tables that store their data in S3.

[CurtizJ]

I'll try to add test a bit later.

[KochetovNicolai]

It looks like the main change is the removing of need_to_check_marks_from_the_right flag. But it is not clear why this may may be a fix. Could you write some comment about it?

[KochetovNicolai]

Upper bound looks ok. But may be an overkill. I think the only case when we can have many consequent granules with the same offset is a LowCardinality dictionary.

[CurtizJ]

Not only LowCardinality dictionary, but also values of sparse columns (which is the point of this PR) and empty arrays. I tried to generalize several cases with upper bound.

[CurtizJ]

Stress test: some issues with Keeper:

2022.06.13 23:01:36.845402 [ 114353 ] {} <Error> DatabaseReplicated (test_1jutmt): Code: 999. Coordination::Exception: Transaction failed (No node): Op #0, path: /test/db/test_2/replicas/s1|r1. (KEEPER_EXCEPTION), Stack trace (when copying this message, always include the lines below):

Stateless tests (release, DatabaseReplicated, actions): some issues with Keeper:

2022.06.13 14:20:53.422357 [ 675 ] {} <Warning> RaftInstance: peer (3) response error: failed to connect to peer 3, localhost:9236, error 111
2022.06.13 14:20:53.576857 [ 678 ] {} <Error> RaftInstance: failed to accept a rpc connection due to error 125

[alesapin]

Unfortunately, look like it breaks CI in rare cases: https://play.clickhouse.com/play?user=play#c2VsZWN0IAp0b1N0YXJ0T2ZEYXkoY2hlY2tfc3RhcnRfdGltZSkgYXMgZCwKY291bnQoKSwgZ3JvdXBVbmlxQXJyYXkocHVsbF9yZXF1ZXN0X251bWJlciksIGdyb3VwVW5pcUFycmF5KHN1YnN0cih0ZXN0X25hbWUsIDEsIDUpKSwgIGFueShyZXBvcnRfdXJsKQpmcm9tIGNoZWNrcyB3aGVyZSAnMjAyMi0wNC0wMScgPD0gY2hlY2tfc3RhcnRfdGltZSBhbmQgbm90IHRlc3RfbmFtZSBsaWtlICclcXVlcnlfcHJvZmlsZXIlJyBhbmQgY2hlY2tfbmFtZSBpbGlrZSAnJXN0YXRlJXMzJScgYW5kIHRlc3Rfc3RhdHVzIGluICgnRkFJTCcsICdGTEFLWScsICdFUlJPUicpIGdyb3VwIGJ5IGQgb3JkZXIgYnkgZCBkZXNj

I'm not sure but there just no related diff in other PRs.
Another suspicious PR is #38088, but it was merged after first failure like this:

0. DB::Exception::Exception(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, int, bool) @ 0xb8ab77a in /usr/bin/clickhouse
1. DB::ReadBuffer::readStrict(char*, unsigned long) @ 0xb908ab2 in /usr/bin/clickhouse
2. ? @ 0x1587d911 in /usr/bin/clickhouse
3. DB::ISerialization::deserializeBinaryBulkWithMultipleStreams(COW<DB::IColumn>::immutable_ptr<DB::IColumn>&, unsigned long, DB::ISerialization::DeserializeBinaryBulkSettings&, std::__1::shared_ptr<DB::ISerialization::DeserializeBinaryBulkState>&, std::__1::unordered_map<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, COW<DB::IColumn>::immutable_ptr<DB::IColumn>, std::__1::hash<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::equal_to<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::allocator<std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const, COW<DB::IColumn>::immutable_ptr<DB::IColumn> > > >*) const @ 0x1583b4f5 in /usr/bin/clickhouse
4. DB::MergeTreeReaderWide::readData(DB::NameAndTypePair const&, COW<DB::IColumn>::immutable_ptr<DB::IColumn>&, unsigned long, bool, unsigned long, unsigned long, std::__1::unordered_map<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, COW<DB::IColumn>::immutable_ptr<DB::IColumn>, std::__1::hash<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::equal_to<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::allocator<std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const, COW<DB::IColumn>::immutable_ptr<DB::IColumn> > > >&, bool) @ 0x16cb89af in /usr/bin/clickhouse
5. DB::MergeTreeReaderWide::readRows(unsigned long, unsigned long, bool, unsigned long, std::__1::vector<COW<DB::IColumn>::immutable_ptr<DB::IColumn>, std::__1::allocator<COW<DB::IColumn>::immutable_ptr<DB::IColumn> > >&) @ 0x16cb78fe in /usr/bin/clickhouse
6. DB::MergeTreeRangeReader::DelayedStream::finalize(std::__1::vector<COW<DB::IColumn>::immutable_ptr<DB::IColumn>, std::__1::allocator<COW<DB::IColumn>::immutable_ptr<DB::IColumn> > >&) @ 0x174c716a in /usr/bin/clickhouse
7. DB::MergeTreeRangeReader::startReadingChain(unsigned long, std::__1::deque<DB::MarkRange, std::__1::allocator<DB::MarkRange> >&) @ 0x174ccf0b in /usr/bin/clickhouse
8. DB::MergeTreeRangeReader::read(unsigned long, std::__1::deque<DB::MarkRange, std::__1::allocator<DB::MarkRange> >&) @ 0x174cac45 in /usr/bin/clickhouse
9. DB::MergeTreeRangeReader::read(unsigned long, std::__1::deque<DB::MarkRange, std::__1::allocator<DB::MarkRange> >&) @ 0x174caba8 in /usr/bin/clickhouse
10. DB::MergeTreeBaseSelectProcessor::readFromPartImpl() @ 0x174bffc0 in /usr/bin/clickhouse
11. DB::MergeTreeBaseSelectProcessor::readFromPart() @ 0x174c0aed in /usr/bin/clickhouse
12. DB::MergeTreeBaseSelectProcessor::generate() @ 0x174bf0bf in /usr/bin/clickhouse
13. DB::ISource::tryGenerate() @ 0x1718ea75 in /usr/bin/clickhouse
14. DB::ISource::work() @ 0x1718e606 in /usr/bin/clickhouse
15. DB::ExecutionThreadContext::executeTask() @ 0x171aa99a in /usr/bin/clickhouse
16. DB::PipelineExecutor::executeStepImpl(unsigned long, std::__1::atomic<bool>*) @ 0x1719f8be in /usr/bin/clickhouse
17. ? @ 0x171a1184 in /usr/bin/clickhouse
18. ThreadPoolImpl<std::__1::thread>::worker(std::__1::__list_iterator<std::__1::thread, void*>) @ 0xb9573b7 in /usr/bin/clickhouse
19. ? @ 0xb95a7dd in /usr/bin/clickhouse
20. ? @ 0x7f8fb95eb609 in ?
21. __clone @ 0x7f8fb9510133 in ?
 (version 22.7.1.198 (official build))

[tavplubix]

Stress test: some issues with Keeper:

It's not an issue with Keeper, it's #33867

Stateless tests (release, DatabaseReplicated, actions): some issues with Keeper:

It's not an issue with Keeper either, it's #37791

[alesapin]

let's try to revert, we have multiple failures on master each day.

[timson]

Hi! I get this error, could it be relevant to this fix?

*clickhouse.Exception: code: 33, message: Received from chi-tesseract-01-tesseract-2-1:9000. DB::Exception: 
Cannot read all data. Bytes read: 26. Bytes expected: 380.: (while reading column Metadata): 
(while reading from part /var/lib/clickhouse/disks/s3/data/testdb/samples_1day_local_store/20220619_0_367846_24/ 
from mark 22046 with max_rows_to_read = 8192): While executing MergeTreeThread. Stack trace:

0. DB::Exception::Exception(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, int, bool) @ 0xaebed1a in /usr/bin/clickhouse
1. DB::ReadBuffer::readStrict(char*, unsigned long) @ 0xaf18ad4 in /usr/bin/clickhouse
2. ? @ 0x1445d831 in /usr/bin/clickhouse
3. DB::SerializationLowCardinality::deserializeBinaryBulkWithMultipleStreams(COW<DB::IColumn>::immutable_ptr<DB::IColumn>&, unsigned long, DB::ISerialization::DeserializeBinaryBulkSettings&, std::__1::shared_ptr<DB::ISerialization::DeserializeBinaryBulkState>&, std::__1::unordered_map<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, COW<DB::IColumn>::immutable_ptr<DB::IColumn>, std::__1::hash<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::equal_to<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::allocator<std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const, COW<DB::IColumn>::immutable_ptr<DB::IColumn> > > >*) const @ 0x14445517 in /usr/bin/clickhouse
4. DB::MergeTreeReaderWide::readData(DB::NameAndTypePair const&, COW<DB::IColumn>::immutable_ptr<DB::IColumn>&, unsigned long, bool, unsigned long, unsigned long, std::__1::unordered_map<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, COW<DB::IColumn>::immutable_ptr<DB::IColumn>, std::__1::hash<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::equal_to<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::allocator<std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const, COW<DB::IColumn>::immutable_ptr<DB::IColumn> > > >&, bool) @ 0x1558da8f in /usr/bin/clickhouse
5. DB::MergeTreeReaderWide::readRows(unsigned long, unsigned long, bool, unsigned long, std::__1::vector<COW<DB::IColumn>::immutable_ptr<DB::IColumn>, std::__1::allocator<COW<DB::IColumn>::immutable_ptr<DB::IColumn> > >&) @ 0x1558ca0b in /usr/bin/clickhouse
6. DB::MergeTreeRangeReader::DelayedStream::finalize(std::__1::vector<COW<DB::IColumn>::immutable_ptr<DB::IColumn>, std::__1::allocator<COW<DB::IColumn>::immutable_ptr<DB::IColumn> > >&) @ 0x15d2630e in /usr/bin/clickhouse
7. DB::MergeTreeRangeReader::startReadingChain(unsigned long, std::__1::deque<DB::MarkRange, std::__1::allocator<DB::MarkRange> >&) @ 0x15d2ace0 in /usr/bin/clickhouse
8. DB::MergeTreeRangeReader::read(unsigned long, std::__1::deque<DB::MarkRange, std::__1::allocator<DB::MarkRange> >&) @ 0x15d294d5 in /usr/bin/clickhouse
9. DB::MergeTreeBaseSelectProcessor::readFromPartImpl() @ 0x15d1f388 in /usr/bin/clickhouse
10. DB::MergeTreeBaseSelectProcessor::readFromPart() @ 0x15d2048d in /usr/bin/clickhouse
11. DB::MergeTreeBaseSelectProcessor::generate() @ 0x15d1ec00 in /usr/bin/clickhouse
12. DB::ISource::tryGenerate() @ 0x15a40dd5 in /usr/bin/clickhouse
13. DB::ISource::work() @ 0x15a4099a in /usr/bin/clickhouse
14. DB::SourceWithProgress::work() @ 0x15c92a62 in /usr/bin/clickhouse
15. DB::ExecutionThreadContext::executeTask() @ 0x15a60ca3 in /usr/bin/clickhouse
16. DB::PipelineExecutor::executeStepImpl(unsigned long, std::__1::atomic<bool>*) @ 0x15a54b7e in /usr/bin/clickhouse
17. ? @ 0x15a56468 in /usr/bin/clickhouse
18. ThreadPoolImpl<std::__1::thread>::worker(std::__1::__list_iterator<std::__1::thread, void*>) @ 0xaf62837 in /usr/bin/clickhouse
19. ? @ 0xaf662fd in /usr/bin/clickhouse
20. ? @ 0x7fc296657609 in ?
21. clone @ 0x7fc29657e293 in ?
: While executing Remote
gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer.(*span).setTagError
	/go/pkg/mod/gopkg.in/!data!dog/dd-trace-go.v1@v1.34.0/ddtrace/tracer/span.go:176
gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer.(*span).Finish
	/go/pkg/mod/gopkg.in/!data!dog/dd-trace-go.v1@v1.34.0/ddtrace/tracer/span.go:310
restflamedb/db.(*ClickHouseClient).GetTopFrames.func1
	/go/src/db/clickhouse.go:319
runtime.goexit
	/usr/local/go/src/runtime/asm_amd64.s:1571