kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a...@apache.org
Subject [kudu] branch master updated: messenger: adjust lock usage
Date Wed, 18 Sep 2019 18:06:58 GMT
This is an automated email from the ASF dual-hosted git repository.

adar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new 0ecc2c7  messenger: adjust lock usage
0ecc2c7 is described below

commit 0ecc2c7715505fa6d5a03f8ef967a1a96d4f55d5
Author: Adar Dembo <adar@cloudera.com>
AuthorDate: Tue Sep 17 12:09:28 2019 -0700

    messenger: adjust lock usage
    
    The Messenger's lock is only intended to protect closing_, acceptor_pools_,
    and rpc_services_. This change adjusts its usage to reflect that:
    1. There's no need to take the lock in the destructor.
    2. It was held for longer than necessary in QueueInboundCall.
    3. It wasn't needed at all in DumpConnections.
    
    The motivation for this was a TSAN lock inversion warning I saw in a
    precommit job, between the Messenger lock and glog's vmodule lock. The
    warning seems wrong (the vmodule lock is released after a VLOG statement
    ends), but one way to avoid it altogether is to not take the Messenger lock
    in its destructor.
    
    WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock) (pid=5867)
      Cycle in lock order graph: M1870 (0x7b14000172f8) => M37857528269694952 (0x000000000000)
=> M1870
    
      Mutex M37857528269694952 acquired here while holding mutex M1870 in main thread:
        #0 pthread_rwlock_wrlock /home/jenkins-slave/workspace/kudu-master/2/thirdparty/src/llvm-6.0.0.src/projects/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:1352
(kudu+0x4a360f)
        #1 glog_internal_namespace_::Mutex::Lock() /home/jenkins-slave/workspace/kudu-master/2/thirdparty/src/glog-0.3.5/src/base/mutex.h:250:30
(libglog.so.0+0x1abe7)
        #2 glog_internal_namespace_::MutexLock::MutexLock(glog_internal_namespace_::Mutex*)
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/src/glog-0.3.5/src/base/mutex.h:290
(libglog.so.0+0x1abe7)
        #3 google::InitVLOG3__(int**, int*, char const*, int) /home/jenkins-slave/workspace/kudu-master/2/thirdparty/src/glog-0.3.5/src/vlog_is_on.cc:199
(libglog.so.0+0x1abe7)
        #4 kudu::rpc::Messenger::ShutdownInternal(kudu::rpc::Messenger::ShutdownMode) /home/jenkins-slave/workspace/kudu-master/2/src/kudu/rpc/messenger.cc:283:5
(libkrpc.so+0xab101)
        #5 kudu::rpc::Messenger::AllExternalReferencesDropped() /home/jenkins-slave/workspace/kudu-master/2/src/kudu/rpc/messenger.cc:249:3
(libkrpc.so+0xaaeb7)
        #6 std::__1::mem_fun_t<void, kudu::rpc::Messenger>::operator()(kudu::rpc::Messenger*)
const /home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/functional:1120:17
(libkrpc.so+0xaf3a5)
        #7 std::__1::__shared_ptr_pointer<kudu::rpc::Messenger*, std::__1::mem_fun_t<void,
kudu::rpc::Messenger>, std::__1::allocator<kudu::rpc::Messenger> >::__on_zero_shared()
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:3586
(libkrpc.so+0xaf3a5)
        #8 std::__1::__shared_count::__release_shared() /home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:3490:9
(kudu+0x56affe)
        #9 std::__1::__shared_weak_count::__release_shared() /home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:3532
(kudu+0x56affe)
        #10 std::__1::shared_ptr<kudu::rpc::Messenger>::~shared_ptr() /home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:4468
(kudu+0x56affe)
        #11 kudu::client::KuduClient::Data::~Data() /home/jenkins-slave/workspace/kudu-master/2/src/kudu/client/client-internal.cc:179:1
(libkudu_client.so+0x136260)
        #12 kudu::client::KuduClient::~KuduClient() /home/jenkins-slave/workspace/kudu-master/2/src/kudu/client/client.cc:394:3
