Skip to content

Conversation

@tedyu
Copy link

@tedyu tedyu commented Oct 31, 2022

There is data race between clear_and_update_back and update_partitions (observed via CppCassandraDriverTest_TestCreateUniqueIndexPartial)

WARNING: ThreadSanitizer: data race (pid=14928)
  Write of size 8 at 0x7b7c00001458 by thread T37:
    #0 cass::Metadata::clear_and_update_back(cass::VersionNumber const&) /opt/yb-build/thirdparty/yugabyte-db-thirdparty-v20221019013514-1aec6dda08-centos7-x86_64-clang15/src/cassandra-cpp-driver-2.9.0-yb-13/src/metadata.cpp:955:13 (libcassandra.so.2+0x1a4860)
    #1 cass::ControlConnection::on_query_meta_schema(cass::ControlConnection*, cass::ControlConnection::UnusedData const&, {std::}map<{string}, cass::SharedRefPtr<cass::Response>, {std::}less<{string}>, {std::}allocator<{std::}pair<{string} const, cass::SharedRefPtr<cass::Response>>>> const&) /opt/yb-build/thirdparty/yugabyte-db-thirdparty-v20221019013514-1aec6dda08-centos7-x86_64-clang15/src/cassandra-cpp-driver-2.9.0-yb-13/src/control_connection.cpp (libcassandra.so.2+0x1596dd)
    #2 cass::ControlConnection::ControlMultipleRequestCallback<cass::ControlConnection::UnusedData>::on_set({std::}map<{string}, cass::SharedRefPtr<cass::Response>, {std::}less<{string}>, {std::}allocator<{std::}pair<{string} const, cass::SharedRefPtr<cass::Response>>>> const&) /opt/yb-build/thirdparty/yugabyte-db-thirdparty-v20221019013514-1aec6dda08-centos7-x86_64-clang15/src/cassandra-cpp-driver-2.9.0-yb-13/src/control_connection.cpp:1171:3 (libcassandra.so.2+0x15e187)

  Previous read of size 8 at 0x7b7c00001458 by thread T38 (mutexes: write M0, write M1):
    #0 cass::Metadata::update_partitions(int, cass::VersionNumber const&, cass::ResultResponse*) /opt/yb-build/thirdparty/yugabyte-db-thirdparty-v20221019013514-1aec6dda08-centos7-x86_64-clang15/src/cassandra-cpp-driver-2.9.0-yb-13/src/metadata.cpp:895:3 (libcassandra.so.2+0x1a2379)
    #1 cass::Session::refresh_metadata_callback(CassFuture_*, void*) /opt/yb-build/thirdparty/yugabyte-db-thirdparty-v20221019013514-1aec6dda08-centos7-x86_64-clang15/src/cassandra-cpp-driver-2.9.0-yb-13/src/session.cpp:500:23 (libcassandra.so.2+0x20c2cd)
    #2 cass::Future::internal_set(cass::ScopedLock<cass::Mutex>&) /opt/yb-build/thirdparty/yugabyte-db-thirdparty-v20221019013514-1aec6dda08-centos7-x86_64-clang15/src/cassandra-cpp-driver-2.9.0-yb-13/src/future.cpp:190:5 (libcassandra.so.2+0x181c10)

This PR adds lock in clear_and_update_back.

Also fixes another race :

WARNING: ThreadSanitizer: data race (pid=14928)
  Write of size 4 at 0x7b7c00001480 by thread T37:
    #0 cass::Metadata::update_keyspaces(int, cass::VersionNumber const&, cass::ResultResponse*) /opt/yb-build/thirdparty/yugabyte-db-thirdparty-v20221019013514-1aec6dda08-centos7-x86_64-clang15/src/cassandra-cpp-driver-2.9.0-yb-13/src/metadata.cpp:799:27 (libcassandra.so.2+0x19b134)
    #1 cass::ControlConnection::on_query_meta_schema(cass::ControlConnection*, cass::ControlConnection::UnusedData const&, {std::}map<{string}, cass::SharedRefPtr<cass::Response>, {std::}less<{string}>, {std::}allocator<{std::}pair<{string} const, cass::SharedRefPtr<cass::Response>>>> const&) /opt/yb-build/thirdparty/yugabyte-db-thirdparty-v20221019013514-1aec6dda08-centos7-x86_64-clang15/src/cassandra-cpp-driver-2.9.0-yb-13/src/control_connection.cpp:597:27 (libcassandra.so.2+0x159763)

  Previous write of size 4 at 0x7b7c00001480 by thread T38 (mutexes: write M0, write M1):
    #0 cass::Metadata::update_partitions(int, cass::VersionNumber const&, cass::ResultResponse*) /opt/yb-build/thirdparty/yugabyte-db-thirdparty-v20221019013514-1aec6dda08-centos7-x86_64-clang15/src/cassandra-cpp-driver-2.9.0-yb-13/src/metadata.cpp:894:27 (libcassandra.so.2+0x1a236c)
    #1 cass::Session::refresh_metadata_callback(CassFuture_*, void*) /opt/yb-build/thirdparty/yugabyte-db-thirdparty-v20221019013514-1aec6dda08-centos7-x86_64-clang15/src/cassandra-cpp-driver-2.9.0-yb-13/src/session.cpp:500:23 (libcassandra.so.2+0x20c2cd)

schema_snapshot_version_ is converted to atomic int for dealing with the second race condition.

@tedyu tedyu requested a review from mbautin October 31, 2022 14:40
Copy link

@mbautin mbautin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment specifying why it is necessary to acquire this mutex (e.g. say what variable is guarded by what mutex) and also mention why there would be no deadlocks.

@tedyu tedyu force-pushed the meta-back branch 4 times, most recently from 36c7749 to a5305f0 Compare November 1, 2022 19:58
Copy link

@OlegLoginov OlegLoginov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Please add/describe a TEST PLAN.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants