From ee8e66a5ef3fbe771e299844ababf4849ab41368 Mon Sep 17 00:00:00 2001 From: Nitzan Mordechai Date: Mon, 27 May 2024 08:17:58 +0000 Subject: [PATCH] monclient: try to resend the mon commands to the same monitor if available When we have a socket failure or connection issue, we may send a mon command and never check if it completed. If we resend the command to another monitor, the resent command may complete before the first sent command. This can cause users to send the command twice, which can lead to issues in automated environments. For example: We have 2 monitors: mon.a and mon.b 1. Send command to delete pool - monclient targets mon.a 2. A socket failure occurs, and mon.a has a delay in response 3. Monclient hunts for another monitor to resend the delete pool command and finds mon.b 4. Mon.b removes the pool and sends an acknowledgment 5. The user script now sends a create pool command, but mon.a now sends the acknowledgment for the pool delete from step #1 We end up without a pool, as mon.a deleted it. The mon_client_hunt_on_resent configuration was added to control the behavior of retrying commands on monitor connection failures. By default, this option is enabled to prevent situations where a command is retried on the same monitor, potentially missing better monitor candidates. Clients experiencing specific conditions that require retrying on the same monitor can disable this feature by setting the configuration to false. Fixes: https://tracker.ceph.com/issues/63789 Signed-off-by: Nitzan Mordechai --- src/common/options/global.yaml.in | 8 ++++++++ src/mon/MonClient.cc | 21 +++++++++++++++++++++ src/mon/MonClient.h | 1 + 3 files changed, 30 insertions(+) diff --git a/src/common/options/global.yaml.in b/src/common/options/global.yaml.in index 7366bbb31c645b..b0268156ef0918 100644 --- a/src/common/options/global.yaml.in +++ b/src/common/options/global.yaml.in @@ -2308,6 +2308,14 @@ options: level: advanced default: 10 with_legacy: true +- name: mon_client_hunt_on_resent + type: bool + level: advanced + default: true + fmt_desc: On commands connection failure, hunt for any monitor. + If false, try to resend the command to the same monitor to prevent + command race conditions. + with_legacy: true - name: mon_client_max_log_entries_per_message type: int level: advanced diff --git a/src/mon/MonClient.cc b/src/mon/MonClient.cc index 0667c078b630d7..4344822c9e1860 100644 --- a/src/mon/MonClient.cc +++ b/src/mon/MonClient.cc @@ -1249,6 +1249,7 @@ void MonClient::_send_command(MonCommand *r) } // normal CLI command + r->sent_name = monmap.get_name(active_con->get_con()->get_peer_addr()); ldout(cct, 10) << __func__ << " " << r->tid << " " << r->cmd << dendl; auto m = ceph::make_message(monmap.fsid); m->set_tid(r->tid); @@ -1286,6 +1287,26 @@ void MonClient::_resend_mon_commands() // starting with octopus, tell commands use their own connetion and need no // special resend when we finish hunting. } else { + if (!cct->_conf->mon_client_hunt_on_resent) { + // Ensure the target name of the resend mon command matches the rank + // of the current active connection. + ldout(cct, 20) << __func__ << " " << cmd->tid << " " << cmd->cmd + << " last sent_name " << cmd->sent_name << dendl; + if (!monmap.contains(cmd->sent_name)) { + ldout(cct, 20) << __func__ << " " << cmd->tid << " " << cmd->cmd + << " sent_name " << cmd->sent_name << " not in monmap using mon:" << + monmap.get_name(active_con->get_con()->get_peer_addr()) << dendl; + cmd->sent_name = monmap.get_name(active_con->get_con()->get_peer_addr()); + } else if (active_con && cmd->sent_name.length() && + cmd->sent_name != monmap.get_name(active_con->get_con()->get_peer_addr()) && + monmap.contains(cmd->sent_name)) { + ldout(cct, 20) << __func__ << " " << cmd->tid << " " << cmd->cmd + << " wants mon " << cmd->sent_name + << " current connection is " << monmap.get_name(active_con->get_con()->get_peer_addr()) + << ", reopening session" << dendl; + _reopen_session(monmap.get_rank(cmd->sent_name)); + } + } _send_command(cmd); // might remove cmd from mon_commands } } diff --git a/src/mon/MonClient.h b/src/mon/MonClient.h index 803c74eb7f6294..00e1401ef6ca19 100644 --- a/src/mon/MonClient.h +++ b/src/mon/MonClient.h @@ -559,6 +559,7 @@ class MonClient : public Dispatcher, struct MonCommand { // for tell only std::string target_name; + std::string sent_name; int target_rank = -1; ConnectionRef target_con; std::unique_ptr target_session;