The original motivation for this commit was that shared_ptr_helper used
std::shared_ptr<>() which does two heap allocations instead of
make_shared<>() which does a single allocation. Turned out that
1. the affected code (--> Storages/) is not on a hot path (rendering the
performance argument moot ...)
2. yet copying Storage objects is potentially dangerous and was
previously allowed.
Hence, this change
- removes shared_ptr_helper and as a result all inherited create() methods,
- instead, Storage objects are now created using make_shared<>() by the
caller (for that to work, many constructors had to be made public), and
- all Storage classes were marked as noncopyable using boost::noncopyable.
In sum, we are (likely) not making things faster but the code becomes
cleaner and harder to misuse.
CI founds [1], TSan report:
WARNING: ThreadSanitizer: data race (pid=497)
Write of size 8 at 0x7b1c00e4e8d8 by thread T922:
4 DB::LogSink::writeMarks() obj-x86_64-linux-gnu/../src/Storages/StorageLog.cpp:462:72 (clickhouse+0x1665cdaa)
5 DB::LogSink::consume() obj-x86_64-linux-gnu/../src/Storages/StorageLog.cpp:324:5 (clickhouse+0x1665c216)
Previous read of size 8 at 0x7b1c00e4e8d8 by thread T711:
1 DB::LogSource::readData()::$_0::operator()(bool) const::'lambda'()::operator()() const obj-x86_64-linux-gnu/../src/Storages/StorageLog.cpp:188:26 (clickhouse+0x16661baf)
9 DB::LogSource::readData() obj-x86_64-linux-gnu/../src/Storages/StorageLog.cpp:204:20 (clickhouse+0x1665bbcb)
[1]: https://clickhouse-test-reports.s3.yandex.net/29930/4bc90d1dd7dbd4b8a9b6920d00ca24e8b160358e/stress_test_(thread).html#fail1Fixes: #29470
v2: rework locking, but this produce deadlock for 01499_log_deadlock test
v3: introduce per-file marks mutex