(libkudu_client.so+0x1130cc)
        #13 std::__1::default_delete<kudu::client::KuduClient>::operator()(kudu::client::KuduClient*)
const /home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:2285:5
(libkudu_client.so+0x12be1b)
        #14 std::__1::__shared_ptr_pointer<kudu::client::KuduClient*, std::__1::default_delete<kudu::client::KuduClient>,
std::__1::allocator<kudu::client::KuduClient> >::__on_zero_shared() /home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:3586
(libkudu_client.so+0x12be1b)
        #15 std::__1::__shared_count::__release_shared() /home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:3490:9
(kudu+0x550d1e)
        #16 std::__1::__shared_weak_count::__release_shared() /home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:3532
(kudu+0x550d1e)
        #17 std::__1::shared_ptr<kudu::client::KuduClient>::~shared_ptr() /home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:4468
(kudu+0x550d1e)
        #18 kudu::tools::LeaderMasterProxy::~LeaderMasterProxy() /home/jenkins-slave/workspace/kudu-master/2/src/kudu/tools/tool_action_common.h:233:7
(kudu+0x576cf9)
        #19 kudu::tools::(anonymous namespace)::ListMasters(kudu::tools::RunnerContext const&)
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/tools/tool_action_master.cc:180:1 (kudu+0x572d0b)
        #20 _ZNSt3__18__invokeIRPFN4kudu6StatusERKNS1_5tools13RunnerContextEEJS6_EEEDTclclsr3std3__1E7forwardIT_Efp_Espclsr3std3__1E7forwardIT0_Efp0_EEEOSA_DpOSB_
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/type_traits:4482:1
(kudu+0x52e48b)
        #21 kudu::Status std::__1::__invoke_void_return_wrapper<kudu::Status>::__call<kudu::Status
(*&)(kudu::tools::RunnerContext const&), kudu::tools::RunnerContext const&>(kudu::Status
(*&)(kudu::tools::RunnerContext const&), kudu::tools::RunnerContext const&) /home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/__functional_base:318
(kudu+0x52e48b)
        #22 std::__1::__function::__func<kudu::Status (*)(kudu::tools::RunnerContext const&),
std::__1::allocator<kudu::Status (*)(kudu::tools::RunnerContext const&)>, kudu::Status
(kudu::tools::RunnerContext const&)>::operator()(kudu::tools::RunnerContext const&)
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/functional:1562:12
(kudu+0x52e3bd)
        #23 std::__1::function<kudu::Status (kudu::tools::RunnerContext const&)>::operator()(kudu::tools::RunnerContext
const&) const /home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/functional:1916:12
(libkudu_tools_util.so+0x6c1c4)
        #24 kudu::tools::Action::Run(std::__1::vector<kudu::tools::Mode*, std::__1::allocator<kudu::tools::Mode*>
> const&, std::__1::unordered_map<std::__1::basic_string<char, std::__1::char_traits<char>,
std::__1::allocator<char> >, std::__1::basic_string<char, std::__1::char_traits<char>,
std::__1::allocator<char> >, 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_trai [...]
        #25 kudu::tools::DispatchCommand(std::__1::vector<kudu::tools::Mode*, std::__1::allocator<kudu::tools::Mode*>
> const&, kudu::tools::Action*, std::__1::deque<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> > > > const&)
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/tools/tool_main.cc:132:15 (kudu+0x5b42b6)
        #26 kudu::tools::RunTool(int, char**, bool) /home/jenkins-slave/workspace/kudu-master/2/src/kudu/tools/tool_main.cc:204:16
(kudu+0x5b5211)
        #27 main /home/jenkins-slave/workspace/kudu-master/2/src/kudu/tools/tool_main.cc:265:10
(kudu+0x5b557e)
    
        Hint: use TSAN_OPTIONS=second_deadlock_stack=1 to get more informative warning message
    
      Mutex M1870 acquired here while holding mutex M37857528269694952 in thread T8:
        #0 AnnotateRWLockAcquired /home/jenkins-slave/workspace/kudu-master/2/thirdparty/src/llvm-6.0.0.src/projects/compiler-rt/lib/tsan/rtl/tsan_interface_ann.cc:271
