From dce3d26d84f140d01a968dc0a615372c1cb8f7f3 Mon Sep 17 00:00:00 2001 From: Joao Eduardo Luis Date: Thu, 3 Oct 2013 19:07:12 +0100 Subject: [PATCH] mon: MonmapMonitor: make 'ceph mon add' idempotent MonMap changes lead to bootstraps. Callbacks waiting for a proposal to finish can have several fates, depending on what happens: finished, rerun or aborted. In the case of a bootstrap right after a monmap change, callbacks are rerun. Considering we queued the message that lead to the monmap change on this queue, if we instead of finishing it end up reruning it, we will end up trying to perform the same modification twice -- the last one will try to modify an already existing state and we will return just that: whatever you're attempting to do has already been done. This patch makes 'ceph mon add' completely idempotent. If one tries to add an already existing monitor (i.e., same name, same ip:port), one simply gets a 'monitor foo added', with return 0, no matter how many times one runs the command. Fixes: #5896 Signed-off-by: Joao Eduardo Luis --- src/mon/MonmapMonitor.cc | 39 ++++++++++++++++++++++++++++++++------- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/src/mon/MonmapMonitor.cc b/src/mon/MonmapMonitor.cc index 799f19df15452..ca855592445f1 100644 --- a/src/mon/MonmapMonitor.cc +++ b/src/mon/MonmapMonitor.cc @@ -298,20 +298,45 @@ bool MonmapMonitor::prepare_command(MMonCommand *m) addr.set_port(CEPH_MON_PORT); } - if (pending_map.contains(addr) || - pending_map.contains(name)) { + /** + * If we have a monitor with the same name and different addr, then EEXIST + * If we have a monitor with the same addr and different name, then EEXIST + * If we have a monitor with the same addr and same name, then return as if + * we had just added the monitor. + * If we don't have the monitor, add it. + */ + + err = 0; + if (!ss.str().empty()) + ss << "; "; + + do { + if (pending_map.contains(addr)) { + string n = pending_map.get_name(addr); + if (n == name) + break; + } else if (pending_map.contains(name)) { + entity_addr_t tmp_addr = pending_map.get_addr(name); + if (tmp_addr == addr) + break; + } else { + break; + } err = -EEXIST; - if (!ss.str().empty()) - ss << "; "; - ss << "mon " << name << " " << addr << " already exists"; + ss << "mon." << name << " at " << addr << " already exists"; + goto out; + } while (false); + + ss << "added mon." << name << " at " << addr; + if (pending_map.contains(name)) { goto out; } pending_map.add(name, addr); pending_map.last_changed = ceph_clock_now(g_ceph_context); - ss << "added mon." << name << " at " << addr; getline(ss, rs); - wait_for_finished_proposal(new Monitor::C_Command(mon, m, 0, rs, get_last_committed())); + wait_for_finished_proposal(new Monitor::C_Command(mon, m, 0, rs, + get_last_committed())); return true; } else if (prefix == "mon remove") {