From da390122289fd948a68b07b4f693ca03bd7d7076 Mon Sep 17 00:00:00 2001 From: Casey Bodley Date: Wed, 25 Sep 2024 16:20:31 -0400 Subject: [PATCH] librados/asio: add version_t to completion signatures IoCtx::aio_operate() doesn't update IoCtx::get_last_version(). to make the resulting version_t available to the caller, we have to read it out of the AioCompletionImpl and return it to the caller Signed-off-by: Casey Bodley (cherry picked from commit 5cc7cf44dba51f8071a6b7cd919c019283ac4ce1) --- src/librados/librados_asio.h | 44 +++++++------ src/test/librados/asio.cc | 121 +++++++++++++++++++++-------------- 2 files changed, 97 insertions(+), 68 deletions(-) diff --git a/src/librados/librados_asio.h b/src/librados/librados_asio.h index 19a8c8fc01de4..0aedc37657529 100644 --- a/src/librados/librados_asio.h +++ b/src/librados/librados_asio.h @@ -16,6 +16,7 @@ #include "include/rados/librados.hpp" #include "common/async/completion.h" +#include "librados/AioCompletionImpl.h" /// Defines asynchronous librados operations that satisfy all of the /// "Requirements on asynchronous operations" imposed by the C++ Networking TS @@ -53,20 +54,20 @@ using unique_aio_completion_ptr = /// argument to the handler. template struct Invoker { - using Signature = void(boost::system::error_code, Result); + using Signature = void(boost::system::error_code, version_t, Result); Result result; template - void dispatch(Completion&& completion, boost::system::error_code ec) { - ceph::async::dispatch(std::move(completion), ec, std::move(result)); + void dispatch(Completion&& completion, boost::system::error_code ec, version_t ver) { + ceph::async::dispatch(std::move(completion), ec, ver, std::move(result)); } }; // specialization for Result=void template <> struct Invoker { - using Signature = void(boost::system::error_code); + using Signature = void(boost::system::error_code, version_t); template - void dispatch(Completion&& completion, boost::system::error_code ec) { - ceph::async::dispatch(std::move(completion), ec); + void dispatch(Completion&& completion, boost::system::error_code ec, version_t ver) { + ceph::async::dispatch(std::move(completion), ec, ver); } }; @@ -82,12 +83,15 @@ struct AsyncOp : Invoker { auto p = std::unique_ptr{static_cast(arg)}; // move result out of Completion memory being freed auto op = std::move(p->user_data); - const int ret = op.aio_completion->get_return_value(); + // access AioCompletionImpl directly to avoid locking + const librados::AioCompletionImpl* pc = op.aio_completion->pc; + const int ret = pc->rval; + const version_t ver = pc->objver; boost::system::error_code ec; if (ret < 0) { ec.assign(-ret, librados::detail::err_category()); } - op.dispatch(std::move(p), ec); + op.dispatch(std::move(p), ec, ver); } template @@ -103,7 +107,7 @@ struct AsyncOp : Invoker { /// Calls IoCtx::aio_read() and arranges for the AioCompletion to call a -/// given handler with signature (boost::system::error_code, bufferlist). +/// given handler with signature (error_code, version_t, bufferlist). template auto async_read(ExecutionContext& ctx, IoCtx& io, const std::string& oid, size_t len, uint64_t off, CompletionToken&& token) @@ -119,7 +123,7 @@ auto async_read(ExecutionContext& ctx, IoCtx& io, const std::string& oid, int ret = io.aio_read(oid, op.aio_completion.get(), &op.result, len, off); if (ret < 0) { auto ec = boost::system::error_code{-ret, librados::detail::err_category()}; - ceph::async::post(std::move(p), ec, bufferlist{}); + ceph::async::post(std::move(p), ec, 0, bufferlist{}); } else { p.release(); // release ownership until completion } @@ -127,24 +131,24 @@ auto async_read(ExecutionContext& ctx, IoCtx& io, const std::string& oid, } /// Calls IoCtx::aio_write() and arranges for the AioCompletion to call a -/// given handler with signature (boost::system::error_code). +/// given handler with signature (error_code, version_t). template auto async_write(ExecutionContext& ctx, IoCtx& io, const std::string& oid, - bufferlist &bl, size_t len, uint64_t off, + const bufferlist &bl, size_t len, uint64_t off, CompletionToken&& token) { using Op = detail::AsyncOp; using Signature = typename Op::Signature; return boost::asio::async_initiate( [] (auto handler, auto ex, IoCtx& io, const std::string& oid, - bufferlist &bl, size_t len, uint64_t off) { + const bufferlist &bl, size_t len, uint64_t off) { auto p = Op::create(ex, std::move(handler)); auto& op = p->user_data; int ret = io.aio_write(oid, op.aio_completion.get(), bl, len, off); if (ret < 0) { auto ec = boost::system::error_code{-ret, librados::detail::err_category()}; - ceph::async::post(std::move(p), ec); + ceph::async::post(std::move(p), ec, 0); } else { p.release(); // release ownership until completion } @@ -152,7 +156,7 @@ auto async_write(ExecutionContext& ctx, IoCtx& io, const std::string& oid, } /// Calls IoCtx::aio_operate() and arranges for the AioCompletion to call a -/// given handler with signature (boost::system::error_code, bufferlist). +/// given handler with signature (error_code, version_t, bufferlist). template auto async_operate(ExecutionContext& ctx, IoCtx& io, const std::string& oid, ObjectReadOperation *read_op, int flags, @@ -170,7 +174,7 @@ auto async_operate(ExecutionContext& ctx, IoCtx& io, const std::string& oid, flags, &op.result); if (ret < 0) { auto ec = boost::system::error_code{-ret, librados::detail::err_category()}; - ceph::async::post(std::move(p), ec, bufferlist{}); + ceph::async::post(std::move(p), ec, 0, bufferlist{}); } else { p.release(); // release ownership until completion } @@ -178,7 +182,7 @@ auto async_operate(ExecutionContext& ctx, IoCtx& io, const std::string& oid, } /// Calls IoCtx::aio_operate() and arranges for the AioCompletion to call a -/// given handler with signature (boost::system::error_code). +/// given handler with signature (error_code, version_t). template auto async_operate(ExecutionContext& ctx, IoCtx& io, const std::string& oid, ObjectWriteOperation *write_op, int flags, @@ -196,7 +200,7 @@ auto async_operate(ExecutionContext& ctx, IoCtx& io, const std::string& oid, int ret = io.aio_operate(oid, op.aio_completion.get(), write_op, flags, trace_ctx); if (ret < 0) { auto ec = boost::system::error_code{-ret, librados::detail::err_category()}; - ceph::async::post(std::move(p), ec); + ceph::async::post(std::move(p), ec, 0); } else { p.release(); // release ownership until completion } @@ -204,7 +208,7 @@ auto async_operate(ExecutionContext& ctx, IoCtx& io, const std::string& oid, } /// Calls IoCtx::aio_notify() and arranges for the AioCompletion to call a -/// given handler with signature (boost::system::error_code, bufferlist). +/// given handler with signature (error_code, version_t, bufferlist). template auto async_notify(ExecutionContext& ctx, IoCtx& io, const std::string& oid, bufferlist& bl, uint64_t timeout_ms, CompletionToken &&token) @@ -221,7 +225,7 @@ auto async_notify(ExecutionContext& ctx, IoCtx& io, const std::string& oid, bl, timeout_ms, &op.result); if (ret < 0) { auto ec = boost::system::error_code{-ret, librados::detail::err_category()}; - ceph::async::post(std::move(p), ec, bufferlist{}); + ceph::async::post(std::move(p), ec, 0, bufferlist{}); } else { p.release(); // release ownership until completion } diff --git a/src/test/librados/asio.cc b/src/test/librados/asio.cc index 9f8844eb7bb82..01ebb95715034 100644 --- a/src/test/librados/asio.cc +++ b/src/test/librados/asio.cc @@ -28,8 +28,6 @@ #define dout_subsys ceph_subsys_rados #define dout_context g_ceph_context -using namespace std; - // test fixture for global setup/teardown class AsioRados : public ::testing::Test { static constexpr auto poolname = "ceph_test_rados_api_asio"; @@ -73,6 +71,9 @@ librados::Rados AsioRados::rados; librados::IoCtx AsioRados::io; librados::IoCtx AsioRados::snapio; +using boost::system::error_code; +using read_result = std::tuple; + void rethrow(std::exception_ptr eptr) { if (eptr) std::rethrow_exception(eptr); } @@ -81,14 +82,17 @@ TEST_F(AsioRados, AsyncReadCallback) { boost::asio::io_context service; - auto success_cb = [&] (boost::system::error_code ec, bufferlist bl) { + auto success_cb = [&] (error_code ec, version_t ver, bufferlist bl) { EXPECT_FALSE(ec); + EXPECT_LT(0, ver); EXPECT_EQ("hello", bl.to_str()); }; librados::async_read(service, io, "exist", 256, 0, success_cb); - auto failure_cb = [&] (boost::system::error_code ec, bufferlist bl) { + auto failure_cb = [&] (error_code ec, version_t ver, bufferlist bl) { EXPECT_EQ(boost::system::errc::no_such_file_or_directory, ec); + EXPECT_EQ(0, ver); + EXPECT_EQ(0, bl.length()); }; librados::async_read(service, io, "noexist", 256, 0, failure_cb); @@ -99,17 +103,17 @@ TEST_F(AsioRados, AsyncReadFuture) { boost::asio::io_context service; - std::future f1 = librados::async_read(service, io, "exist", 256, - 0, boost::asio::use_future); - std::future f2 = librados::async_read(service, io, "noexist", 256, - 0, boost::asio::use_future); + auto f1 = librados::async_read(service, io, "exist", 256, + 0, boost::asio::use_future); + auto f2 = librados::async_read(service, io, "noexist", 256, + 0, boost::asio::use_future); service.run(); - EXPECT_NO_THROW({ - auto bl = f1.get(); - EXPECT_EQ("hello", bl.to_str()); - }); + auto [ver, bl] = f1.get(); + EXPECT_LT(0, ver); + EXPECT_EQ("hello", bl.to_str()); + EXPECT_THROW(f2.get(), boost::system::system_error); } @@ -118,17 +122,22 @@ TEST_F(AsioRados, AsyncReadYield) boost::asio::io_context service; auto success_cr = [&] (boost::asio::yield_context yield) { - boost::system::error_code ec; - auto bl = librados::async_read(service, io, "exist", 256, 0, yield[ec]); + error_code ec; + auto [ver, bl] = librados::async_read(service, io, "exist", 256, + 0, yield[ec]); EXPECT_FALSE(ec); + EXPECT_LT(0, ver); EXPECT_EQ("hello", bl.to_str()); }; boost::asio::spawn(service, success_cr, rethrow); auto failure_cr = [&] (boost::asio::yield_context yield) { - boost::system::error_code ec; - auto bl = librados::async_read(service, io, "noexist", 256, 0, yield[ec]); + error_code ec; + auto [ver, bl] = librados::async_read(service, io, "noexist", 256, + 0, yield[ec]); EXPECT_EQ(boost::system::errc::no_such_file_or_directory, ec); + EXPECT_EQ(0, ver); + EXPECT_EQ(0, bl.length()); }; boost::asio::spawn(service, failure_cr, rethrow); @@ -142,14 +151,16 @@ TEST_F(AsioRados, AsyncWriteCallback) bufferlist bl; bl.append("hello"); - auto success_cb = [&] (boost::system::error_code ec) { + auto success_cb = [&] (error_code ec, version_t ver) { EXPECT_FALSE(ec); + EXPECT_LT(0, ver); }; librados::async_write(service, io, "exist", bl, bl.length(), 0, success_cb); - auto failure_cb = [&] (boost::system::error_code ec) { + auto failure_cb = [&] (error_code ec, version_t ver) { EXPECT_EQ(boost::system::errc::read_only_file_system, ec); + EXPECT_EQ(0, ver); }; librados::async_write(service, snapio, "exist", bl, bl.length(), 0, failure_cb); @@ -171,7 +182,7 @@ TEST_F(AsioRados, AsyncWriteFuture) service.run(); - EXPECT_NO_THROW(f1.get()); + EXPECT_LT(0, f1.get()); EXPECT_THROW(f2.get(), boost::system::system_error); } @@ -183,19 +194,21 @@ TEST_F(AsioRados, AsyncWriteYield) bl.append("hello"); auto success_cr = [&] (boost::asio::yield_context yield) { - boost::system::error_code ec; - librados::async_write(service, io, "exist", bl, bl.length(), 0, - yield[ec]); + error_code ec; + auto ver = librados::async_write(service, io, "exist", bl, + bl.length(), 0, yield[ec]); EXPECT_FALSE(ec); + EXPECT_LT(0, ver); EXPECT_EQ("hello", bl.to_str()); }; boost::asio::spawn(service, success_cr, rethrow); auto failure_cr = [&] (boost::asio::yield_context yield) { - boost::system::error_code ec; - librados::async_write(service, snapio, "exist", bl, bl.length(), 0, - yield[ec]); + error_code ec; + auto ver = librados::async_write(service, snapio, "exist", bl, + bl.length(), 0, yield[ec]); EXPECT_EQ(boost::system::errc::read_only_file_system, ec); + EXPECT_EQ(0, ver); }; boost::asio::spawn(service, failure_cr, rethrow); @@ -208,8 +221,9 @@ TEST_F(AsioRados, AsyncReadOperationCallback) { librados::ObjectReadOperation op; op.read(0, 0, nullptr, nullptr); - auto success_cb = [&] (boost::system::error_code ec, bufferlist bl) { + auto success_cb = [&] (error_code ec, version_t ver, bufferlist bl) { EXPECT_FALSE(ec); + EXPECT_LT(0, ver); EXPECT_EQ("hello", bl.to_str()); }; librados::async_operate(service, io, "exist", &op, 0, nullptr, success_cb); @@ -217,8 +231,10 @@ TEST_F(AsioRados, AsyncReadOperationCallback) { librados::ObjectReadOperation op; op.read(0, 0, nullptr, nullptr); - auto failure_cb = [&] (boost::system::error_code ec, bufferlist bl) { + auto failure_cb = [&] (error_code ec, version_t ver, bufferlist bl) { EXPECT_EQ(boost::system::errc::no_such_file_or_directory, ec); + EXPECT_EQ(0, ver); + EXPECT_EQ(0, bl.length()); }; librados::async_operate(service, io, "noexist", &op, 0, nullptr, failure_cb); } @@ -228,14 +244,14 @@ TEST_F(AsioRados, AsyncReadOperationCallback) TEST_F(AsioRados, AsyncReadOperationFuture) { boost::asio::io_context service; - std::future f1; + std::future f1; { librados::ObjectReadOperation op; op.read(0, 0, nullptr, nullptr); f1 = librados::async_operate(service, io, "exist", &op, 0, nullptr, boost::asio::use_future); } - std::future f2; + std::future f2; { librados::ObjectReadOperation op; op.read(0, 0, nullptr, nullptr); @@ -244,10 +260,10 @@ TEST_F(AsioRados, AsyncReadOperationFuture) } service.run(); - EXPECT_NO_THROW({ - auto bl = f1.get(); - EXPECT_EQ("hello", bl.to_str()); - }); + auto [ver, bl] = f1.get(); + EXPECT_LT(0, ver); + EXPECT_EQ("hello", bl.to_str()); + EXPECT_THROW(f2.get(), boost::system::system_error); } @@ -258,10 +274,11 @@ TEST_F(AsioRados, AsyncReadOperationYield) auto success_cr = [&] (boost::asio::yield_context yield) { librados::ObjectReadOperation op; op.read(0, 0, nullptr, nullptr); - boost::system::error_code ec; - auto bl = librados::async_operate(service, io, "exist", &op, 0, nullptr, - yield[ec]); + error_code ec; + auto [ver, bl] = librados::async_operate(service, io, "exist", &op, + 0, nullptr, yield[ec]); EXPECT_FALSE(ec); + EXPECT_LT(0, ver); EXPECT_EQ("hello", bl.to_str()); }; boost::asio::spawn(service, success_cr, rethrow); @@ -269,10 +286,12 @@ TEST_F(AsioRados, AsyncReadOperationYield) auto failure_cr = [&] (boost::asio::yield_context yield) { librados::ObjectReadOperation op; op.read(0, 0, nullptr, nullptr); - boost::system::error_code ec; - auto bl = librados::async_operate(service, io, "noexist", &op, 0, nullptr, - yield[ec]); + error_code ec; + auto [ver, bl] = librados::async_operate(service, io, "noexist", &op, + 0, nullptr, yield[ec]); EXPECT_EQ(boost::system::errc::no_such_file_or_directory, ec); + EXPECT_EQ(0, ver); + EXPECT_EQ(0, bl.length()); }; boost::asio::spawn(service, failure_cr, rethrow); @@ -289,16 +308,18 @@ TEST_F(AsioRados, AsyncWriteOperationCallback) { librados::ObjectWriteOperation op; op.write_full(bl); - auto success_cb = [&] (boost::system::error_code ec) { + auto success_cb = [&] (error_code ec, version_t ver) { EXPECT_FALSE(ec); + EXPECT_LT(0, ver); }; librados::async_operate(service, io, "exist", &op, 0, nullptr, success_cb); } { librados::ObjectWriteOperation op; op.write_full(bl); - auto failure_cb = [&] (boost::system::error_code ec) { + auto failure_cb = [&] (error_code ec, version_t ver) { EXPECT_EQ(boost::system::errc::read_only_file_system, ec); + EXPECT_EQ(0, ver); }; librados::async_operate(service, snapio, "exist", &op, 0, nullptr, failure_cb); } @@ -312,14 +333,14 @@ TEST_F(AsioRados, AsyncWriteOperationFuture) bufferlist bl; bl.append("hello"); - std::future f1; + std::future f1; { librados::ObjectWriteOperation op; op.write_full(bl); f1 = librados::async_operate(service, io, "exist", &op, 0, nullptr, boost::asio::use_future); } - std::future f2; + std::future f2; { librados::ObjectWriteOperation op; op.write_full(bl); @@ -328,7 +349,7 @@ TEST_F(AsioRados, AsyncWriteOperationFuture) } service.run(); - EXPECT_NO_THROW(f1.get()); + EXPECT_LT(0, f1.get()); EXPECT_THROW(f2.get(), boost::system::system_error); } @@ -342,18 +363,22 @@ TEST_F(AsioRados, AsyncWriteOperationYield) auto success_cr = [&] (boost::asio::yield_context yield) { librados::ObjectWriteOperation op; op.write_full(bl); - boost::system::error_code ec; - librados::async_operate(service, io, "exist", &op, 0, nullptr, yield[ec]); + error_code ec; + auto ver = librados::async_operate(service, io, "exist", &op, + 0, nullptr, yield[ec]); EXPECT_FALSE(ec); + EXPECT_LT(0, ver); }; boost::asio::spawn(service, success_cr, rethrow); auto failure_cr = [&] (boost::asio::yield_context yield) { librados::ObjectWriteOperation op; op.write_full(bl); - boost::system::error_code ec; - librados::async_operate(service, snapio, "exist", &op, 0, nullptr, yield[ec]); + error_code ec; + auto ver = librados::async_operate(service, snapio, "exist", &op, + 0, nullptr, yield[ec]); EXPECT_EQ(boost::system::errc::read_only_file_system, ec); + EXPECT_EQ(0, ver); }; boost::asio::spawn(service, failure_cr, rethrow); -- 2.39.5