client: explicit stop LeaseExecutor in FileInstance::UnInitialize

When uninit FileClient and some files are not closed, in FileInstance's destructor it will destroy LeaseExecutor, but LeaseExecutor's destructor will wait for backend lease task to stop, so without explicit close a file, thread will be stuck here. Signed-off-by: wuhanqing <wuhanqing@hotmail.com>
opencurve · May 25, 2021 · 5edc463 · 5edc463
1 parent 7a6ed9e
commit 5edc463
Show file tree

Hide file tree

Showing 11 changed files with 208 additions and 63 deletions.
diff --git a/src/client/copyset_client.cpp b/src/client/copyset_client.cpp
@@ -54,7 +54,7 @@ int CopysetClient::Init(MetaCache *metaCache,
     iosenderopt_ = ioSenderOpt;
 
     LOG(INFO) << "CopysetClient init success, conf info: "
-              << ", chunkserverOPRetryIntervalUS = "
+                 "chunkserverOPRetryIntervalUS = "
               << iosenderopt_.failRequestOpt.chunkserverOPRetryIntervalUS
               << ", chunkserverOPMaxRetry = "
               << iosenderopt_.failRequestOpt.chunkserverOPMaxRetry

diff --git a/src/client/file_instance.cpp b/src/client/file_instance.cpp
@@ -26,13 +26,8 @@
 #include <glog/logging.h>
 #include <utility>
 
-#include "proto/nameserver2.pb.h"
-#include "proto/topology.pb.h"
 #include "src/client/iomanager4file.h"
 #include "src/client/mds_client.h"
-#include "src/client/metacache.h"
-#include "src/client/request_scheduler.h"
-#include "src/client/request_sender_manager.h"
 #include "src/common/timeutility.h"
 
 namespace curve {
@@ -99,6 +94,8 @@ bool FileInstance::Initialize(const std::string& filename,
 }
 
 void FileInstance::UnInitialize() {
+    StopLease();
+
     iomanager4file_.UnInitialize();
 
     // release the ownership of mdsclient
@@ -191,10 +188,7 @@ int FileInstance::Close() {
         return 0;
     }
 
-    if (leaseExecutor_ != nullptr) {
-        leaseExecutor_->Stop();
-        leaseExecutor_.reset();
-    }
+    StopLease();
 
     LIBCURVE_ERROR ret =
         mdsclient_->CloseFile(finfo_.fullPathName, finfo_.userinfo, "");
@@ -254,5 +248,12 @@ FileInstance* FileInstance::Open4Readonly(const FileServiceOption& opt,
     return instance;
 }
 
+void FileInstance::StopLease() {
+    if (leaseExecutor_) {
+        leaseExecutor_->Stop();
+        leaseExecutor_.reset();
+    }
+}
+
 }   // namespace client
 }   // namespace curve
diff --git a/src/client/file_instance.h b/src/client/file_instance.h
@@ -162,6 +162,9 @@ class CURVE_CACHELINE_ALIGNMENT FileInstance {
                                        const std::string& filename,
                                        const UserInfo& userInfo);
 
+ private:
+    void StopLease();
+
  private:
     // 保存当前file的文件信息
     FInfo_t                 finfo_;

diff --git a/src/client/iomanager4file.cpp b/src/client/iomanager4file.cpp
@@ -87,7 +87,7 @@ bool IOManager4File::Initialize(const std::string& filename,
     discardTaskManager_.reset(
         new DiscardTaskManager(&(fileMetric_->discardMetric)));
 
-    LOG(INFO) << "iomanager init success! conf info: "
+    LOG(INFO) << "iomanager init success, conf info: "
               << "isolationTaskThreadPoolSize = "
               << ioopt_.taskThreadOpt.isolationTaskThreadPoolSize
               << ", isolationTaskQueueCapacity = "

diff --git a/src/client/lease_executor.cpp b/src/client/lease_executor.cpp
@@ -30,20 +30,21 @@ using curve::common::TimeUtility;
 namespace curve {
 namespace client {
 LeaseExecutor::LeaseExecutor(const LeaseOption& leaseOpt,
-                           UserInfo_t userinfo,
-                           MDSClient* mdsclient,
-                           IOManager4File* iomanager):
-                           isleaseAvaliable_(true),
-                           failedrefreshcount_(0) {
-    userinfo_    = userinfo;
-    mdsclient_   = mdsclient;
-    iomanager_   = iomanager;
-    leaseoption_ = leaseOpt;
-    task_ = nullptr;
-}
+                             const UserInfo& userinfo, MDSClient* mdsclient,
+                             IOManager4File* iomanager)
+    : fullFileName_(),
+      mdsclient_(mdsclient),
+      userinfo_(userinfo),
+      iomanager_(iomanager),
+      leaseoption_(leaseOpt),
+      leasesession_(),
+      isleaseAvaliable_(true),
+      failedrefreshcount_(0),
+      task_() {}
 
 LeaseExecutor::~LeaseExecutor() {
     if (task_) {
+        task_->Stop();
         task_->WaitTaskExit();
     }
 }
@@ -78,6 +79,9 @@ bool LeaseExecutor::Start(const FInfo_t& fi, const LeaseSession_t& lease) {
     timespec abstime = butil::microseconds_from_now(interval);
     brpc::PeriodicTaskManager::StartTaskAt(task_.get(), abstime);
 
+    LOG(INFO) << "LeaseExecutor for " << fullFileName_
+              << " started, lease interval is " << interval << " us";
+
     return true;
 }
 
@@ -133,13 +137,11 @@ bool LeaseExecutor::RefreshLease() {
     return true;
 }
 
-std::string LeaseExecutor::GetLeaseSessionID() {
-    return leasesession_.sessionID;
-}
-
 void LeaseExecutor::Stop() {
     if (task_ != nullptr) {
         task_->Stop();
+
+        LOG(INFO) << "LeaseExecutor for " << fullFileName_ << " stopped";
     }
 }
 

diff --git a/src/client/lease_executor.h b/src/client/lease_executor.h
@@ -70,7 +70,7 @@ class LeaseExecutor {
      * @param: mdsclient是与mds续约的client
      * @param: iomanager会在续约失败或者版本变更的时候进行io调度
      */
-    LeaseExecutor(const LeaseOption& leaseOpt, UserInfo_t userinfo,
+    LeaseExecutor(const LeaseOption& leaseOpt, const UserInfo& userinfo,
                   MDSClient* mdscllent, IOManager4File* iomanager);
 
     ~LeaseExecutor();
@@ -84,11 +84,6 @@ class LeaseExecutor {
      */
     bool Start(const FInfo_t& fi, const LeaseSession_t&  lease);
 
-    /**
-     * 获取当前lease的sessionid信息，外围close文件的时候需要用到
-     */
-    std::string GetLeaseSessionID();
-
     /**
      * 停止续约
      */

diff --git a/src/client/metacache.cpp b/src/client/metacache.cpp
@@ -45,8 +45,7 @@ void MetaCache::Init(const MetaCacheOption& metaCacheOpt,
                      MDSClient* mdsclient) {
     mdsclient_ = mdsclient;
     metacacheopt_ = metaCacheOpt;
-    LOG(INFO) << "metacache init success!"
-              << ", get leader retry times = "
+    LOG(INFO) << "metacache init success, get leader retry times = "
               << metacacheopt_.metacacheGetLeaderRetry
               << ", get leader retry interval us = "
               << metacacheopt_.metacacheRPCRetryIntervalUS

diff --git a/test/client/BUILD b/test/client/BUILD
@@ -283,6 +283,7 @@ cc_test(
         "//src/client:curve_client",
         "@com_google_googletest//:gtest",
         "@com_google_googletest//:gtest_main",
+        "//test/client/mock:client_mock_lib",
     ],
 )
 

diff --git a/test/client/client.conf b/test/client/client.conf
@@ -0,0 +1,41 @@
+chunkserver.checkHealthTimeoutMs=100
+chunkserver.enableAppliedIndexRead=1
+chunkserver.maxRPCTimeoutMS=8000
+chunkserver.maxRetrySleepIntervalUS=8000000
+chunkserver.maxRetryTimesBeforeConsiderSuspend=20
+chunkserver.maxStableTimeoutTimes=10
+chunkserver.minRetryTimesForceTimeoutBackoff=5
+chunkserver.opMaxRetry=3
+chunkserver.opRetryIntervalUS=100000
+chunkserver.rpcTimeoutMS=1000
+chunkserver.serverStableThreshold=3
+closefd.timeInterval=600
+closefd.timeout=300
+discard.enable=true
+discard.granularity=4096
+discard.taskDelayMs=60000
+global.fileIOSplitMaxSizeKB=64
+global.fileMaxInFlightRPCNum=2048
+global.logLevel=0
+global.logPath=./runlog/
+global.metricDummyServerStartPort=9000
+global.turnOffHealthCheck=true
+isolation.taskQueueCapacity=1000000
+isolation.taskThreadPoolSize=1
+mds.listen.addr=127.0.0.1:9104,127.0.0.1:9104
+mds.maxFailedTimesBeforeChangeMDS=2
+mds.maxRPCTimeoutMS=2000
+mds.maxRetryMS=8000
+mds.maxRetryMsInIOPath=86400000
+mds.normalRetryTimesBeforeTriggerWait=3
+mds.refreshTimesPerLease=4
+mds.registerToMDS=true
+mds.rpcRetryIntervalUS=500
+mds.rpcTimeoutMS=500
+mds.waitSleepMs=10000
+metacache.getLeaderRetry=3
+metacache.getLeaderTimeOutMS=1000
+metacache.rpcRetryIntervalUS=500
+schedule.queueCapacity=1000000
+schedule.threadpoolSize=2
+throttle.enable=true