(kudu+0x4d53ff)
        #1 kudu::rw_spinlock::lock() /home/jenkins-slave/workspace/kudu-master/2/src/kudu/util/locks.h:112:5
(libkudu_client.so+0x177762)
        #2 kudu::percpu_rwlock::lock() /home/jenkins-slave/workspace/kudu-master/2/src/kudu/util/locks.h:222:22
(libkudu_client.so+0x1776f2)
        #3 std::__1::lock_guard<kudu::percpu_rwlock>::lock_guard(kudu::percpu_rwlock&)
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/__mutex_base:104:27
(libkrpc.so+0xac9c9)
        #4 kudu::rpc::Messenger::~Messenger() /home/jenkins-slave/workspace/kudu-master/2/src/kudu/rpc/messenger.cc:430
(libkrpc.so+0xac9c9)
        #5 std::__1::default_delete<kudu::rpc::Messenger>::operator()(kudu::rpc::Messenger*)
const /home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:2285:5
(libkrpc.so+0xb246b)
        #6 std::__1::__shared_ptr_pointer<kudu::rpc::Messenger*, std::__1::default_delete<kudu::rpc::Messenger>,
std::__1::allocator<kudu::rpc::Messenger> >::__on_zero_shared() /home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:3586
(libkrpc.so+0xb246b)
        #7 std::__1::__shared_count::__release_shared() /home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:3490:9
(kudu+0x56affe)
        #8 std::__1::__shared_weak_count::__release_shared() /home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:3532
(kudu+0x56affe)
        #9 std::__1::shared_ptr<kudu::rpc::Messenger>::~shared_ptr() /home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:4468
(kudu+0x56affe)
        #10 std::__1::shared_ptr<kudu::rpc::Messenger>::reset() /home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/memory:4603:5
(libkrpc.so+0xc0771)
        #11 kudu::rpc::ReactorThread::RunThread() /home/jenkins-slave/workspace/kudu-master/2/src/kudu/rpc/reactor.cc:499
(libkrpc.so+0xc0771)
        #12 boost::_mfi::mf0<void, kudu::rpc::ReactorThread>::operator()(kudu::rpc::ReactorThread*)
const /home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/boost/bind/mem_fn_template.hpp:49:29
(libkrpc.so+0xca669)
        #13 void boost::_bi::list1<boost::_bi::value<kudu::rpc::ReactorThread*> >::operator()<boost::_mfi::mf0<void,
kudu::rpc::ReactorThread>, boost::_bi::list0>(boost::_bi::type<void>, boost::_mfi::mf0<void,
kudu::rpc::ReactorThread>&, boost::_bi::list0&, int) /home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/boost/bind/bind.hpp:259:9
(libkrpc.so+0xca5ba)
        #14 boost::_bi::bind_t<void, boost::_mfi::mf0<void, kudu::rpc::ReactorThread>,
boost::_bi::list1<boost::_bi::value<kudu::rpc::ReactorThread*> > >::operator()()
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/boost/bind/bind.hpp:1222:16
(libkrpc.so+0xca543)
        #15 boost::detail::function::void_function_obj_invoker0<boost::_bi::bind_t<void,
boost::_mfi::mf0<void, kudu::rpc::ReactorThread>, boost::_bi::list1<boost::_bi::value<kudu::rpc::ReactorThread*>
> >, void>::invoke(boost::detail::function::function_buffer&) /home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/boost/function/function_template.hpp:159:11
(libkrpc.so+0xca339)
        #16 boost::function0<void>::operator()() const /home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/boost/function/function_template.hpp:770:14
(libkrpc.so+0xba0b1)
        #17 kudu::Thread::SuperviseThread(void*) /home/jenkins-slave/workspace/kudu-master/2/src/kudu/util/thread.cc:657:3
