Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix race in WriteBufferFromS3, add TSA annotations #40950

Merged
merged 5 commits into from
Sep 5, 2022
Merged

Fix race in WriteBufferFromS3, add TSA annotations #40950

merged 5 commits into from
Sep 5, 2022

Conversation

kssenii
Copy link
Member

@kssenii kssenii commented Sep 2, 2022

Changelog category (leave one):

  • Improvement

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

Fix race in WriteBufferFromS3, add TSA annotations.

WARNING: ThreadSanitizer: data race (pid=658)
  Read of size 8 at 0x7b54043eacc0 by thread T279:
    #0 <null> <null> (clickhouse+0x1a4a534b) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #1 <null> <null> (clickhouse+0x1a4a62a3) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #2 <null> <null> (clickhouse+0x1bb4a187) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #3 <null> <null> (clickhouse+0xb6c5d06) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #4 <null> <null> (clickhouse+0xb6c88cc) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #5 <null> <null> (clickhouse+0xb6c8841) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #6 <null> <null> (clickhouse+0xb6c3641) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #7 <null> <null> (clickhouse+0xb6c6df1) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #8 <null> <null> (clickhouse+0xb563dc6) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)



➜  ~ addr2line -afiCe ./clickhouse.1 0x1a4a534b 0x1a4a62a3 0x1bb4a187 0xb6c5d06 0xb6c88cc 0xb6c8841
0x000000001a4a534b
std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > >::size() const
./build_docker/../contrib/libcxx/include/vector:505
DB::WriteBufferFromS3::processUploadRequest(DB::WriteBufferFromS3::UploadPartTask&)
./build_docker/../src/IO/WriteBufferFromS3.cpp:305
0x000000001a4a62a3
operator()
./build_docker/../src/IO/WriteBufferFromS3.cpp:260
decltype ((static_cast<DB::WriteBufferFromS3::writePart()::$_1&>({parm#1}))()) std::__1::__invoke<DB::WriteBufferFromS3::writePart()::$_1&>(DB::WriteBufferFromS3::writePart()::$_1&)
./build_docker/../contrib/libcxx/include/type_traits:3640
void std::__1::__invoke_void_return_wrapper<void, true>::__call<DB::WriteBufferFromS3::writePart()::$_1&>(DB::WriteBufferFromS3::writePart()::$_1&)
./build_docker/../contrib/libcxx/include/__functional/invoke.h:61
std::__1::__function::__default_alloc_func<DB::WriteBufferFromS3::writePart()::$_1, void ()>::operator()()
./build_docker/../contrib/libcxx/include/__functional/function.h:230
void std::__1::__function::__policy_invoker<void ()>::__call_impl<std::__1::__function::__default_alloc_func<DB::WriteBufferFromS3::writePart()::$_1, void ()> >(std::__1::__function::__policy_storage const*)
./build_docker/../contrib/libcxx/include/__functional/function.h:711
0x000000001bb4a187
std::__1::__function::__policy_func<void ()>::operator()() const
./build_docker/../contrib/libcxx/include/__functional/function.h:843
std::__1::function<void ()>::operator()() const
./build_docker/../contrib/libcxx/include/__functional/function.h:1184
operator()
./build_docker/../src/Interpreters/threadPoolCallbackRunner.cpp:35
decltype ((static_cast<DB::threadPoolCallbackRunner(ThreadPoolImpl<ThreadFromGlobalPool>&)::$_0::operator()<std::__1::function<void ()> >(std::__1::function<void ()>)::{lambda()#1}&>({parm#1}))()) std::__1::__invoke<DB::threadPoolCallbackRunner(ThreadPoolImpl<ThreadFromGlobalPool>&)::$_0::operator()<std::__1::function<void ()> >(std::__1::function<void ()>)::{lambda()#1}&>(DB::threadPoolCallbackRunner(ThreadPoolImpl<ThreadFromGlobalPool>&)::$_0::operator()<std::__1::function<void ()> >(std::__1::function<void ()>)::{lambda()#1}&)
./build_docker/../contrib/libcxx/include/type_traits:3640
void std::__1::__invoke_void_return_wrapper<void, true>::__call<DB::threadPoolCallbackRunner(ThreadPoolImpl<ThreadFromGlobalPool>&)::$_0::operator()<std::__1::function<void ()> >(std::__1::function<void ()>)::{lambda()#1}&>(DB::threadPoolCallbackRunner(ThreadPoolImpl<ThreadFromGlobalPool>&)::$_0::operator()<std::__1::function<void ()> >(std::__1::function<void ()>)::{lambda()#1}&)
./build_docker/../contrib/libcxx/include/__functional/invoke.h:61
std::__1::__function::__default_alloc_func<DB::threadPoolCallbackRunner(ThreadPoolImpl<ThreadFromGlobalPool>&)::$_0::operator()<std::__1::function<void ()> >(std::__1::function<void ()>)::{lambda()#1}, void ()>::operator()()
./build_docker/../contrib/libcxx/include/__functional/function.h:230
void std::__1::__function::__policy_invoker<void ()>::__call_impl<std::__1::__function::__default_alloc_func<DB::threadPoolCallbackRunner(ThreadPoolImpl<ThreadFromGlobalPool>&)::$_0::operator()<std::__1::function<void ()> >(std::__1::function<void ()>)::{lambda()#1}, void ()> >(std::__1::__function::__policy_storage const*)
./build_docker/../contrib/libcxx/include/__functional/function.h:711
0x000000000b6c5d06
std::__1::__function::__policy_func<void ()>::operator()() const
./build_docker/../contrib/libcxx/include/__functional/function.h:843
std::__1::function<void ()>::operator()() const
./build_docker/../contrib/libcxx/include/__functional/function.h:1184
ThreadPoolImpl<ThreadFromGlobalPool>::worker(std::__1::__list_iterator<ThreadFromGlobalPool, void*>)
./build_docker/../src/Common/ThreadPool.cpp:281
0x000000000b6c88cc
operator()
./build_docker/../src/Common/ThreadPool.cpp:143
decltype ((static_cast<ThreadPoolImpl<ThreadFromGlobalPool>::scheduleImpl<void>(std::__1::function<void ()>, int, std::__1::optional<unsigned long>)::{lambda()#2}&>({parm#1}))()) std::__1::__invoke_constexpr<ThreadPoolImpl<ThreadFromGlobalPool>::scheduleImpl<void>(std::__1::function<void ()>, int, std::__1::optional<unsigned long>)::{lambda()#2}&>(ThreadPoolImpl<ThreadFromGlobalPool>::scheduleImpl<void>(std::__1::function<void ()>, int, std::__1::optional<unsigned long>)::{lambda()#2}&)
./build_docker/../contrib/libcxx/include/type_traits:3648
decltype(auto) std::__1::__apply_tuple_impl<ThreadPoolImpl<ThreadFromGlobalPool>::scheduleImpl<void>(std::__1::function<void ()>, int, std::__1::optional<unsigned long>)::{lambda()#2}&, std::__1::tuple<>&>(ThreadPoolImpl<ThreadFromGlobalPool>::scheduleImpl<void>(std::__1::function<void ()>, int, std::__1::optional<unsigned long>)::{lambda()#2}&, std::__1::tuple<>&, std::__1::__tuple_indices<>)
./build_docker/../contrib/libcxx/include/tuple:1595
decltype(auto) std::__1::apply<ThreadPoolImpl<ThreadFromGlobalPool>::scheduleImpl<void>(std::__1::function<void ()>, int, std::__1::optional<unsigned long>)::{lambda()#2}&, std::__1::tuple<>&>(ThreadPoolImpl<ThreadFromGlobalPool>::scheduleImpl<void>(std::__1::function<void ()>, int, std::__1::optional<unsigned long>)::{lambda()#2}&, std::__1::tuple<>&)
./build_docker/../contrib/libcxx/include/tuple:1604
operator()
./build_docker/../src/Common/ThreadPool.h:187
decltype ((static_cast<ThreadFromGlobalPool::ThreadFromGlobalPool<ThreadPoolImpl<ThreadFromGlobalPool>::scheduleImpl<void>(std::__1::function<void ()>, int, std::__1::optional<unsigned long>)::{lambda()#2}>(ThreadPoolImpl<ThreadFromGlobalPool>::scheduleImpl<void>(std::__1::function<void ()>, int, std::__1::optional<unsigned long>)::{lambda()#2}&&)::{lambda()#1}&>({parm#1}))()) std::__1::__invoke<ThreadFromGlobalPool::ThreadFromGlobalPool<ThreadPoolImpl<ThreadFromGlobalPool>::scheduleImpl<void>(std::__1::function<void ()>, int, std::__1::optional<unsigned long>)::{lambda()#2}>(ThreadPoolImpl<ThreadFromGlobalPool>::scheduleImpl<void>(std::__1::function<void ()>, int, std::__1::optional<unsigned long>)::{lambda()#2}&&)::{lambda()#1}&>(ThreadPoolImpl<ThreadFromGlobalPool>::scheduleImpl<void>(std::__1::function<void ()>, int, std::__1::optional<unsigned long>)::{lambda()#2}&&)
./build_docker/../contrib/libcxx/include/type_traits:3640



  Previous write of size 8 at 0x7b54043eacc0 by thread T2200 (mutexes: write M0):
    #0 <null> <null> (clickhouse+0x1a4a1db7) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #1 <null> <null> (clickhouse+0x1a4a02c1) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #2 <null> <null> (clickhouse+0x1aa20014) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #3 <null> <null> (clickhouse+0x1c5e33ca) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #4 <null> <null> (clickhouse+0xb614fbd) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #5 <null> <null> (clickhouse+0x1a699357) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #6 <null> <null> (clickhouse+0x1c5e33ca) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #7 <null> <null> (clickhouse+0xb614fbd) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #8 <null> <null> (clickhouse+0x1a93e38d) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #9 <null> <null> (clickhouse+0x1a8bb743) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #10 <null> <null> (clickhouse+0x1c76e24b) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #11 <null> <null> (clickhouse+0x1c76d628) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #12 <null> <null> (clickhouse+0x1c76bbb5) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #13 <null> <null> (clickhouse+0x1c88e7f8) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #14 <null> <null> (clickhouse+0x1c665e63) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #15 <null> <null> (clickhouse+0x1c672781) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #16 <null> <null> (clickhouse+0x1c665d14) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #17 <null> <null> (clickhouse+0x1c66b508) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #18 <null> <null> (clickhouse+0x1c9b1552) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #19 <null> <null> (clickhouse+0x1c98ceaf) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #20 <null> <null> (clickhouse+0x1c9917ee) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #21 <null> <null> (clickhouse+0x1b70e55c) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #22 <null> <null> (clickhouse+0x1bb13b1b) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #23 <null> <null> (clickhouse+0x1bb109cb) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #24 <null> <null> (clickhouse+0x1cb2a6c5) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #25 <null> <null> (clickhouse+0x1cb3e1a7) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #26 <null> <null> (clickhouse+0x1fe78762) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #27 <null> <null> (clickhouse+0x1fe78fd2) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #28 <null> <null> (clickhouse+0x200d0979) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #29 <null> <null> (clickhouse+0x200ced8f) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)
    #30 <null> <null> (clickhouse+0x200cd3c7) (BuildId: d2ebb17bed752608aee1f7dc96da8a6be947340c)



➜  ~ addr2line -afiCe ./clickhouse.1 0x1a4a1db7 0x1a4a02c1 0x1aa20014 0x1c5e33ca 0xb614fbd 0x1a699357 0x1c5e33ca 0xb614fbd 0x1a93e38d 0x1a8bb743 0x1c76e24b
0x000000001a4a1db7
~_ConstructTransaction
./build_docker/../contrib/libcxx/include/vector:757
void std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > >::__construct_one_at_end<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&>(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)
./build_docker/../contrib/libcxx/include/vector:781
std::__1::vector<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > >::push_back(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)
./build_docker/../contrib/libcxx/include/vector:1531
DB::WriteBufferFromS3::waitForReadyBackGroundTasks()
./build_docker/../src/IO/WriteBufferFromS3.cpp:449
0x000000001a4a02c1
DB::WriteBufferFromS3::nextImpl()
./build_docker/../src/IO/WriteBufferFromS3.cpp:105
0x000000001aa20014
DB::WriteBuffer::next()
./build_docker/../src/IO/WriteBuffer.h:49
DB::WriteBufferFromFileDecorator::nextImpl()
./build_docker/../src/IO/WriteBufferFromFileDecorator.cpp:51
0x000000001c5e33ca
DB::WriteBuffer::next()
./build_docker/../src/IO/WriteBuffer.h:49
DB::HashingWriteBuffer::nextImpl()
./build_docker/../src/IO/HashingWriteBuffer.h:64
0x000000000b614fbd
DB::WriteBuffer::next()
./build_docker/../src/IO/WriteBuffer.h:49
DB::WriteBuffer::nextIfAtEnd()
./build_docker/../src/IO/WriteBuffer.h:71
DB::WriteBuffer::write(char const*, unsigned long)
./build_docker/../src/IO/WriteBuffer.h:87
0x000000001a699357
DB::CompressedWriteBuffer::nextImpl()
./build_docker/../src/Compression/CompressedWriteBuffer.cpp:53
0x000000001c5e33ca
DB::WriteBuffer::next()
./build_docker/../src/IO/WriteBuffer.h:49
DB::HashingWriteBuffer::nextImpl()
./build_docker/../src/IO/HashingWriteBuffer.h:64
0x000000000b614fbd
DB::WriteBuffer::next()
./build_docker/../src/IO/WriteBuffer.h:49
DB::WriteBuffer::nextIfAtEnd()
./build_docker/../src/IO/WriteBuffer.h:71
DB::WriteBuffer::write(char const*, unsigned long)
./build_docker/../src/IO/WriteBuffer.h:87
0x000000001a93e38d
DB::SerializationString::serializeBinaryBulk(DB::IColumn const&, DB::WriteBuffer&, unsigned long, unsigned long) const
./build_docker/../src/DataTypes/Serializations/SerializationString.cpp:105
0x000000001a8bb743
DB::ISerialization::serializeBinaryBulkWithMultipleStreams(DB::IColumn const&, unsigned long, unsigned long, DB::ISerialization::SerializeBinaryBulkSettings&, std::__1::shared_ptr<DB::ISerialization::SerializeBinaryBulkState>&) const
./build_docker/../src/DataTypes/Serializations/ISerialization.cpp:114
0x000000001c76e24b
DB::MergeTreeDataPartWriterWide::writeSingleGranule(DB::NameAndTypePair const&, DB::IColumn const&, std::__1::set<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, std::__1::less<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >, std::__1::allocator<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > > >&, std::__1::shared_ptr<DB::ISerialization::SerializeBinaryBulkState>&, DB::ISerialization::SerializeBinaryBulkSettings&, DB::Granule const&)
./build_docker/../src/Storages/MergeTree/MergeTreeDataPartWriterWide.cpp:317

@kssenii
Copy link
Member Author

kssenii commented Sep 2, 2022

includes commit from #40943, so should be merged after.

@robot-ch-test-poll1 robot-ch-test-poll1 added the pr-improvement Pull request with some product improvements label Sep 2, 2022
@alesapin alesapin mentioned this pull request Sep 4, 2022
Merged
@alexey-milovidov alexey-milovidov self-assigned this Sep 5, 2022
@kssenii kssenii merged commit 64168da into ClickHouse:master Sep 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexey-milovidov alexey-milovidov alexey-milovidov approved these changes

Assignees

@alexey-milovidov alexey-milovidov

Labels
pr-improvement Pull request with some product improvements
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kssenii @alexey-milovidov @robot-ch-test-poll1