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: v15.2.8~5^2~1^2~21^2~6
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=c25acb50eaa0ec746de529734ebc2c1761e78d73;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>
(cherry picked from commit a8a23747aa081d938c9b277ab42507dd506bf6c2)

Conflicts:
	src/client/Client.cc
	src/librados/RadosClient.cc
	src/mds/MDSRank.cc
	src/mgr/MgrStandby.cc
	src/mon/MonClient.h
	src/neorados/RADOSImpl.cc
	src/osd/OSD.cc
	src/osdc/Objecter.cc
	src/osdc/Objecter.h
	src/test/mon/test_mon_workloadgen.cc
	src/tools/cephfs/MDSUtility.cc

        Notes: different Objecter cons arguments. Added conf obs for
        RadosClient.
---

diff --git a/src/client/Client.cc b/src/client/Client.cc
index 57295f19aba..1485dd3d59d 100755
--- a/src/client/Client.cc
+++ b/src/client/Client.cc
@@ -14744,7 +14744,7 @@ mds_rank_t Client::_get_random_up_mds() const
 
 
 StandaloneClient::StandaloneClient(Messenger *m, MonClient *mc)
-    : Client(m, mc, new Objecter(m->cct, m, mc, NULL, 0, 0))
+    : Client(m, mc, new Objecter(m->cct, m, mc, nullptr))
 {
   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 4b2422f6443..e59913b37bf 100644
--- a/src/common/legacy_config_opts.h
+++ b/src/common/legacy_config_opts.h
@@ -1239,8 +1239,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 baba92debd1..e79be858e95 100644
--- a/src/common/options.cc
+++ b/src/common/options.cc
@@ -5145,13 +5145,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 4de30e1e7df..3e591d63513 100644
--- a/src/librados/RadosClient.cc
+++ b/src/librados/RadosClient.cc
@@ -71,6 +71,7 @@ librados::RadosClient::RadosClient(CephContext *cct_)
     log_last_version(0), log_cb(NULL), log_cb2(NULL), log_cb_arg(NULL),
     finisher(cct, "radosclient", "fn-radosclient")
 {
+  conf.add_observer(this);
 }
 
 int64_t librados::RadosClient::lookup_pool(const char *name)
@@ -260,10 +261,7 @@ int librados::RadosClient::connect()
 
   ldout(cct, 1) << "starting objecter" << dendl;
 
-  objecter = new (std::nothrow) Objecter(cct, messenger, &monclient,
-			  &finisher,
-			  cct->_conf->rados_mon_op_timeout,
-			  cct->_conf->rados_osd_op_timeout);
+  objecter = new (std::nothrow) Objecter(cct, messenger, &monclient, &finisher);
   if (!objecter)
     goto out;
   objecter->set_balanced_budget();
@@ -469,6 +467,7 @@ int librados::RadosClient::get_min_compatible_client(int8_t* min_compat_client,
 
 librados::RadosClient::~RadosClient()
 {
+  conf.remove_observer(this);
   if (messenger)
     delete messenger;
   if (objecter)
@@ -574,11 +573,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) {
@@ -864,8 +859,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();
   }
@@ -888,8 +883,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();
   }
@@ -1175,3 +1170,20 @@ int librados::RadosClient::get_inconsistent_pgs(int64_t pool_id,
   }
   return 0;
 }
