[kudu] 01/02: [master] fix race in MiniMaster-based scenarios

[al...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

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

commit e730ebd7ee0577e43098af697d2bace4c7384da3
Author: Alexey Serbin <al...@apache.org>
AuthorDate: Tue Jan 28 20:59:25 2020 -0800

    [master] fix race in MiniMaster-based scenarios
    
    I saw a warning from the ThreadSanitizer while running the
    TsTabletManagerITest.TestTableStats scenario (TSAN build).  It turned
    to be a test-only race in scenarios where MiniMaster is involved in
    single-master test mini-cluster if calling MiniMaster::Shutdown()
    after MiniMaster::Start() but before the catalog manager has completed
    its initialization.
    
    The relevant snippet from the warning:
    
    WARNING: ThreadSanitizer: data race (pid=6972)
      Write of size 8 at 0x7b7000000c70 by main thread:
        #0 pthread_cond_destroy
        #1 ConditionVariable::~ConditionVariable()
        #2 CountDownLatch::~CountDownLatch()
        #3 Promise<Status>::~Promise()
        #4 Master::~Master() src/kudu/master/master.cc:129
        #5 Master::~Master() src/kudu/master/master.cc:127
        #6 std::__1::default_delete<Master>::operator(Master*)
        #7 std::__1::unique_ptr<Master, std::__1::default_delete<Master>>::reset(Master*)
        #8 MiniMaster::Shutdown() src/kudu/master/mini_master.cc:118
    ...
    
      Previous read of size 8 at 0x7b7000000c70 by thread T20 (mutexes: write M50614):
        #0 pthread_cond_broadcast
        #1 ConditionVariable::Broadcast()
        #2 CountDownLatch::CountDown(int)
        #3 CountDownLatch::CountDown()
        #4 Promise<Status>::Set(Status const&)
        #5 Master::InitCatalogManagerTask() src/kudu/master/master.cc:206
    ...
    
    Change-Id: If5122a16bb04f089fe1ec4a0d0ff157164ebfdc4
    Reviewed-on: http://gerrit.cloudera.org:8080/15125
    Tested-by: Kudu Jenkins
    Reviewed-by: Adar Dembo <ad...@cloudera.com>
---
 src/kudu/master/master.cc | 1 +
 src/kudu/master/master.h  | 6 +++---
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/kudu/master/master.cc b/src/kudu/master/master.cc
index 6594c96..650b4fc 100644
--- a/src/kudu/master/master.cc
+++ b/src/kudu/master/master.cc
@@ -249,6 +249,7 @@ void Master::Shutdown() {
     UnregisterAllServices();
 
     // 2. Shut down the master's subsystems.
+    init_pool_->Shutdown();
     maintenance_manager_->Shutdown();
     catalog_manager_->Shutdown();
     fs_manager_->UnsetErrorNotificationCb(ErrorHandlerType::DISK_ERROR);
diff --git a/src/kudu/master/master.h b/src/kudu/master/master.h
index 783c39f..4f7ce66 100644
--- a/src/kudu/master/master.h
+++ b/src/kudu/master/master.h
@@ -142,13 +142,13 @@ class Master : public kserver::KuduServer {
   std::unique_ptr<CatalogManager> catalog_manager_;
   std::unique_ptr<MasterPathHandlers> path_handlers_;
 
-  // For initializing the catalog manager.
-  std::unique_ptr<ThreadPool> init_pool_;
-
   // The status of the master initialization. This is set
   // by the async initialization task.
   Promise<Status> init_status_;
 
+  // For initializing the catalog manager.
+  std::unique_ptr<ThreadPool> init_pool_;
+
   MasterOptions opts_;
 
   ServerRegistrationPB registration_;