From 51c8ffff6db02ff8cdde25a46b9dc7ac66452006 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Thu, 21 Jan 2021 15:27:24 +0800 Subject: [PATCH 1/9] crimson/mon: print out entity addr type when non peer address matches Signed-off-by: Kefu Chai --- src/crimson/mon/MonClient.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/crimson/mon/MonClient.cc b/src/crimson/mon/MonClient.cc index 9dfbb103a38..7aa7b65f578 100644 --- a/src/crimson/mon/MonClient.cc +++ b/src/crimson/mon/MonClient.cc @@ -960,7 +960,8 @@ seastar::future<> Client::reopen_session(int rank) auto peer = monmap.get_addrs(rank).pick_addr(msgr.get_myaddr().get_type()); if (peer == entity_addr_t{}) { // crimson msgr only uses the first bound addr - logger().warn("mon.{} does not have an addr compatible with me", rank); + logger().warn("mon.{} does not have an addr compatible with my type: {}", + rank, msgr.get_myaddr().get_type()); return seastar::now(); } logger().info("connecting to mon.{}", rank); From 264e710a374f24bfa4d8891cf39b9ae5e6075ac6 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Thu, 21 Jan 2021 20:35:55 +0800 Subject: [PATCH 2/9] crimson/mon: fallback to msgr v2 for unbound msgr so, for instance, if we want to connect to monitor without bind to any address, we can try to use the v2 addresses advertised in monmap or local settings, instead of being unable to connect to mon because we are using an `entity_addr_t::TYPE_NONE` address which is returned by `entity_addrvec_t::front()` if the addrvec is empty. see also AsyncMessenger::should_use_msgr2(). Signed-off-by: Kefu Chai --- src/crimson/mon/MonClient.cc | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/crimson/mon/MonClient.cc b/src/crimson/mon/MonClient.cc index 7aa7b65f578..defa03f3422 100644 --- a/src/crimson/mon/MonClient.cc +++ b/src/crimson/mon/MonClient.cc @@ -957,7 +957,13 @@ seastar::future<> Client::reopen_session(int rank) pending_conns.reserve(mons.size()); return seastar::parallel_for_each(mons, [this](auto rank) { // TODO: connect to multiple addrs - auto peer = monmap.get_addrs(rank).pick_addr(msgr.get_myaddr().get_type()); + // connect to v2 addresses if we have not bound to any address, otherwise + // use the advertised msgr protocol + uint32_t type = msgr.get_myaddr().get_type(); + if (type == entity_addr_t::TYPE_NONE) { + type = entity_addr_t::TYPE_MSGR2; + } + auto peer = monmap.get_addrs(rank).pick_addr(type); if (peer == entity_addr_t{}) { // crimson msgr only uses the first bound addr logger().warn("mon.{} does not have an addr compatible with my type: {}", From 9a248da3feedc2b7e3de0c7c5c585e030568681c Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Thu, 21 Jan 2021 20:40:06 +0800 Subject: [PATCH 3/9] crimson/mon: add mon::Client::wait_for_config() just for waiting for monmap and config from mon. crimson-osd needs this for populating settings related to booting and transport layer before it starts. Signed-off-by: Kefu Chai --- src/crimson/mon/MonClient.cc | 13 ++++++++++++- src/crimson/mon/MonClient.h | 2 ++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/crimson/mon/MonClient.cc b/src/crimson/mon/MonClient.cc index defa03f3422..4d66fd9792d 100644 --- a/src/crimson/mon/MonClient.cc +++ b/src/crimson/mon/MonClient.cc @@ -894,7 +894,11 @@ seastar::future<> Client::handle_log_ack(Ref m) seastar::future<> Client::handle_config(Ref m) { - return crimson::common::local_conf().set_mon_vals(m->config); + return crimson::common::local_conf().set_mon_vals(m->config).then([this] { + if (config_updated) { + config_updated->set_value(); + } + }); } std::vector Client::get_random_mons(unsigned n) const @@ -1110,6 +1114,13 @@ seastar::future<> Client::renew_subs() }); } +seastar::future<> Client::wait_for_config() +{ + assert(!config_updated); + config_updated = seastar::promise<>(); + return config_updated->get_future(); +} + void Client::print(std::ostream& out) const { out << "mon." << entity_name; diff --git a/src/crimson/mon/MonClient.h b/src/crimson/mon/MonClient.h index e7d2df86393..7e13846039b 100644 --- a/src/crimson/mon/MonClient.h +++ b/src/crimson/mon/MonClient.h @@ -90,6 +90,7 @@ public: void sub_unwant(const std::string& what); bool sub_want_increment(const std::string& what, version_t start, unsigned flags); seastar::future<> renew_subs(); + seastar::future<> wait_for_config(); void print(std::ostream&) const; private: @@ -173,6 +174,7 @@ private: seastar::promise<> pr; }; std::deque pending_messages; + std::optional> config_updated; }; inline std::ostream& operator<<(std::ostream& out, const Client& client) { From c54bcf660f89a2d0a89e7df3bbe9a327a19dc39c Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Thu, 21 Jan 2021 20:44:59 +0800 Subject: [PATCH 4/9] crimson/net: move implementation of dtor into .cc file ceph_assert() is expanded into 5 lines of code. it'd help to speed up the compiling a little bit. Signed-off-by: Kefu Chai --- src/crimson/net/SocketMessenger.cc | 5 +++++ src/crimson/net/SocketMessenger.h | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/crimson/net/SocketMessenger.cc b/src/crimson/net/SocketMessenger.cc index db9421e79e2..0a86eb0736f 100644 --- a/src/crimson/net/SocketMessenger.cc +++ b/src/crimson/net/SocketMessenger.cc @@ -38,6 +38,11 @@ SocketMessenger::SocketMessenger(const entity_name_t& myname, nonce{nonce} {} +SocketMessenger::~SocketMessenger() +{ + ceph_assert(!listener); +} + seastar::future<> SocketMessenger::set_myaddrs(const entity_addrvec_t& addrs) { assert(seastar::this_shard_id() == master_sid); diff --git a/src/crimson/net/SocketMessenger.h b/src/crimson/net/SocketMessenger.h index 44c1d3c2137..9bd3cfc4f78 100644 --- a/src/crimson/net/SocketMessenger.h +++ b/src/crimson/net/SocketMessenger.h @@ -54,7 +54,7 @@ class SocketMessenger final : public Messenger { SocketMessenger(const entity_name_t& myname, const std::string& logic_name, uint32_t nonce); - ~SocketMessenger() override { ceph_assert(!listener); } + ~SocketMessenger() override; seastar::future<> set_myaddrs(const entity_addrvec_t& addr) override; From 84b2e0679400235fe3cedb039e71a6b46e34d9f0 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Thu, 21 Jan 2021 20:46:47 +0800 Subject: [PATCH 5/9] crimson/osd: fetch_config() before mkfs * fetch_config() before mkfs and starting osd for populating settings related to booting and transport layer before it starts. * set fsid read from monitor before mkfs it's crucial to mkfs if osd is supposed to retrieve the fsid from monitor. Signed-off-by: Kefu Chai --- src/crimson/osd/main.cc | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/crimson/osd/main.cc b/src/crimson/osd/main.cc index a90903e7227..edc712b2b21 100644 --- a/src/crimson/osd/main.cc +++ b/src/crimson/osd/main.cc @@ -16,6 +16,7 @@ #include "common/ceph_argparse.h" #include "crimson/common/buffer_io.h" #include "crimson/common/config_proxy.h" +#include "crimson/mon/MonClient.h" #include "crimson/net/Messenger.h" #include "global/pidfile.h" @@ -117,6 +118,43 @@ uint64_t get_nonce() } } +seastar::future<> fetch_config() +{ + // i don't have any client before joining the cluster, so no need to have + // a proper auth handler + class DummyAuthHandler : public crimson::common::AuthHandler { + public: + void handle_authentication(const EntityName& name, + const AuthCapsInfo& caps) + {} + }; + auto auth_handler = std::make_unique(); + auto msgr = crimson::net::Messenger::create(entity_name_t::CLIENT(), + "temp_mon_client", + get_nonce()); + auto monc = std::make_unique(*msgr, *auth_handler); + msgr->set_auth_client(monc.get()); + return msgr->start({monc.get()}).then([monc=monc.get()] { + return monc->start(); + }).then([monc=monc.get()] { + monc->sub_want("config", 0, 0); + return monc->renew_subs(); + }).then([monc=monc.get()] { + // wait for monmap and config + return monc->wait_for_config(); + }).then([monc=monc.get()] { + return local_conf().set_val("fsid", monc->get_fsid().to_string()); + }).then([monc=monc.get(), msgr=msgr.get()] { + msgr->stop(); + return monc->stop(); + }).then([msgr=msgr.get()] { + return msgr->shutdown(); + }).then([msgr=std::move(msgr), + auth_handler=std::move(auth_handler), + monc=std::move(monc)] + {}); +} + int main(int argc, char* argv[]) { seastar::app_template app; @@ -198,6 +236,7 @@ int main(int argc, char* argv[]) seastar::engine().exit(1); }).get(); } + fetch_config().get(); if (config.count("mkfs")) { osd.invoke_on( 0, From 71229d557d98d4420edd3643d8f5ebdab2f175d4 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Fri, 22 Jan 2021 10:31:10 +0800 Subject: [PATCH 6/9] crimson/mon: use switch case for checking return code also, since seastar supports returning plain value instead a ready future, let's return plain value. simpler this way. Signed-off-by: Kefu Chai --- src/crimson/mon/MonClient.cc | 28 +++++++++++++++------------- 1 file changed, 15 insertions(+), 13 deletions(-) diff --git a/src/crimson/mon/MonClient.cc b/src/crimson/mon/MonClient.cc index 4d66fd9792d..a76ce8f129f 100644 --- a/src/crimson/mon/MonClient.cc +++ b/src/crimson/mon/MonClient.cc @@ -254,8 +254,7 @@ Connection::do_auth_single(Connection::request_t what) if (!m) { ceph_assert(closed); logger().info("do_auth: connection closed"); - return seastar::make_ready_future>( - std::make_optional(auth_result_t::canceled)); + return std::make_optional(auth_result_t::canceled); } logger().info( "do_auth: mon {} => {} returns {}: {}", @@ -264,19 +263,22 @@ Connection::do_auth_single(Connection::request_t what) auto p = m->result_bl.cbegin(); auto ret = auth->handle_response(m->result, p, nullptr, nullptr); - if (ret != 0 && ret != -EAGAIN) { + std::optional auth_result; + switch (ret) { + case -EAGAIN: + auth_result = std::nullopt; + break; + case 0: + auth_result = auth_result_t::success; + break; + default: + auth_result = auth_result_t::failure; logger().error( - "do_auth: got error {} on mon {}", - ret, - conn->get_peer_addr()); + "do_auth: got error {} on mon {}", + ret, conn->get_peer_addr()); + break; } - return seastar::make_ready_future>( - ret == -EAGAIN - ? std::nullopt - : std::make_optional(ret == 0 - ? auth_result_t::success - : auth_result_t::failure - )); + return auth_result; }); } From 237cbc9ba5da4c2a947a363a4dd90d8c01795869 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Fri, 22 Jan 2021 10:33:56 +0800 Subject: [PATCH 7/9] crimson/os: do not use __func__ in lambda Signed-off-by: Kefu Chai --- src/crimson/os/cyanstore/cyan_store.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/crimson/os/cyanstore/cyan_store.cc b/src/crimson/os/cyanstore/cyan_store.cc index eb93d72ec58..e0be221ee59 100644 --- a/src/crimson/os/cyanstore/cyan_store.cc +++ b/src/crimson/os/cyanstore/cyan_store.cc @@ -95,7 +95,7 @@ seastar::future<> CyanStore::mkfs(uuid_d new_osd_fsid) } else if (r < 0) { throw std::runtime_error("read_meta"); } else { - logger().info("{} already has fsid {}", __func__, fsid_str); + logger().info("mkfs already has fsid {}", fsid_str); if (!osd_fsid.parse(fsid_str.c_str())) { throw std::runtime_error("failed to parse fsid"); } else if (osd_fsid != new_osd_fsid) { From 37dc3ea9c2d837963a1c1caba2286daec5b067fb Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Fri, 22 Jan 2021 10:35:00 +0800 Subject: [PATCH 8/9] mon/MonClient: do not include unused header MGetConfig.h is not used anywhere in this source file, so no need to include it. Signed-off-by: Kefu Chai --- src/mon/MonClient.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/mon/MonClient.cc b/src/mon/MonClient.cc index cc6a67ae604..a1c67ad4499 100644 --- a/src/mon/MonClient.cc +++ b/src/mon/MonClient.cc @@ -31,7 +31,6 @@ #include "messages/MMonGetVersionReply.h" #include "messages/MMonMap.h" #include "messages/MConfig.h" -#include "messages/MGetConfig.h" #include "messages/MAuth.h" #include "messages/MLogAck.h" #include "messages/MAuthReply.h" From 2d63bc0fbd08babdcf1b1ca9f6a19175b4e85265 Mon Sep 17 00:00:00 2001 From: Kefu Chai Date: Fri, 22 Jan 2021 12:41:04 +0800 Subject: [PATCH 9/9] vstart.sh: print out osd mkfs command for better understanding the progress of vstart, and also allows developer to repeat the command. Signed-off-by: Kefu Chai --- src/vstart.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vstart.sh b/src/vstart.sh index 21f26b851a1..29e50ed8b12 100755 --- a/src/vstart.sh +++ b/src/vstart.sh @@ -913,7 +913,7 @@ EOF echo "{\"cephx_secret\": \"$OSD_SECRET\"}" > $CEPH_DEV_DIR/osd$osd/new.json ceph_adm osd new $uuid -i $CEPH_DEV_DIR/osd$osd/new.json rm $CEPH_DEV_DIR/osd$osd/new.json - $SUDO $CEPH_BIN/$ceph_osd $extra_osd_args -i $osd $ARGS --mkfs --key $OSD_SECRET --osd-uuid $uuid $extra_seastar_args + prun $SUDO $CEPH_BIN/$ceph_osd $extra_osd_args -i $osd $ARGS --mkfs --key $OSD_SECRET --osd-uuid $uuid $extra_seastar_args local key_fn=$CEPH_DEV_DIR/osd$osd/keyring cat > $key_fn<