(libkudu_util.so+0x1ee174)
    
      Thread T8 'rpc reactor-588' (tid=5886, running) created by main thread at:
        #0 pthread_create /home/jenkins-slave/workspace/kudu-master/2/thirdparty/src/llvm-6.0.0.src/projects/compiler-rt/lib/tsan/rtl/tsan_interceptors.cc:992
(kudu+0x490e36)
        #1 kudu::Thread::StartThread(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&, boost::function<void ()> const&,
unsigned long, scoped_refptr<kudu::Thread>*) /home/jenkins-slave/workspace/kudu-master/2/src/kudu/util/thread.cc:601:15
(libkudu_util.so+0x1ed95b)
        #2 kudu::Status kudu::Thread::Create<void (kudu::rpc::ReactorThread::*)(), kudu::rpc::ReactorThread*>(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&, void (kudu::rpc::ReactorThread::*
const&)(), kudu::rpc::ReactorThread* const&, scoped_refptr<kudu::Thread>*) /home/jenkins-slave/workspace/kudu-master/2/src/kudu/util/thread.h:164:12
(libkrpc.so+ [...]
        #3 kudu::rpc::ReactorThread::Init() /home/jenkins-slave/workspace/kudu-master/2/src/kudu/rpc/reactor.cc:185:10
(libkrpc.so+0xc026e)
        #4 kudu::rpc::Reactor::Init() /home/jenkins-slave/workspace/kudu-master/2/src/kudu/rpc/reactor.cc:759:18
(libkrpc.so+0xc4911)
        #5 kudu::rpc::Messenger::Init() /home/jenkins-slave/workspace/kudu-master/2/src/kudu/rpc/messenger.cc:446:5
(libkrpc.so+0xaad72)
        #6 kudu::rpc::MessengerBuilder::Build(std::__1::shared_ptr<kudu::rpc::Messenger>*)
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/rpc/messenger.cc:205:3 (libkrpc.so+0xaa7cd)
        #7 kudu::client::KuduClientBuilder::Build(std::__1::shared_ptr<kudu::client::KuduClient>*)
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/client/client.cc:349:3 (libkudu_client.so+0x112561)
        #8 kudu::tools::LeaderMasterProxy::Init(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> > > > const&,
kudu::MonoDelta const&) /home/jenkins-slave/workspace/kudu-master/2/src/kudu/tools/tool_action_common.cc:786:30
(libkudu_tools_util.so+0x7740c)
        #9 kudu::tools::LeaderMasterProxy::Init(kudu::tools::RunnerContext const&) /home/jenkins-slave/workspace/kudu-master/2/src/kudu/tools/tool_action_common.cc:792:10
(libkudu_tools_util.so+0x774d6)
        #10 kudu::tools::(anonymous namespace)::ListMasters(kudu::tools::RunnerContext const&)
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/tools/tool_action_master.cc:109:3 (kudu+0x572be3)
        #11 _ZNSt3__18__invokeIRPFN4kudu6StatusERKNS1_5tools13RunnerContextEEJS6_EEEDTclclsr3std3__1E7forwardIT_Efp_Espclsr3std3__1E7forwardIT0_Efp0_EEEOSA_DpOSB_
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/type_traits:4482:1
(kudu+0x52e48b)
        #12 kudu::Status std::__1::__invoke_void_return_wrapper<kudu::Status>::__call<kudu::Status
(*&)(kudu::tools::RunnerContext const&), kudu::tools::RunnerContext const&>(kudu::Status
(*&)(kudu::tools::RunnerContext const&), kudu::tools::RunnerContext const&) /home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/__functional_base:318
(kudu+0x52e48b)
        #13 std::__1::__function::__func<kudu::Status (*)(kudu::tools::RunnerContext const&),
std::__1::allocator<kudu::Status (*)(kudu::tools::RunnerContext const&)>, kudu::Status
(kudu::tools::RunnerContext const&)>::operator()(kudu::tools::RunnerContext const&)
/home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/functional:1562:12
(kudu+0x52e3bd)
        #14 std::__1::function<kudu::Status (kudu::tools::RunnerContext const&)>::operator()(kudu::tools::RunnerContext
