From: Adam C. Emerson Date: Wed, 24 Aug 2022 13:51:18 +0000 (-0400) Subject: rgw: `SimpleRadosReadAttrsCR` uses an async RADOS call X-Git-Tag: v18.1.0~499^2~13 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=04d3a4f0688c4d444b23067cbf92ddd8c18db41f;p=ceph.git rgw: `SimpleRadosReadAttrsCR` uses an async RADOS call Don't go through the 'system object' cache. This also saves us the use of the RADOS async completion processor. Signed-off-by: Adam C. Emerson --- diff --git a/src/rgw/driver/rados/rgw_cr_rados.cc b/src/rgw/driver/rados/rgw_cr_rados.cc index 05079723792fe..eb5db86f25a9f 100644 --- a/src/rgw/driver/rados/rgw_cr_rados.cc +++ b/src/rgw/driver/rados/rgw_cr_rados.cc @@ -131,21 +131,39 @@ RGWAsyncGetSystemObj::RGWAsyncGetSystemObj(const DoutPrefixProvider *_dpp, RGWCo int RGWSimpleRadosReadAttrsCR::send_request(const DoutPrefixProvider *dpp) { - req = new RGWAsyncGetSystemObj(dpp, this, stack->create_completion_notifier(), - svc, objv_tracker, obj, true, raw_attrs); - async_rados->queue(req); - return 0; + int r = store->getRados()->get_raw_obj_ref(dpp, obj, &ref); + if (r < 0) { + ldpp_dout(dpp, -1) << "ERROR: failed to get ref for (" << obj << ") ret=" + << r << dendl; + return r; + } + + set_status() << "sending request"; + + librados::ObjectReadOperation op; + if (objv_tracker) { + objv_tracker->prepare_op_for_read(&op); + } + + if (raw_attrs && pattrs) { + op.getxattrs(pattrs, nullptr); + } else { + op.getxattrs(&unfiltered_attrs, nullptr); + } + + cn = stack->create_completion_notifier(); + return ref.pool.ioctx().aio_operate(ref.obj.oid, cn->completion(), &op, + nullptr); } int RGWSimpleRadosReadAttrsCR::request_complete() { - if (pattrs) { - *pattrs = std::move(req->attrs); - } - if (objv_tracker) { - *objv_tracker = req->objv_tracker; + int ret = cn->completion()->get_return_value(); + set_status() << "request complete; ret=" << ret; + if (!raw_attrs && pattrs) { + rgw_filter_attrset(unfiltered_attrs, RGW_ATTR_PREFIX, pattrs); } - return req->get_ret_status(); + return ret; } int RGWAsyncPutSystemObj::_send_request(const DoutPrefixProvider *dpp) diff --git a/src/rgw/driver/rados/rgw_cr_rados.h b/src/rgw/driver/rados/rgw_cr_rados.h index ed7611dc1f211..8d14be261cc65 100644 --- a/src/rgw/driver/rados/rgw_cr_rados.h +++ b/src/rgw/driver/rados/rgw_cr_rados.h @@ -484,38 +484,28 @@ int RGWSimpleRadosReadCR::request_complete() } class RGWSimpleRadosReadAttrsCR : public RGWSimpleCoroutine { - const DoutPrefixProvider *dpp; - RGWAsyncRadosProcessor *async_rados; - RGWSI_SysObj *svc; + const DoutPrefixProvider* dpp; + rgw::sal::RadosStore* const store; - rgw_raw_obj obj; - std::map *pattrs; - bool raw_attrs; - RGWObjVersionTracker* objv_tracker; - RGWAsyncGetSystemObj *req = nullptr; + const rgw_raw_obj obj; + std::map* const pattrs; + const bool raw_attrs; + RGWObjVersionTracker* const objv_tracker; + + rgw_rados_ref ref; + std::map unfiltered_attrs; + boost::intrusive_ptr cn; public: - RGWSimpleRadosReadAttrsCR(const DoutPrefixProvider *_dpp, RGWAsyncRadosProcessor *_async_rados, RGWSI_SysObj *_svc, - const rgw_raw_obj& _obj, std::map *_pattrs, - bool _raw_attrs, RGWObjVersionTracker* objv_tracker = nullptr) - : RGWSimpleCoroutine(_svc->ctx()), - dpp(_dpp), - async_rados(_async_rados), svc(_svc), - obj(_obj), - pattrs(_pattrs), - raw_attrs(_raw_attrs), - objv_tracker(objv_tracker) - {} - ~RGWSimpleRadosReadAttrsCR() override { - request_cleanup(); - } - - void request_cleanup() override { - if (req) { - req->finish(); - req = NULL; - } - } + RGWSimpleRadosReadAttrsCR(const DoutPrefixProvider* dpp, + rgw::sal::RadosStore* store, + rgw_raw_obj obj, + std::map* pattrs, + bool raw_attrs, + RGWObjVersionTracker* objv_tracker = nullptr) + : RGWSimpleCoroutine(store->ctx()), dpp(dpp), store(store), + obj(std::move(obj)), pattrs(pattrs), raw_attrs(raw_attrs), + objv_tracker(objv_tracker) {} int send_request(const DoutPrefixProvider *dpp) override; int request_complete() override; diff --git a/src/rgw/driver/rados/rgw_data_sync.cc b/src/rgw/driver/rados/rgw_data_sync.cc index ab322b8146e58..2c4b5783729d7 100644 --- a/src/rgw/driver/rados/rgw_data_sync.cc +++ b/src/rgw/driver/rados/rgw_data_sync.cc @@ -3306,7 +3306,7 @@ public: int RGWReadBucketPipeSyncStatusCoroutine::operate(const DoutPrefixProvider *dpp) { reenter(this) { - yield call(new RGWSimpleRadosReadAttrsCR(dpp, sync_env->async_rados, sync_env->svc->sysobj, + yield call(new RGWSimpleRadosReadAttrsCR(dpp, sync_env->driver, rgw_raw_obj(sync_env->svc->zone->get_zone_params().log_pool, oid), &attrs, true, objv_tracker)); if (retcode == -ENOENT) { diff --git a/src/test/rgw/CMakeLists.txt b/src/test/rgw/CMakeLists.txt index 1883d3949bb24..7d21f65ca35da 100644 --- a/src/test/rgw/CMakeLists.txt +++ b/src/test/rgw/CMakeLists.txt @@ -271,3 +271,12 @@ target_include_directories(unittest_rgw_lua SYSTEM PRIVATE "${CMAKE_SOURCE_DIR}/src/rgw/store/rados") target_link_libraries(unittest_rgw_lua ${rgw_libs}) +add_executable(radosgw-cr-test rgw_cr_test.cc) +target_link_libraries(radosgw-cr-test ${rgw_libs} librados + cls_rgw_client cls_otp_client cls_lock_client cls_refcount_client + cls_log_client cls_timeindex_client neorados_cls_fifo + cls_version_client cls_user_client + global ${LIB_RESOLV} + OATH::OATH + ${CURL_LIBRARIES} ${EXPAT_LIBRARIES} ${BLKID_LIBRARIES} + GTest::GTest) diff --git a/src/test/rgw/rgw_cr_test.cc b/src/test/rgw/rgw_cr_test.cc new file mode 100644 index 0000000000000..e9f7596aab54f --- /dev/null +++ b/src/test/rgw/rgw_cr_test.cc @@ -0,0 +1,182 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab ft=cpp + +#include +#include +#include +#include + +#undef FMT_HEADER_ONLY +#define FMT_HEADER_ONLY 1 +#include + +#include "include/rados/librados.hpp" + +#include "common/common_init.h" +#include "common/config.h" +#include "common/ceph_argparse.h" +#include "common/debug.h" + +#include "rgw_coroutine.h" +#include "rgw_cr_rados.h" +#include "rgw_sal.h" +#include "rgw_sal_rados.h" + +#include "gtest/gtest.h" + +using namespace std::literals; + +static constexpr auto dout_subsys = ceph_subsys_rgw; + +static rgw::sal::RadosStore* store = nullptr; + +static const DoutPrefixProvider* dpp() { + struct GlobalPrefix : public DoutPrefixProvider { + CephContext *get_cct() const override { return g_ceph_context; } + unsigned get_subsys() const override { return dout_subsys; } + std::ostream& gen_prefix(std::ostream& out) const override { return out; } + }; + static GlobalPrefix global_dpp; + return &global_dpp; +} + +class StoreDestructor { + rgw::sal::Driver* driver; +public: + explicit StoreDestructor(rgw::sal::RadosStore* _s) : driver(_s) {} + ~StoreDestructor() { + DriverManager::close_storage(store); + } +}; + +struct TempPool { + inline static uint64_t num = 0; + std::string name = + fmt::format("{}-{}-{}", ::time(nullptr), ::getpid(),num++); + + TempPool() { + auto r = store->getRados()->get_rados_handle()->pool_create(name.c_str()); + assert(r == 0); + } + + ~TempPool() { + auto r = store->getRados()->get_rados_handle()->pool_delete(name.c_str()); + assert(r == 0); + } + + operator rgw_pool() { + return { name }; + } + + operator librados::IoCtx() { + librados::IoCtx ioctx; + auto r = store->getRados()->get_rados_handle()->ioctx_create(name.c_str(), + ioctx); + assert(r == 0); + return ioctx; + } +}; + +int run(RGWCoroutine* cr) { + RGWCoroutinesManager cr_mgr{store->ctx(), + store->getRados()->get_cr_registry()}; + std::list stacks; + auto stack = new RGWCoroutinesStack(store->ctx(), &cr_mgr); + stack->call(cr); + stacks.push_back(stack); + return cr_mgr.run(dpp(), stacks); +} + +TEST(ReadAttrs, Unfiltered) { + TempPool pool; + ceph::bufferlist bl; + auto dummy = "Dummy attribute value"s; + encode(dummy, bl); + const std::map ref_attrs{ + { "foo"s, bl }, { "bar"s, bl }, { "baz"s, bl } + }; + auto oid = "object"s; + { + librados::IoCtx ioctx(pool); + librados::ObjectWriteOperation op; + op.setxattr("foo", bl); + op.setxattr("bar", bl); + op.setxattr("baz", bl); + auto r = ioctx.operate(oid, &op); + ASSERT_EQ(0, r); + } + std::map attrs; + auto r = run(new RGWSimpleRadosReadAttrsCR(dpp(), store, {pool, oid}, &attrs, + true)); + ASSERT_EQ(0, r); + ASSERT_EQ(ref_attrs, attrs); +} + +TEST(ReadAttrs, Filtered) { + TempPool pool; + ceph::bufferlist bl; + auto dummy = "Dummy attribute value"s; + encode(dummy, bl); + const std::map ref_attrs{ + { RGW_ATTR_PREFIX "foo"s, bl }, + { RGW_ATTR_PREFIX "bar"s, bl }, + { RGW_ATTR_PREFIX "baz"s, bl } + }; + auto oid = "object"s; + { + librados::IoCtx ioctx(pool); + librados::ObjectWriteOperation op; + op.setxattr(RGW_ATTR_PREFIX "foo", bl); + op.setxattr(RGW_ATTR_PREFIX "bar", bl); + op.setxattr(RGW_ATTR_PREFIX "baz", bl); + op.setxattr("oneOfTheseThingsIsNotLikeTheOthers", bl); + auto r = ioctx.operate(oid, &op); + ASSERT_EQ(0, r); + } + std::map attrs; + auto r = run(new RGWSimpleRadosReadAttrsCR(dpp(), store, {pool, oid}, &attrs, + false)); + ASSERT_EQ(0, r); + ASSERT_EQ(ref_attrs, attrs); +} + +int main(int argc, const char **argv) +{ + auto args = argv_to_vec(argc, argv); + auto cct = rgw_global_init(NULL, args, CEPH_ENTITY_TYPE_CLIENT, + CODE_ENVIRONMENT_UTILITY, 0); + + // for region -> zonegroup conversion (must happen before common_init_finish()) + if (!g_conf()->rgw_region.empty() && g_conf()->rgw_zonegroup.empty()) { + g_conf().set_val_or_die("rgw_zonegroup", g_conf()->rgw_region.c_str()); + } + + /* common_init_finish needs to be called after g_conf().set_val() */ + common_init_finish(g_ceph_context); + + + DriverManager::Config cfg = DriverManager::get_config(true, g_ceph_context); + + store = static_cast( + DriverManager::get_storage(dpp(), + g_ceph_context, + cfg, + false, + false, + false, + false, + false, + true, + false)); + if (!store) { + std::cerr << "couldn't init storage provider" << std::endl; + return 5; //EIO + } + StoreDestructor store_destructor(static_cast(store)); + + std::string pool{"rgw_cr_test"}; + store->getRados()->create_pool(dpp(), pool); + + testing::InitGoogleTest(); + return RUN_ALL_TESTS(); +}