+
+const char** librados::RadosClient::get_tracked_conf_keys() const
+{
+  static const char *config_keys[] = {
+    "rados_mon_op_timeout",
+    nullptr
+  };
+  return config_keys;
+}
+
+void librados::RadosClient::handle_conf_change(const ConfigProxy& conf,
+					       const std::set<std::string> &changed)
+{
+  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 a3b8d20dfb4..4425018c3c4 100644
--- a/src/librados/RadosClient.h
+++ b/src/librados/RadosClient.h
@@ -19,6 +19,7 @@
 #include "common/Timer.h"
 #include "common/ceph_mutex.h"
 #include "common/ceph_time.h"
+#include "common/config_obs.h"
 #include "include/common_fwd.h"
 #include "include/rados/librados.h"
 #include "include/rados/librados.hpp"
@@ -36,14 +37,14 @@ class MLog;
 class Messenger;
 class AioCompletionImpl;
 
-class librados::RadosClient : public Dispatcher
+class librados::RadosClient : public Dispatcher, public md_config_obs_t
 {
   std::unique_ptr<CephContext,
 		  std::function<void(CephContext*)> > cct_deleter;
 
 public:
   using Dispatcher::cct;
-  const ConfigProxy& conf;
+  ConfigProxy& conf{cct->_conf};
 private:
   enum {
     DISCONNECTED,
@@ -81,6 +82,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();
 
@@ -179,6 +181,9 @@ public:
   mon_feature_t get_required_monitor_features() const;
 
   int get_inconsistent_pgs(int64_t pool_id, std::vector<std::string>* pgs);
+
+  const char** get_tracked_conf_keys() const override;
+  void handle_conf_change(const ConfigProxy&, const std::set<std::string>&) override;
 };
 
 #endif
diff --git a/src/mds/MDSRank.cc b/src/mds/MDSRank.cc
index e7005534065..5f7741de3aa 100644
--- a/src/mds/MDSRank.cc
+++ b/src/mds/MDSRank.cc
@@ -485,7 +485,7 @@ MDSRank::MDSRank(
     Context *suicide_hook_) :
     cct(msgr->cct), mds_lock(mds_lock_), clog(clog_),
     timer(timer_), mdsmap(mdsmap_),
-    objecter(new Objecter(g_ceph_context, msgr, monc_, nullptr, 0, 0)),
+    objecter(new Objecter(g_ceph_context, msgr, monc_, nullptr)),
     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 296494ece14..770a75d4dc2 100644
--- a/src/mgr/MgrStandby.cc
+++ b/src/mgr/MgrStandby.cc
@@ -46,7 +46,7 @@ MgrStandby::MgrStandby(int argc, const char **argv) :
 		     "mgr",
 		     Messenger::get_pid_nonce(),
 		     0)),
-  objecter{g_ceph_context, client_messenger.get(), &monc, NULL, 0, 0},
+  objecter{g_ceph_context, client_messenger.get(), &monc, NULL},
   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.cc b/src/mon/MonClient.cc