const&) const /home/jenkins-slave/workspace/kudu-master/2/thirdparty/installed/tsan/include/c++/v1/functional:1916:12
(libkudu_tools_util.so+0x6c1c4)
        #15 kudu::tools::Action::Run(std::__1::vector<kudu::tools::Mode*, std::__1::allocator<kudu::tools::Mode*>
> const&, std::__1::unordered_map<std::__1::basic_string<char, std::__1::char_traits<char>,
std::__1::allocator<char> >, std::__1::basic_string<char, std::__1::char_traits<char>,
std::__1::allocator<char> >, 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_trai [...]
        #16 kudu::tools::DispatchCommand(std::__1::vector<kudu::tools::Mode*, std::__1::allocator<kudu::tools::Mode*>
> const&, kudu::tools::Action*, std::__1::deque<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> > > > const&)
/home/jenkins-slave/workspace/kudu-master/2/src/kudu/tools/tool_main.cc:132:15 (kudu+0x5b42b6)
        #17 kudu::tools::RunTool(int, char**, bool) /home/jenkins-slave/workspace/kudu-master/2/src/kudu/tools/tool_main.cc:204:16
(kudu+0x5b5211)
        #18 main /home/jenkins-slave/workspace/kudu-master/2/src/kudu/tools/tool_main.cc:265:10
(kudu+0x5b557e)
    
    Change-Id: I1fd93c06b14bc97a9ac4a37a5b6ca55ffa38f544
    Reviewed-on: http://gerrit.cloudera.org:8080/14250
    Tested-by: Kudu Jenkins
    Reviewed-by: Andrew Wong <awong@cloudera.com>
    Reviewed-by: Alexey Serbin <aserbin@cloudera.com>
---
 src/kudu/rpc/messenger.cc | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/src/kudu/rpc/messenger.cc b/src/kudu/rpc/messenger.cc
index 4129172..871192a 100644
--- a/src/kudu/rpc/messenger.cc
+++ b/src/kudu/rpc/messenger.cc
@@ -372,9 +372,7 @@ void Messenger::QueueOutboundCall(const shared_ptr<OutboundCall>
&call) {
 }
 
 void Messenger::QueueInboundCall(gscoped_ptr<InboundCall> call) {
-  shared_lock<rw_spinlock> guard(lock_.get_lock());
-  scoped_refptr<RpcService>* service = FindOrNull(rpc_services_,
-                                                  call->remote_method().service_name());
+  const auto service = rpc_service(call->remote_method().service_name());
   if (PREDICT_FALSE(!service)) {
     Status s =  Status::ServiceUnavailable(Substitute("service $0 not registered on $1",
                                                       call->remote_method().service_name(),
name_));
@@ -383,10 +381,10 @@ void Messenger::QueueInboundCall(gscoped_ptr<InboundCall> call)
{
     return;
   }
 
-  call->set_method_info((*service)->LookupMethod(call->remote_method()));
+  call->set_method_info(service->LookupMethod(call->remote_method()));
 
   // The RpcService will respond to the client on success or failure.
-  WARN_NOT_OK((*service)->QueueInboundCall(std::move(call)), "Unable to handle RPC call");
+  WARN_NOT_OK(service->QueueInboundCall(std::move(call)), "Unable to handle RPC call");
 }
 
 void Messenger::QueueCancellation(const shared_ptr<OutboundCall> &call) {
@@ -427,7 +425,6 @@ Messenger::Messenger(const MessengerBuilder &bld)
 }
 
 Messenger::~Messenger() {
-  std::lock_guard<percpu_rwlock> guard(lock_);
   CHECK(closing_) << "Should have already shut down";
   STLDeleteElements(&reactors_);
 }
@@ -451,7 +448,6 @@ Status Messenger::Init() {
 
 Status Messenger::DumpConnections(const DumpConnectionsRequestPB& req,
                                   DumpConnectionsResponsePB* resp) {
-  shared_lock<rw_spinlock> guard(lock_.get_lock());
   for (Reactor* reactor : reactors_) {
     RETURN_NOT_OK(reactor->DumpConnections(req, resp));
   }


Mime
View raw message