From 36f2ed56d4700b1d338158c2c1cf55f580e75766 Mon Sep 17 00:00:00 2001
From: Marcel Lauhoff <marcel.lauhoff@clyso.com>
Date: Tue, 26 Nov 2024 21:35:43 +0100
Subject: [PATCH] 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>
---
 src/msg/async/AsyncMessenger.cc | 59 ++++++++++++++++++++++-----------
 src/msg/async/AsyncMessenger.h  |  2 +-
 2 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/src/msg/async/AsyncMessenger.cc b/src/msg/async/AsyncMessenger.cc
index 82fcce93e9a5..64868f93c013 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 8304ac406ff5..7003bf939f73 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;
 };
-- 
2.47.3