From 6eebd03175ce222a777909d45bc87fac2a28fa82 Mon Sep 17 00:00:00 2001 From: Nitzan Mordechai Date: Thu, 14 Sep 2023 09:41:13 +0000 Subject: [PATCH] mon/OSDMonitor: remove unused crush rules after erasure code pools deleted When erasure code pools are created, a corresponding Crush rule is concurrently added to the Crush map. However, when these pools are subsequently deleted, the associated rule persists within the Crush map. In the event that a pool is re-created with the same name, the rule already exists. However, if any modifications were made to the rule prior to the pool's deletion, the new pool will inherit these modifications. The proposed solution involves the automatic deletion of the Crush rule when a pool is deleted, but only if no other pools are utilizing that particular rule. Fixes: https://tracker.ceph.com/issues/62826 Signed-off-by: Nitzan Mordechai --- qa/standalone/mon/osd-crush.sh | 45 ++++++++++++++++++++++++++++++++++ src/mon/OSDMonitor.cc | 20 +++++++++++++++ 2 files changed, 65 insertions(+) diff --git a/qa/standalone/mon/osd-crush.sh b/qa/standalone/mon/osd-crush.sh index aa7cac69469..cf21f921026 100755 --- a/qa/standalone/mon/osd-crush.sh +++ b/qa/standalone/mon/osd-crush.sh @@ -120,6 +120,51 @@ function TEST_crush_rule_create_erasure() { ! ceph osd crush rule ls | grep $rule || return 1 } +function TEST_erasure_pool_crush_rule_rm() { + local dir=$1 + + run_mon $dir a || return 1 + # should have at least one OSD + run_osd $dir 0 || return 1 + + # create a new rule with pool name as rule name + # delete the rule when the pool is deleted + local pool_name=cpool + ceph osd erasure-code-profile set profile_name plugin=jerasure technique=reed_sol_van k=2 m=2 crush-failure-domain=osd || return 1 + ceph osd pool create $pool_name 12 12 erasure profile_name || return 1 + ceph osd crush rule ls | grep $pool_name || return 1 + ceph osd pool delete $pool_name $pool_name --yes-i-really-really-mean-it || return 1 + ! ceph osd crush rule ls | grep $pool_name || return 1 + + # create few pools that using the same rule + # make sure the rule is not deleted when the pool is deleted + # unlease it was the last pool using the rule + local pool_name1=pool_name1 + local pool_name2=pool_name2 + + ceph osd pool create $pool_name1 12 12 erasure profile_name || return 1 + ceph osd crush rule ls | grep $pool_name1 || return 1 + ceph osd pool create $pool_name2 12 12 erasure profile_name || return 1 + ceph osd crush rule ls | grep $pool_name2 || return 1 + ceph osd pool delete $pool_name1 $pool_name1 --yes-i-really-really-mean-it || return 1 + ! ceph osd crush rule ls | grep $pool_name || return 1 + ceph osd pool delete $pool_name2 $pool_name2 --yes-i-really-really-mean-it || return 1 + ! ceph osd crush rule ls | grep $pool_name2 || return 1 + + # crush rule should be deleted when the last pool using it is deleted + local default_rule_name=erasure-code + ceph osd pool create $pool_name-1 12 12 erasure || return 1 + ceph osd pool create $pool_name-2 12 12 erasure || return 1 + ceph osd pool create $pool_name-3 12 12 erasure || return 1 + ceph osd crush rule ls | grep $default_rule_name || return 1 + ceph osd pool delete $pool_name-1 $pool_name-1 --yes-i-really-really-mean-it || return 1 + ceph osd crush rule ls | grep $default_rule_name || return 1 + ceph osd pool delete $pool_name-2 $pool_name-2 --yes-i-really-really-mean-it || return 1 + ceph osd crush rule ls | grep $default_rule_name || return 1 + ceph osd pool delete $pool_name-3 $pool_name-3 --yes-i-really-really-mean-it || return 1 + ! ceph osd crush rule ls | grep $default_rule_name || return 1 +} + function TEST_add_rule_failed() { local dir=$1 diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc index 1c746113ce9..d1dff959563 100644 --- a/src/mon/OSDMonitor.cc +++ b/src/mon/OSDMonitor.cc @@ -15382,6 +15382,26 @@ int OSDMonitor::_prepare_remove_pool( pending_inc.crush.clear(); newcrush.encode(pending_inc.crush, mon.get_quorum_con_features()); } + + // remove any crush rules for this pool + const pg_pool_t *pi = osdmap.get_pg_pool(pool); + if (pi->is_erasure() && newcrush.rule_exists(pi->get_crush_rule())) { + int ruleno = pi->get_crush_rule(); + ceph_assert(ruleno >= 0); + + auto rule_in_use = false; + for (const auto &_pool : osdmap.pools) { + if (_pool.second.get_crush_rule() == ruleno && pool != _pool.first) + rule_in_use = true; + } + if (!rule_in_use) { + dout(10) << __func__ << " removing crush rule for pool " << pool << dendl; + newcrush.remove_rule(ruleno); + pending_inc.crush.clear(); + newcrush.encode(pending_inc.crush, mon.get_quorum_con_features()); + } + } + return 0; } -- 2.47.3