index ccfa5dafd2a..2320b4a3f04 100644
--- a/src/mon/MonClient.cc
+++ b/src/mon/MonClient.cc
@@ -1323,7 +1323,8 @@ void MonClient::start_mon_command(const std::vector<string>& cmd,
   r->poutbl = outbl;
   r->prs = outs;
   r->onfinish = onfinish;
-  if (cct->_conf->rados_mon_op_timeout > 0) {
+  auto timeout = cct->_conf.get_val<std::chrono::seconds>("rados_mon_op_timeout");
+  if (timeout.count() > 0) {
     class C_CancelMonCommand : public Context
     {
       uint64_t tid;
@@ -1335,7 +1336,7 @@ void MonClient::start_mon_command(const std::vector<string>& cmd,
       }
     };
     r->ontimeout = new C_CancelMonCommand(r->tid, this);
-    timer.add_event_after(cct->_conf->rados_mon_op_timeout, r->ontimeout);
+    timer.add_event_after(static_cast<double>(timeout.count()), r->ontimeout);
   }
   mon_commands[r->tid] = r;
   _send_command(r);
diff --git a/src/neorados/RADOSImpl.cc b/src/neorados/RADOSImpl.cc
new file mode 100644
index 00000000000..bddd62c2f8b
--- /dev/null
+++ b/src/neorados/RADOSImpl.cc
@@ -0,0 +1,121 @@
+// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*-
+// vim: ts=8 sw=2 smarttab
+/*
+ * Ceph - scalable distributed file system
+ *
+ * Copyright (C) 2004-2012 Sage Weil <sage@newdream.net>
+ *
+ * This is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License version 2.1, as published by the Free Software
+ * Foundation.  See file COPYING.
+ *
+ */
+
+
+#include <boost/system/system_error.hpp>
+
+#include "common/common_init.h"
+
+#include "global/global_init.h"
+
+#include "RADOSImpl.h"
+
+namespace neorados {
+namespace detail {
+
+RADOS::RADOS(boost::asio::io_context& ioctx,
+	     boost::intrusive_ptr<CephContext> cct)
+  : Dispatcher(cct.get()),
+    ioctx(ioctx),
+    cct(cct),
+    monclient(cct.get(), ioctx),
+    mgrclient(cct.get(), nullptr, &monclient.monmap) {
+  auto err = monclient.build_initial_monmap();
+  if (err < 0)
+    throw std::system_error(ceph::to_error_code(err));
+
+  messenger.reset(Messenger::create_client_messenger(cct.get(), "radosclient"));
+  if (!messenger)
+    throw std::bad_alloc();
+
+  // Require OSDREPLYMUX feature. This means we will fail to talk to
+  // old servers. This is necessary because otherwise we won't know
+  // how to decompose the reply data into its constituent pieces.
+  messenger->set_default_policy(
+    Messenger::Policy::lossy_client(CEPH_FEATURE_OSDREPLYMUX));
+
+  objecter = std::make_unique<Objecter>(cct.get(), messenger.get(), &monclient, ioctx);
+
+  objecter->set_balanced_budget();
+  monclient.set_messenger(messenger.get());
+  mgrclient.set_messenger(messenger.get());
+  objecter->init();
+  messenger->add_dispatcher_head(&mgrclient);
+  messenger->add_dispatcher_tail(objecter.get());
+  messenger->start();
+  monclient.set_want_keys(CEPH_ENTITY_TYPE_MON | CEPH_ENTITY_TYPE_OSD | CEPH_ENTITY_TYPE_MGR);
+  err = monclient.init();
+  if (err) {
+    throw boost::system::system_error(ceph::to_error_code(err));
+  }
+  err = monclient.authenticate(cct->_conf->client_mount_timeout);
+  if (err) {
+    throw boost::system::system_error(ceph::to_error_code(err));
+  }
+  messenger->set_myname(entity_name_t::CLIENT(monclient.get_global_id()));
+  // Detect older cluster, put mgrclient into compatible mode
+  mgrclient.set_mgr_optional(
+      !get_required_monitor_features().contains_all(
+        ceph::features::mon::FEATURE_LUMINOUS));
+
+  // MgrClient needs this (it doesn't have MonClient reference itself)
+  monclient.sub_want("mgrmap", 0, 0);
+  monclient.renew_subs();
+
+  mgrclient.init();
+  objecter->set_client_incarnation(0);
+  objecter->start();
+
+  messenger->add_dispatcher_tail(this);
+
+  std::unique_lock l(lock);
+  instance_id = monclient.get_global_id();
+}
+
+RADOS::~RADOS() {
+  if (objecter && objecter->initialized) {
+    objecter->shutdown();
+  }
+
+  mgrclient.shutdown();
+  monclient.shutdown();
+
+  if (messenger) {
+    messenger->shutdown();
+    messenger->wait();
+  }
+}
+
+bool RADOS::ms_dispatch(Message *m)
+{
+  switch (m->get_type()) {
+  // OSD
+  case CEPH_MSG_OSD_MAP:
+    m->put();
+    return true;
+  }
+  return false;
+}
+
+void RADOS::ms_handle_connect(Connection *con) {}
+bool RADOS::ms_handle_reset(Connection *con) {
+  return false;
+}
+void RADOS::ms_handle_remote_reset(Connection *con) {}
+bool RADOS::ms_handle_refused(Connection *con) {
+  return false;
+}
+
+} // namespace detail
+} // namespace neorados
diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index 7b6e1aca215..34dd356ebf6 100644
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -250,7 +250,7 @@ OSDService::OSDService(OSD *osd) :
   promote_max_bytes(0),
   objecter(make_unique<Objecter>(osd->client_messenger->cct,
 				 osd->objecter_messenger,
-				 osd->monc, nullptr, 0, 0)),
+				 osd->monc, nullptr)),
   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 f16e4855093..ff8c1c48e16 100644
