From: Matty Williams Date: Fri, 5 Jun 2026 15:22:26 +0000 (+0100) Subject: osd: Enable omaps in Fast EC pools X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=58640e23f05492198fa6ade0d7124401dd42e9ee;p=ceph.git osd: Enable omaps in Fast EC pools Enable supports_omap flag in Fast EC pools. Enable testing of omap operations in Fast EC pools. Fixes: https://tracker.ceph.com/issues/77329 Signed-off-by: Matty Williams --- diff --git a/doc/dev/osd_internals/erasure_coding/client_support.rst b/doc/dev/osd_internals/erasure_coding/client_support.rst index 96c84bd7432..f899c98e51a 100644 --- a/doc/dev/osd_internals/erasure_coding/client_support.rst +++ b/doc/dev/osd_internals/erasure_coding/client_support.rst @@ -908,7 +908,8 @@ To enable the new features, the following OSDMap pool settings are required: - ``allows_ec_overwrites = true` - ``allows_ec_optimizations = true`` -- ``supports_omap = true`` These settings can be configured per-pool. The cluster will enforce that all -OSDs are at the Umbrella release before allowing omap support to be enabled. \ No newline at end of file +OSDs are at the Umbrella release before allowing omap support to be enabled. + +The `supports_omap` pool flag is automatically enabled in pools with EC optimizations enabled. \ No newline at end of file diff --git a/doc/glossary.rst b/doc/glossary.rst index 0df78710717..a965b68cd8a 100644 --- a/doc/glossary.rst +++ b/doc/glossary.rst @@ -353,9 +353,8 @@ reduce the time it takes to read data from and to write to the Ceph cluster. RGW bucket indexes are stored as omaps. Erasure-coded pools, by default, cannot store RADOS omap data - structures. This functionality can be enabled in erasure-coded - pools with optimizations enabled by setting the - ``supports_omap`` flag to true. + structures. This functionality can be enabled in non-Crimson + erasure-coded pools by enabling EC optimizations. Run the command ``ceph osd df`` to see the storage space used by omaps on each OSD. diff --git a/doc/rados/operations/pools.rst b/doc/rados/operations/pools.rst index dca87534d23..f00156d02f4 100644 --- a/doc/rados/operations/pools.rst +++ b/doc/rados/operations/pools.rst @@ -461,7 +461,7 @@ You may set values for the following keys: .. describe:: supports_omap - :Description: Determines whether omap operations can be performed on a pool. On by default for replicated pools, off by default for erasure coded pools. The flag can be enabled for erasure coded pools that allow ec optimizations. + :Description: Determines whether omap operations can be performed on a pool. On for replicated and erasure-coded pools with EC optimizations enabled (excluding Crimson pools), off for all other erasure-coded pools. :Type: Boolean .. describe:: hashpspool diff --git a/src/mon/MonCommands.h b/src/mon/MonCommands.h index 3c7f13db567..8891d1f8dc8 100644 --- a/src/mon/MonCommands.h +++ b/src/mon/MonCommands.h @@ -1292,7 +1292,6 @@ COMMAND("osd pool set " "|scrub_priority" "|set_pool_flags" "|size" - "|supports_omap" "|target_max_bytes" "|target_max_objects" "|target_size_bytes" diff --git a/src/mon/OSDMonitor.cc b/src/mon/OSDMonitor.cc index c27879fcd44..87ee9f8a1f5 100644 --- a/src/mon/OSDMonitor.cc +++ b/src/mon/OSDMonitor.cc @@ -1996,9 +1996,12 @@ void OSDMonitor::encode_pending(MonitorDBStore::TransactionRef t) } } - // Auto-enable omap support for replicated pools + // Auto-enable omap support for replicated and fast EC pools + // Omap is not supported by EC pools in crimson for (auto& [pool_id, pool] : tmp.get_pools()) { - if (!pool.has_flag(pg_pool_t::FLAG_OMAP) && pool.is_replicated()) { + if (!pool.has_flag(pg_pool_t::FLAG_OMAP) && + (pool.is_replicated() || + (pool.allows_ecoptimizations() && !pool.is_crimson()))) { pg_pool_t p = pool; p.flags |= pg_pool_t::FLAG_OMAP; pending_inc.new_pools[pool_id] = p; @@ -8351,10 +8354,6 @@ int OSDMonitor::prepare_new_pool(string& name, if (crimson) { pi->set_flag(pg_pool_t::FLAG_CRIMSON); } - if (pool_type == pg_pool_t::TYPE_REPLICATED - && osdmap.require_osd_release >= ceph_release_t::umbrella) { - pi->set_flag(pg_pool_t::FLAG_OMAP); - } pi->size = size; pi->min_size = min_size; @@ -8455,6 +8454,14 @@ int OSDMonitor::prepare_new_pool(string& name, maybe_enable_pool_split_ops(*pi); + // Auto-enable omap support for replicated and fast EC pools + // Omap is not supported by EC pools in crimson + if (osdmap.require_osd_release >= ceph_release_t::umbrella && + (pool_type == pg_pool_t::TYPE_REPLICATED || + (pi->allows_ecoptimizations() && !crimson))) { + pi->set_flag(pg_pool_t::FLAG_OMAP); + } + pending_inc.new_pool_names[pool] = name; return 0; } @@ -8546,6 +8553,12 @@ int OSDMonitor::enable_pool_ec_optimizations(pg_pool_t &p, } } p.flags |= pg_pool_t::FLAG_EC_OPTIMIZATIONS; + + // Automatically enable omap support in fast EC pools + // Omap is not supported by EC pools in crimson + if (!p.is_crimson() && osdmap.require_osd_release >= ceph_release_t::umbrella) { + p.flags |= pg_pool_t::FLAG_OMAP; + } } else { if ((p.flags & pg_pool_t::FLAG_EC_OPTIMIZATIONS) != 0) { if (ss) { @@ -9149,25 +9162,6 @@ int OSDMonitor::prepare_command_pool_set(const cmdmap_t& cmdmap, } else { p.unset_flag(n); } - } else if (var == "supports_omap") { - if ((val == "true") && osdmap.require_osd_release < ceph_release_t::umbrella) { - ss << "supports_omap cannot be enabled until require_osd_release is set to umbrella or later"; - return -EPERM; - } - // Disabling omap support will leave omap data in RocksDB which cannot be cleaned up - // It will also break any services that depend on this pool to store metadata - if ((val == "false") && (p.has_flag(pg_pool_t::FLAG_OMAP))) { - ss << "supports_omap cannot be disabled once enabled"; - return -EINVAL; - } - // This restriction is temporary until omap support is well tested in Fast EC pools - if ((val == "true") && p.is_erasure()) { - ss << "supports_omap cannot be enabled in ec pools"; - return -EINVAL; - } - if (val == "true") { - p.flags |= pg_pool_t::FLAG_OMAP; - } } else if (var == "target_max_objects") { if (interr.length()) { ss << "error parsing int '" << val << "': " << interr; diff --git a/src/test/librados/misc_cxx.cc b/src/test/librados/misc_cxx.cc index f3f34655bcc..14c785e2486 100644 --- a/src/test/librados/misc_cxx.cc +++ b/src/test/librados/misc_cxx.cc @@ -18,6 +18,7 @@ #include "include/stringify.h" #include "common/Checksummer.h" #include "common/config_proxy.h" // for class ConfigProxy +#include "common/ceph_json.h" #include "mds/mdstypes.h" #include "global/global_context.h" #include "test/librados/testcase_cxx.h" @@ -472,6 +473,56 @@ TEST_P(LibRadosMiscPP, CopyPP) { } } +// Helper function to check if a pool supports omap operations +// This function will cause the test to fail if it cannot determine the pool's omap support +static bool supports_omap(librados::IoCtx& ioctx, librados::Rados& cluster) { + std::string pool_name = ioctx.get_pool_name(); + + bufferlist inbl, outbl; + std::ostringstream cmd; + cmd << "{\"prefix\": \"osd pool get\", " + << "\"pool\": \"" << pool_name << "\", " + << "\"var\": \"supports_omap\", " + << "\"format\": \"json\"}"; + + // Send command to determine omap support for the pool + int ret = cluster.mon_command(cmd.str(), std::move(inbl), &outbl, nullptr); + EXPECT_EQ(0, ret); + if (ret != 0) { + return false; + } + + // Parse JSON response safely + std::string outstr = outbl.to_str(); + EXPECT_FALSE(outstr.empty()); + if (outstr.empty()) { + return false; + } + + JSONParser parser; + bool parse_success = parser.parse(outstr.c_str(), outstr.length()); + EXPECT_TRUE(parse_success); + if (!parse_success) { + return false; + } + + // Extract supports_omap value from JSON + bool supports_omap_value = false; + bool decode_success = false; + try { + JSONDecoder::decode_json("supports_omap", supports_omap_value, &parser); + decode_success = true; + } catch (const JSONDecoder::err& e) { + // Will be caught by EXPECT below + } + EXPECT_TRUE(decode_success); + if (!decode_success) { + return false; + } + + return supports_omap_value; +} + class LibRadosTwoPoolsECPP : public RadosTestECPP { public: @@ -533,6 +584,9 @@ TEST_P(LibRadosTwoPoolsECPP, CopyFrom) { bufferlist b; b.append("copyfrom"); + // Check if target pool supports omap + bool target_supports_omap = supports_omap(ioctx, cluster); + // create big object w/ omapheader { ASSERT_EQ(0, src_ioctx.write_full("foo", z)); @@ -540,7 +594,8 @@ TEST_P(LibRadosTwoPoolsECPP, CopyFrom) { version_t uv = src_ioctx.get_last_version(); ObjectWriteOperation op; op.copy_from("foo", src_ioctx, uv, 0); - ASSERT_EQ(-EOPNOTSUPP, ioctx.operate("foo.copy", &op)); + int expected = target_supports_omap ? 0 : -EOPNOTSUPP; + ASSERT_EQ(expected, ioctx.operate("foo.copy", &op)); } // same with small object @@ -549,7 +604,8 @@ TEST_P(LibRadosTwoPoolsECPP, CopyFrom) { version_t uv = src_ioctx.get_last_version(); ObjectWriteOperation op; op.copy_from("bar", src_ioctx, uv, 0); - ASSERT_EQ(-EOPNOTSUPP, ioctx.operate("bar.copy", &op)); + int expected = target_supports_omap ? 0 : -EOPNOTSUPP; + ASSERT_EQ(expected, ioctx.operate("bar.copy", &op)); } } diff --git a/src/test/librados/omap_cxx.cc b/src/test/librados/omap_cxx.cc index eff925454b8..be6e88ea334 100644 --- a/src/test/librados/omap_cxx.cc +++ b/src/test/librados/omap_cxx.cc @@ -46,41 +46,21 @@ protected: void SetUp() override { SKIP_IF_CRIMSON(); - // Skip EC pools before creating resources - if (GetParam() == PoolType::FAST_EC) { - GTEST_SKIP() << "EC pools do not support omap yet"; - } + // Call base class SetUp to create pool and ioctx PoolTypeTestFixture::SetUp(); nspace = get_temp_pool_name(); ioctx.set_namespace(nspace); - - // Enable omap for EC pools - if (pool_type == PoolType::FAST_EC) { - enable_omap(); - } } void TearDown() override { SKIP_IF_CRIMSON(); - if (GetParam() == PoolType::FAST_EC) { - GTEST_SKIP() << "EC pools do not support omap yet"; - } + // Call base class TearDown to clean up pool PoolTypeTestFixture::TearDown(); } - // Helper methods - void enable_omap() { - bufferlist inbl, outbl; - std::ostringstream oss; - oss << "{\"prefix\": \"osd pool set\", \"pool\": \"" << pool_name - << "\", \"var\": \"supports_omap\", \"val\": \"true\"}"; - int ret = rados.mon_command(oss.str(), std::move(inbl), &outbl, nullptr); - ASSERT_EQ(0, ret); - } - void turn_balancing_off() { int rc; std::ostringstream oss; diff --git a/src/test/librados/test_cxx.cc b/src/test/librados/test_cxx.cc index 8d153739015..1f9f34c0705 100644 --- a/src/test/librados/test_cxx.cc +++ b/src/test/librados/test_cxx.cc @@ -108,13 +108,13 @@ int destroy_ec_profile_and_rule_pp(Rados &cluster, } std::string create_one_ec_pool_pp(const std::string &pool_name, - Rados &cluster, bool optimised_ec, bool enable_omap) + Rados &cluster, bool fast_ec) { std::string err = connect_cluster_pp(cluster); if (err.length()) return err; - err = create_ec_pool_pp(pool_name, cluster, optimised_ec, enable_omap); + err = create_ec_pool_pp(pool_name, cluster, fast_ec); if (err.length()) { cluster.shutdown(); return err; @@ -145,7 +145,7 @@ std::string create_pool_pp(const std::string &pool_name, Rados &cluster) { return ""; } -std::string create_ec_pool_pp(const std::string &pool_name, Rados &cluster, bool optimised_ec, bool enable_omap) { +std::string create_ec_pool_pp(const std::string &pool_name, Rados &cluster, bool fast_ec) { std::ostringstream oss; int ret = destroy_ec_profile_and_rule_pp(cluster, pool_name, oss); if (ret) { @@ -170,7 +170,7 @@ std::string create_ec_pool_pp(const std::string &pool_name, Rados &cluster, bool return oss.str(); } - if (optimised_ec) { + if (fast_ec) { bufferlist inbl; ret = cluster.mon_command( "{\"prefix\": \"osd pool set\", \"pool\": \"" + pool_name + @@ -182,20 +182,6 @@ std::string create_ec_pool_pp(const std::string &pool_name, Rados &cluster, bool oss << "rados_mon_command osd pool set failed with error " << ret; return oss.str(); } - - if (enable_omap) { - bufferlist inbl, outbl; - std::ostringstream oss; - oss << "{\"prefix\": \"osd pool set\", \"pool\": \"" << pool_name - << "\", \"var\": \"supports_omap\", \"val\": \"true\"}"; - ret = cluster.mon_command(oss.str(), std::move(inbl), &outbl, nullptr); - if (ret) { - destroy_one_ec_pool_pp(pool_name, cluster); - destroy_ec_profile_pp(cluster, pool_name, oss); - oss << "rados_mon_command osd pool set failed with error " << ret; - return oss.str(); - } - } } cluster.wait_for_latest_osdmap(); diff --git a/src/test/librados/test_cxx.h b/src/test/librados/test_cxx.h index c36ac847882..4bedfa4bd40 100644 --- a/src/test/librados/test_cxx.h +++ b/src/test/librados/test_cxx.h @@ -14,13 +14,11 @@ std::string create_one_pool_pp(const std::string &pool_name, std::string create_one_ec_pool_pp( const std::string &pool_name, librados::Rados &cluster, - bool optimised_ec = false, - bool enable_omap = false); + bool fast_ec = false); std::string create_ec_pool_pp( const std::string &pool_name, librados::Rados &cluster, - bool optimised_ec = false, - bool enable_omap = false); + bool fast_ec = false); std::string create_pool_pp(const std::string &pool_name, librados::Rados &cluster); std::string set_allow_ec_overwrites_pp(const std::string &pool_name, diff --git a/src/test/librados/test_pool_types.h b/src/test/librados/test_pool_types.h index 54175df9dc7..cfe5554a65c 100644 --- a/src/test/librados/test_pool_types.h +++ b/src/test/librados/test_pool_types.h @@ -41,7 +41,7 @@ inline std::string create_pool_by_type( case PoolType::REPLICATED: return create_one_pool_pp(pool_name, cluster); case PoolType::FAST_EC: { - std::string result = create_one_ec_pool_pp(pool_name, cluster, true, true); + std::string result = create_one_ec_pool_pp(pool_name, cluster, true); if (result != "") { return result; } @@ -50,7 +50,7 @@ inline std::string create_pool_by_type( return result; } case PoolType::LEGACY_EC: - return create_one_ec_pool_pp(pool_name, cluster, false, false); + return create_one_ec_pool_pp(pool_name, cluster, false); default: return "Unknown pool type"; } diff --git a/src/test/librados/testcase_cxx.cc b/src/test/librados/testcase_cxx.cc index 8a8a52a64fe..45e7e99fdff 100644 --- a/src/test/librados/testcase_cxx.cc +++ b/src/test/librados/testcase_cxx.cc @@ -753,16 +753,6 @@ void RadosTestECOptimisedPP::turn_balancing_on() EXPECT_EQ(rc, 0); } -void RadosTestECOptimisedPP::enable_omap() -{ - bufferlist inbl, outbl; - std::ostringstream oss; - oss << "{\"prefix\": \"osd pool set\", \"pool\": \"" << pool_name - << "\", \"var\": \"supports_omap\", \"val\": \"true\"}"; - int ret = cluster.mon_command(oss.str(), std::move(inbl), &outbl, nullptr); - EXPECT_EQ(ret, 0); -} - int RadosTestECOptimisedPP::request_osd_map( std::string oid, ceph::messaging::osd::OSDMapReply* reply) { diff --git a/src/test/librados/testcase_cxx.h b/src/test/librados/testcase_cxx.h index 42e70c27462..89bd5079927 100644 --- a/src/test/librados/testcase_cxx.h +++ b/src/test/librados/testcase_cxx.h @@ -226,7 +226,6 @@ protected: static std::string pool_name; void turn_balancing_off(); void turn_balancing_on(); - void enable_omap(); int request_osd_map( std::string oid, ceph::messaging::osd::OSDMapReply* reply