From: Patrick Donnelly <pdonnell@redhat.com>
Date: Sat, 3 Oct 2020 00:11:54 +0000 (-0700)
Subject: osdc: add timeout configs for mons/osds
X-Git-Tag: v16.1.0~814^2~5
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=a8a23747aa081d938c9b277ab42507dd506bf6c2;p=ceph.git

osdc: add timeout configs for mons/osds

Have the Objecter track the rados_(mon|osd)_op_timeout configs so that
it can be configured at runtime/startup. This is useful for the
MDS/ceph-fuse so that we can avoid waiting forever for a response from
the Monitors that will never come (statfs on a deleted file system's
pools).

Also: make these configs take a time value rather than double. This is
simpler to deal with in the code and allows time units to be used (e.g.
"5m" for 5 minutes).

Fixes: https://tracker.ceph.com/issues/47734
Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
---

diff --git a/src/client/Client.cc b/src/client/Client.cc
index 7fcc41933c33..90329d04949f 100644
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -15059,7 +15059,7 @@ mds_rank_t Client::_get_random_up_mds() const
 
 StandaloneClient::StandaloneClient(Messenger *m, MonClient *mc,
 				   boost::asio::io_context& ictx)
-  : Client(m, mc, new Objecter(m->cct, m, mc, ictx, 0, 0))
+  : Client(m, mc, new Objecter(m->cct, m, mc, ictx))
 {
   monclient->set_messenger(m);
   objecter->set_client_incarnation(0);
diff --git a/src/common/legacy_config_opts.h b/src/common/legacy_config_opts.h
index 32e6838f68f8..7e6c20de46e5 100644
--- a/src/common/legacy_config_opts.h
+++ b/src/common/legacy_config_opts.h
@@ -1230,8 +1230,6 @@ OPTION(journal_discard, OPT_BOOL) //using ssd disk as journal, whether support d
 
 OPTION(fio_dir, OPT_STR) // fio data directory for fio-objectstore
 
-OPTION(rados_mon_op_timeout, OPT_DOUBLE) // how many seconds to wait for a response from the monitor before returning an error from a rados operation. 0 means no limit.
-OPTION(rados_osd_op_timeout, OPT_DOUBLE) // how many seconds to wait for a response from osds before returning an error from a rados operation. 0 means no limit.
 OPTION(rados_tracing, OPT_BOOL) // true if LTTng-UST tracepoints should be enabled
 
 
diff --git a/src/common/options.cc b/src/common/options.cc
index ee79029e9730..1cc5a4fc74e4 100644
--- a/src/common/options.cc
+++ b/src/common/options.cc
@@ -5249,13 +5249,17 @@ std::vector<Option> get_global_options() {
     .set_default("/tmp/fio")
     .set_description(""),
 
-    Option("rados_mon_op_timeout", Option::TYPE_FLOAT, Option::LEVEL_ADVANCED)
+    Option("rados_mon_op_timeout", Option::TYPE_SECS, Option::LEVEL_ADVANCED)
     .set_default(0)
-    .set_description(""),
+    .set_description("timeout for operations handled by monitors such as statfs (0 is unlimited)")
+    .set_flag(Option::FLAG_RUNTIME)
+    .set_min(0),
 
-    Option("rados_osd_op_timeout", Option::TYPE_FLOAT, Option::LEVEL_ADVANCED)
+    Option("rados_osd_op_timeout", Option::TYPE_SECS, Option::LEVEL_ADVANCED)
     .set_default(0)
-    .set_description(""),
+    .set_description("timeout for operations handled by osds such as write (0 is unlimited)")
+    .set_flag(Option::FLAG_RUNTIME)
+    .set_min(0),
 
     Option("rados_tracing", Option::TYPE_BOOL, Option::LEVEL_ADVANCED)
     .set_default(false)
diff --git a/src/librados/RadosClient.cc b/src/librados/RadosClient.cc
index 49135f64045d..e2495da6ec98 100644
--- a/src/librados/RadosClient.cc
+++ b/src/librados/RadosClient.cc
@@ -247,10 +247,7 @@ int librados::RadosClient::connect()
 
   ldout(cct, 1) << "starting objecter" << dendl;
 
-  objecter = new (std::nothrow) Objecter(cct, messenger, &monclient,
-					 poolctx,
-					 cct->_conf->rados_mon_op_timeout,
-					 cct->_conf->rados_osd_op_timeout);
+  objecter = new (std::nothrow) Objecter(cct, messenger, &monclient, poolctx);
   if (!objecter)
     goto out;
   objecter->set_balanced_budget();
@@ -560,11 +557,7 @@ int librados::RadosClient::wait_for_osdmap()
   if (need_map) {
     std::unique_lock l(lock);
 
-    ceph::timespan timeout{0};
-    if (cct->_conf->rados_mon_op_timeout > 0) {
-      timeout = ceph::make_timespan(cct->_conf->rados_mon_op_timeout);
-    }
-
+    ceph::timespan timeout = rados_mon_op_timeout;
     if (objecter->with_osdmap(std::mem_fn(&OSDMap::get_epoch)) == 0) {
       ldout(cct, 10) << __func__ << " waiting" << dendl;
       while (objecter->with_osdmap(std::mem_fn(&OSDMap::get_epoch)) == 0) {
@@ -860,8 +853,8 @@ int librados::RadosClient::mgr_command(const vector<string>& cmd,
     return r;
 
   lock.unlock();
-  if (conf->rados_mon_op_timeout) {
-    r = cond.wait_for(conf->rados_mon_op_timeout);
+  if (rados_mon_op_timeout.count() > 0) {
+    r = cond.wait_for(rados_mon_op_timeout);
   } else {
     r = cond.wait();
   }
@@ -884,8 +877,8 @@ int librados::RadosClient::mgr_command(
     return r;
 
   lock.unlock();
-  if (conf->rados_mon_op_timeout) {
-    r = cond.wait_for(conf->rados_mon_op_timeout);
+  if (rados_mon_op_timeout.count() > 0) {
+    r = cond.wait_for(rados_mon_op_timeout);
   } else {
     r = cond.wait();
   }
@@ -1158,15 +1151,13 @@ int librados::RadosClient::get_inconsistent_pgs(int64_t pool_id,
   return 0;
 }
 
-namespace {
-const char *config_keys[] = {
-  "librados_thread_count",
-  NULL
-};
-}
-
 const char** librados::RadosClient::get_tracked_conf_keys() const
 {
+  static const char *config_keys[] = {
+    "librados_thread_count",
+    "rados_mon_op_timeout",
+    nullptr
+  };
   return config_keys;
 }
 
@@ -1177,4 +1168,7 @@ void librados::RadosClient::handle_conf_change(const ConfigProxy& conf,
     poolctx.stop();
     poolctx.start(conf.get_val<std::uint64_t>("librados_thread_count"));
   }
+  if (changed.count("rados_mon_op_timeout")) {
+    rados_mon_op_timeout = conf.get_val<std::chrono::seconds>("rados_mon_op_timeout");
+  }
 }
diff --git a/src/librados/RadosClient.h b/src/librados/RadosClient.h
index 237431927c1b..10d3baea13da 100644
--- a/src/librados/RadosClient.h
+++ b/src/librados/RadosClient.h
@@ -92,6 +92,7 @@ private:
   bool service_daemon = false;
   string daemon_name, service_name;
   map<string,string> daemon_metadata;
+  ceph::timespan rados_mon_op_timeout{};
 
   int wait_for_osdmap();
 
diff --git a/src/mds/MDSRank.cc b/src/mds/MDSRank.cc
index faec88ceb993..6735a05cffd9 100644
--- a/src/mds/MDSRank.cc
+++ b/src/mds/MDSRank.cc
@@ -490,7 +490,7 @@ MDSRank::MDSRank(
     boost::asio::io_context& ioc) :
     cct(msgr->cct), mds_lock(mds_lock_), clog(clog_),
     timer(timer_), mdsmap(mdsmap_),
-    objecter(new Objecter(g_ceph_context, msgr, monc_, ioc, 0, 0)),
+    objecter(new Objecter(g_ceph_context, msgr, monc_, ioc)),
     damage_table(whoami_), sessionmap(this),
     op_tracker(g_ceph_context, g_conf()->mds_enable_op_tracker,
                g_conf()->osd_num_op_tracker_shard),
diff --git a/src/mgr/MgrStandby.cc b/src/mgr/MgrStandby.cc
index 4c8334e41634..22e419a9eb2d 100644
--- a/src/mgr/MgrStandby.cc
+++ b/src/mgr/MgrStandby.cc
@@ -45,7 +45,7 @@ MgrStandby::MgrStandby(int argc, const char **argv) :
 		     entity_name_t::MGR(),
 		     "mgr",
 		     Messenger::get_pid_nonce())),
-  objecter{g_ceph_context, client_messenger.get(), &monc, poolctx, 0, 0},
+  objecter{g_ceph_context, client_messenger.get(), &monc, poolctx},
   client{client_messenger.get(), &monc, &objecter},
   mgrc(g_ceph_context, client_messenger.get(), &monc.monmap),
   log_client(g_ceph_context, client_messenger.get(), &monc.monmap, LogClient::NO_FLAGS),
diff --git a/src/mon/MonClient.h b/src/mon/MonClient.h
index c7f245feec4f..cc7805d0675e 100644
--- a/src/mon/MonClient.h
+++ b/src/mon/MonClient.h
@@ -567,9 +567,10 @@ private:
 
     MonCommand(MonClient& monc, uint64_t t, std::unique_ptr<CommandCompletion> onfinish)
       : tid(t), onfinish(std::move(onfinish)) {
-      auto timeout = ceph::maybe_timespan(monc.cct->_conf->rados_mon_op_timeout);
-      if (timeout) {
-	cancel_timer.emplace(monc.service, *timeout);
+      auto timeout =
+          monc.cct->_conf.get_val<std::chrono::seconds>("rados_mon_op_timeout");
+      if (timeout.count() > 0) {
+	cancel_timer.emplace(monc.service, timeout);
 	cancel_timer->async_wait(
           [this, &monc](boost::system::error_code ec) {
 	    if (ec)
diff --git a/src/neorados/RADOSImpl.cc b/src/neorados/RADOSImpl.cc
index 68b87da430c7..bddd62c2f8ba 100644
--- a/src/neorados/RADOSImpl.cc
+++ b/src/neorados/RADOSImpl.cc
@@ -45,10 +45,7 @@ RADOS::RADOS(boost::asio::io_context& ioctx,
   messenger->set_default_policy(
     Messenger::Policy::lossy_client(CEPH_FEATURE_OSDREPLYMUX));
 
-  objecter.reset(new Objecter(cct.get(), messenger.get(), &monclient,
-			      ioctx,
-			      cct->_conf->rados_mon_op_timeout,
-			      cct->_conf->rados_osd_op_timeout));
+  objecter = std::make_unique<Objecter>(cct.get(), messenger.get(), &monclient, ioctx);
 
   objecter->set_balanced_budget();
   monclient.set_messenger(messenger.get());
diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index 7c2ecc83f37e..82cd5a9f3e12 100644
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -277,7 +277,7 @@ OSDService::OSDService(OSD *osd, ceph::async::io_context_pool& poolctx) :
   poolctx(poolctx),
   objecter(make_unique<Objecter>(osd->client_messenger->cct,
 				 osd->objecter_messenger,
-				 osd->monc, poolctx, 0, 0)),
+				 osd->monc, poolctx)),
   m_objecter_finishers(cct->_conf->osd_objecter_finishers),
   watch_timer(osd->client_messenger->cct, watch_lock),
   next_notif_id(0),
diff --git a/src/osdc/Objecter.cc b/src/osdc/Objecter.cc
index 5fcd7c4f9985..b949c0ca5409 100644
--- a/src/osdc/Objecter.cc
+++ b/src/osdc/Objecter.cc
@@ -185,11 +185,6 @@ inline bs::error_code osdcode(int r) {
 
 // config obs ----------------------------
 
-static const char *config_keys[] = {
-  "crush_location",
-  NULL
-};
-
 class Objecter::RequestStateHook : public AdminSocketHook {
   Objecter *m_objecter;
 public:
@@ -214,6 +209,12 @@ std::unique_lock<std::mutex> Objecter::OSDSession::get_lock(object_t& oid)
 
 const char** Objecter::get_tracked_conf_keys() const
 {
+  static const char *config_keys[] = {
+    "crush_location",
+    "rados_mon_op_timeout",
+    "rados_osd_op_timeout",
+    NULL
+  };
   return config_keys;
 }
 
@@ -224,6 +225,12 @@ void Objecter::handle_conf_change(const ConfigProxy& conf,
   if (changed.count("crush_location")) {
     update_crush_location();
   }
+  if (changed.count("rados_mon_op_timeout")) {
+    mon_timeout = conf.get_val<std::chrono::seconds>("rados_mon_op_timeout");
+  }
+  if (changed.count("rados_osd_op_timeout")) {
+    osd_timeout = conf.get_val<std::chrono::seconds>("rados_osd_op_timeout");
+  }
 }
 
 void Objecter::update_crush_location()
@@ -4903,13 +4910,12 @@ Objecter::OSDSession::~OSDSession()
 
 Objecter::Objecter(CephContext *cct,
 		   Messenger *m, MonClient *mc,
-		   boost::asio::io_context& service,
-		   double mon_timeout,
-		   double osd_timeout) :
-  Dispatcher(cct), messenger(m), monc(mc), service(service),
-  mon_timeout(ceph::make_timespan(mon_timeout)),
-  osd_timeout(ceph::make_timespan(osd_timeout))
-{}
+		   boost::asio::io_context& service) :
+  Dispatcher(cct), messenger(m), monc(mc), service(service)
+{
+  mon_timeout = cct->_conf.get_val<std::chrono::seconds>("rados_mon_op_timeout");
+  osd_timeout = cct->_conf.get_val<std::chrono::seconds>("rados_osd_op_timeout");
+}
 
 Objecter::~Objecter()
 {
diff --git a/src/osdc/Objecter.h b/src/osdc/Objecter.h
index 96791d0eb75e..e76bb68ad80c 100644
--- a/src/osdc/Objecter.h
+++ b/src/osdc/Objecter.h
@@ -2575,9 +2575,7 @@ private:
 			     cct->_conf->objecter_inflight_ops)};
  public:
   Objecter(CephContext *cct, Messenger *m, MonClient *mc,
-	   boost::asio::io_context& service,
-	   double mon_timeout,
-	   double osd_timeout);
+	   boost::asio::io_context& service);
   ~Objecter() override;
 
   void init();
diff --git a/src/test/librados/io.cc b/src/test/librados/io.cc
index 6683bd5a7226..9e801dcd4f4c 100644
--- a/src/test/librados/io.cc
+++ b/src/test/librados/io.cc
@@ -46,7 +46,8 @@ TEST_F(LibRadosIo, ReadTimeout) {
     ASSERT_EQ(0, rados_create(&cluster, "admin"));
     ASSERT_EQ(0, rados_conf_read_file(cluster, NULL));
     ASSERT_EQ(0, rados_conf_parse_env(cluster, NULL));
-    ASSERT_EQ(0, rados_conf_set(cluster, "rados_osd_op_timeout", "0.00001")); // use any small value that will result in a timeout
+    ASSERT_EQ(0, rados_conf_set(cluster, "rados_osd_op_timeout", "1")); // use any small value that will result in a timeout
+    ASSERT_EQ(0, rados_conf_set(cluster, "ms_inject_internal_delays", "2")); // create a 2 second delay
     ASSERT_EQ(0, rados_connect(cluster));
     ASSERT_EQ(0, rados_ioctx_create(cluster, pool_name.c_str(), &ioctx));
     rados_ioctx_set_namespace(ioctx, nspace.c_str());
diff --git a/src/test/mon/test_mon_workloadgen.cc b/src/test/mon/test_mon_workloadgen.cc
index 64f7bed5e97c..147ea8bcd04b 100644
--- a/src/test/mon/test_mon_workloadgen.cc
+++ b/src/test/mon/test_mon_workloadgen.cc
@@ -263,7 +263,7 @@ class ClientStub : public TestStub
     dout(10) << "ClientStub::" << __func__ << " starting messenger at "
 	    << messenger->get_myaddrs() << dendl;
 
-    objecter.reset(new Objecter(cct, messenger.get(), &monc, poolctx, 0, 0));
+    objecter.reset(new Objecter(cct, messenger.get(), &monc, poolctx));
     ceph_assert(objecter.get() != NULL);
     objecter->set_balanced_budget();
 
diff --git a/src/tools/cephfs/MDSUtility.cc b/src/tools/cephfs/MDSUtility.cc
index 13247a8ec549..54386d2199f9 100644
--- a/src/tools/cephfs/MDSUtility.cc
+++ b/src/tools/cephfs/MDSUtility.cc
@@ -28,7 +28,7 @@ MDSUtility::MDSUtility() :
   monc = new MonClient(g_ceph_context, poolctx);
   messenger = Messenger::create_client_messenger(g_ceph_context, "mds");
   fsmap = new FSMap();
-  objecter = new Objecter(g_ceph_context, messenger, monc, poolctx, 0, 0);
+  objecter = new Objecter(g_ceph_context, messenger, monc, poolctx);
 }