--- a/src/osdc/Objecter.cc
+++ b/src/osdc/Objecter.cc
@@ -167,11 +167,6 @@ enum {
 
 // config obs ----------------------------
 
-static const char *config_keys[] = {
-  "crush_location",
-  NULL
-};
-
 class Objecter::RequestStateHook : public AdminSocketHook {
   Objecter *m_objecter;
 public:
@@ -232,6 +227,12 @@ Objecter::OSDSession::unique_completion_lock Objecter::OSDSession::get_lock(
 
 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;
 }
 
@@ -242,6 +243,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()
@@ -4962,20 +4969,19 @@ Objecter::OSDSession::~OSDSession()
 }
 
 Objecter::Objecter(CephContext *cct_, Messenger *m, MonClient *mc,
-		   Finisher *fin,
-		   double mon_timeout,
-		   double osd_timeout) :
+		   Finisher *fin) :
   Dispatcher(cct_), messenger(m), monc(mc), finisher(fin),
   trace_endpoint("0.0.0.0", 0, "Objecter"),
   osdmap{std::make_unique<OSDMap>()},
   homeless_session(new OSDSession(cct, -1)),
-  mon_timeout(ceph::make_timespan(mon_timeout)),
-  osd_timeout(ceph::make_timespan(osd_timeout)),
   op_throttle_bytes(cct, "objecter_bytes",
 		    cct->_conf->objecter_inflight_op_bytes),
   op_throttle_ops(cct, "objecter_ops", cct->_conf->objecter_inflight_ops),
   retry_writes_after_first_reply(cct->_conf->objecter_retry_writes_after_first_reply)
-{}
+{
+  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 8db78558901..e676582788c 100644
--- a/src/osdc/Objecter.h
+++ b/src/osdc/Objecter.h
@@ -2119,9 +2119,7 @@ private:
 
  public:
   Objecter(CephContext *cct_, Messenger *m, MonClient *mc,
-	   Finisher *fin,
-	   double mon_timeout,
-	   double osd_timeout);
+	   Finisher *fin);
   ~Objecter() override;
 
   void init();
diff --git a/src/test/librados/io.cc b/src/test/librados/io.cc
index fce3de62122..bdef25a6ba0 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 15792a63af5..fcaff2de960 100644
--- a/src/test/mon/test_mon_workloadgen.cc
+++ b/src/test/mon/test_mon_workloadgen.cc
@@ -259,7 +259,7 @@ class ClientStub : public TestStub
     dout(10) << "ClientStub::" << __func__ << " starting messenger at "
 	    << messenger->get_myaddrs() << dendl;
 
-    objecter.reset(new Objecter(cct, messenger.get(), &monc, NULL, 0, 0));
+    objecter.reset(new Objecter(cct, messenger.get(), &monc, NULL));
     ceph_assert(objecter.get() != NULL);
     objecter->set_balanced_budget();
 
diff --git a/src/tools/cephfs/MDSUtility.cc b/src/tools/cephfs/MDSUtility.cc
index 839b4aea050..eb323a2308c 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);
   messenger = Messenger::create_client_messenger(g_ceph_context, "mds");
   fsmap = new FSMap();
-  objecter = new Objecter(g_ceph_context, messenger, monc, NULL, 0, 0);
+  objecter = new Objecter(g_ceph_context, messenger, monc, NULL);
 }