From: Marcel Lauhoff <marcel.lauhoff@clyso.com>
Date: Tue, 26 Nov 2024 20:35:43 +0000 (+0100)
Subject: msg: messenger dump: Handle name collisions
X-Git-Tag: testing/wip-rishabh-testing-20250315.120910-debug~25^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=36f2ed56d4700b1d338158c2c1cf55f580e75766;p=ceph-ci.git

msg: messenger dump: Handle name collisions

Handle name collisions during the messenger dump registration by
appending the messenger nonce.

Always append it to "radosclient". Multiple librados connections are
common in applications. This avoids the first registered librados
messenger to look special.

Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com>
---

diff --git a/src/msg/async/AsyncMessenger.cc b/src/msg/async/AsyncMessenger.cc
index 82fcce93e9a..64868f93c01 100644
--- a/src/msg/async/AsyncMessenger.cc
+++ b/src/msg/async/AsyncMessenger.cc
@@ -337,9 +337,10 @@ int AsyncMessengerSocketHook::call(
   return -ENOSYS;
 }
 
-void AsyncMessengerSocketHook::add_messenger(
+bool AsyncMessengerSocketHook::add_messenger(
     const std::string& name, AsyncMessenger& msgr) {
-  m_msgrs.try_emplace(name, &msgr);
+  const auto result = m_msgrs.try_emplace(name, &msgr);
+  return result.second;
 }
 
 void AsyncMessengerSocketHook::remove_messenger(
@@ -392,25 +393,45 @@ AsyncMessenger::AsyncMessenger(CephContext *cct, entity_name_t name,
 
   cct->modify_msgr_hook(
       [&]() {
-	auto hook = new AsyncMessengerSocketHook(*this, mname);
-	const int asok_ret = cct->get_admin_socket()->register_command(
-	    AsyncMessengerSocketHook::COMMAND, hook,
-	    "dump messenger status");
-	if (asok_ret != 0) {
-	  ldout(cct, 0) << __func__ << " messenger asok command \""
-                             << AsyncMessengerSocketHook::COMMAND
-                             << "\" failed with" << asok_ret << dendl;
-	}
-	return hook;
+        auto hook = new AsyncMessengerSocketHook(*this, mname);
+        const int asok_ret = cct->get_admin_socket()->register_command(
+            AsyncMessengerSocketHook::COMMAND, hook, "dump messenger status");
+        if (asok_ret != 0) {
+          ldout(cct, 0) << __func__ << " messenger asok command \""
+                        << AsyncMessengerSocketHook::COMMAND << "\" failed with"
+                        << asok_ret << dendl;
+        }
+        return hook;
       },
       [&](AdminSocketHook* ptr) {
-	if (auto hook = dynamic_cast<AsyncMessengerSocketHook*>(ptr)) {
-	  hook->add_messenger(mname, *this);
-	} else {
-	  ceph_abort(
-	      "BUG: messenger hook obj set, but not of type "
-	      "AsyncMessengerSocketHook");
-	}
+        if (auto hook = dynamic_cast<AsyncMessengerSocketHook*>(ptr)) {
+          // Name collisions may occur. A common case is RGW running
+          // multiple librados connections - each with a "radosclient"
+          // messenger.
+          std::string msgr_key(mname);
+          if (mname == "radosclient") {  // librados
+            msgr_key.append("-");
+            msgr_key.append(std::to_string(_nonce));
+          }
+          bool added = hook->add_messenger(msgr_key, *this);
+          if (!added) {
+            msgr_key.append("-");
+            msgr_key.append(std::to_string(_nonce));
+          }
+          added = hook->add_messenger(msgr_key, *this);
+          if (!added) {
+            ldout(cct, 10)
+                << __func__ << " registering messenger " << mname
+                << " using key " << msgr_key
+                << " failed due to a name collision. "
+                   "messenger won't be available to \"messenger dump\""
+                << dendl;
+          }
+        } else {
+          ceph_abort(
+              "BUG: messenger hook obj set, but not of type "
+              "AsyncMessengerSocketHook");
+        }
       });
 }
 
diff --git a/src/msg/async/AsyncMessenger.h b/src/msg/async/AsyncMessenger.h
index 8304ac406ff..7003bf939f7 100644
--- a/src/msg/async/AsyncMessenger.h
+++ b/src/msg/async/AsyncMessenger.h
@@ -81,7 +81,7 @@ class AsyncMessengerSocketHook : public AdminSocketHook {
   int call(
       std::string_view command, const cmdmap_t& cmdmap, const bufferlist&,
       Formatter* f, std::ostream& errss, ceph::buffer::list& out) override;
-  void add_messenger(const std::string& name, AsyncMessenger& msgr);
+  bool add_messenger(const std::string& name, AsyncMessenger& msgr);
   void remove_messenger(AsyncMessenger& msgr);
   std::list<std::string> messengers() const;
 };