Alexey Serbin has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/22434 )

Change subject: KUDU-3633 shutdown DnsResolver in ServerBase::ShutdownImpl()
......................................................................

KUDU-3633 shutdown DnsResolver in ServerBase::ShutdownImpl()

The thread pool of the DNS resolver should be shut down along with the
messenger in ServerBase to prevent retrying of RPCs that failed as a
collateral of the shutdown process in progress.  Those RPCs might be
retried by invoking rpc::Proxy::RefreshDnsAndEnqueueRequest(), etc.

On the related note, I also added a guard to protect ThreadPool::tokens_
in the destructor of the ThreadPool class, as elsewhere.  I also snuck
in an update to call DCHECK() in a loop only when DCHECK_IS_ON()
macro evaluates to 'true'.

This addresses flakiness reported at least in one of the RemoteKsckTest
scenarios (e.g., TestFilterOnNotabletTable in [1]).  One of the related
TSAN reports looked like below:

RemoteKsckTest.TestFilterOnNotabletTable: WARNING: ThreadSanitizer: data race
  Read of size 8 at 0x7b54001e5118 by main thread:
    #0 std::__1::__hash_table<kudu::ThreadPoolToken*, ...>::size() const
    #1 std::__1::unordered_set<kudu::ThreadPoolToken*, ...>::size() const
    #2 kudu::ThreadPool::~ThreadPool()
    ...
    #6 kudu::kserver::KuduServer::~KuduServer()
    #7 kudu::tserver::TabletServer::~TabletServer()
    ...

  Previous write of size 8 at 0x7b54001e5118 by thread T262 ...:
    #0 std::__1::__hash_table<kudu::ThreadPoolToken*, ...>::remove(...)
    ...
    #4 kudu::ThreadPool::ReleaseToken(...)
    #5 kudu::ThreadPoolToken::~ThreadPoolToken()
    ...
    #24 kudu::consensus::LeaderElection::~LeaderElection()
    ...
    #35 kudu::rpc::Proxy::RefreshDnsAndEnqueueRequest(...)
    ...
    #41 kudu::DnsResolver::RefreshAddressesAsync()
    ...

  Thread T262 'dns-resolver [w' (tid=29102, running) created by thread T182 at:
    #0 pthread_create
    #1 kudu::Thread::StartThread(...)
    #2 kudu::Thread::Create(...)
    #3 kudu::ThreadPool::CreateThread()
    #4 kudu::ThreadPool::DoSubmit(..., kudu::ThreadPoolToken*)
    #5 kudu::ThreadPool::Submit(...)
    #6 kudu::DnsResolver::RefreshAddressesAsync(..)
    #7 kudu::rpc::Proxy::RefreshDnsAndEnqueueRequest(...)
    #8 kudu::rpc::Proxy::AsyncRequest(...)
    ...
    #15 kudu::rpc::OutboundCall::CallCallback()
    #16 kudu::rpc::OutboundCall::SetFailed()
    #17 kudu::rpc::Connection::Shutdown()
    #18 kudu::rpc::ReactorThread::ShutdownInternal()
    ...
    #25 kudu::rpc::ReactorThread::RunThread()
    ...

[1] http://dist-test.cloudera.org:8080/test_drilldown?test_name=ksck_remote-test

Change-Id: I525f1078a349dbd2926938bb4fcc3e80888dfbb4
Reviewed-on: http://gerrit.cloudera.org:8080/22434
Tested-by: Alexey Serbin <[email protected]>
Reviewed-by: Abhishek Chennaka <[email protected]>
---
M src/kudu/server/server_base.cc
M src/kudu/util/net/dns_resolver.cc
M src/kudu/util/net/dns_resolver.h
M src/kudu/util/threadpool.cc
4 files changed, 26 insertions(+), 6 deletions(-)

Approvals:
  Alexey Serbin: Verified
  Abhishek Chennaka: Looks good to me, approved

--
To view, visit http://gerrit.cloudera.org:8080/22434
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I525f1078a349dbd2926938bb4fcc3e80888dfbb4
Gerrit-Change-Number: 22434
Gerrit-PatchSet: 3
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <[email protected]>
Gerrit-Reviewer: Ádám Bakai <[email protected]>

Reply via email to