From 3bea0ffec1e7ef7142e0b995c64c262cf75e344c Mon Sep 17 00:00:00 2001 From: Radoslaw Zarzynski Date: Tue, 31 Mar 2020 17:45:51 +0200 Subject: [PATCH] crimson/osd: OP_CALL does support RETURNVEC now. Signed-off-by: Radoslaw Zarzynski --- src/crimson/common/errorator.h | 1 + src/crimson/osd/ops_executer.cc | 33 +++++++++++++++---- src/crimson/osd/ops_executer.h | 9 +++-- .../osd/osd_operations/client_request.cc | 2 +- src/crimson/osd/pg.cc | 19 ++++++++--- src/crimson/osd/pg.h | 3 +- src/test/cls_hello/test_cls_hello.cc | 2 +- 7 files changed, 51 insertions(+), 18 deletions(-) diff --git a/src/crimson/common/errorator.h b/src/crimson/common/errorator.h index b57c8407e5c..bdace987444 100644 --- a/src/crimson/common/errorator.h +++ b/src/crimson/common/errorator.h @@ -880,6 +880,7 @@ namespace ct_error { ct_error_code; using not_connected = ct_error_code; using timed_out = ct_error_code; + using value_too_large = ct_error_code; struct pass_further_all { template diff --git a/src/crimson/osd/ops_executer.cc b/src/crimson/osd/ops_executer.cc index bed7f00be74..039d39d1730 100644 --- a/src/crimson/osd/ops_executer.cc +++ b/src/crimson/osd/ops_executer.cc @@ -89,29 +89,48 @@ OpsExecuter::call_errorator::future<> OpsExecuter::do_op_call(OSDOp& osd_op) [this, prev_rd, prev_wr, &osd_op, flags] (auto outcome) -> call_errorator::future<> { auto& [ret, outdata] = outcome; + osd_op.rval = ret; + logger().debug("do_op_call: method returned ret={}, outdata.length()={}" " while num_read={}, num_write={}", ret, outdata.length(), num_read, num_write); if (num_read > prev_rd && !(flags & CLS_METHOD_RD)) { logger().error("method tried to read object but is not marked RD"); + osd_op.rval = -EIO; return crimson::ct_error::input_output_error::make(); } if (num_write > prev_wr && !(flags & CLS_METHOD_WR)) { logger().error("method tried to update object but is not marked WR"); + osd_op.rval = -EIO; return crimson::ct_error::input_output_error::make(); } - - // for write calls we never return data expect errors. For details refer - // to cls/cls_hello.cc. - if (ret < 0 || (flags & CLS_METHOD_WR) == 0) { - logger().debug("method called response length={}", outdata.length()); + // ceph-osd has this implemented in `PrimaryLogPG::execute_ctx`, + // grep for `ignore_out_data`. + using crimson::common::local_conf; + if (op_info->allows_returnvec() && + op_info->may_write() && + ret >= 0 && + outdata.length() > local_conf()->osd_max_write_op_reply_len) { + // the justification of this limit it to not inflate the pg log. + // that's the reason why we don't worry about pure reads. + logger().error("outdata overflow due to .length()={}, limit={}", + outdata.length(), + local_conf()->osd_max_write_op_reply_len); + osd_op.rval = -EOVERFLOW; + return crimson::ct_error::value_too_large::make(); + } + // for write calls we never return data expect errors or RETURNVEC. + // please refer cls/cls_hello.cc to details. + if (!op_info->may_write() || op_info->allows_returnvec() || ret < 0) { osd_op.op.extent.length = outdata.length(); osd_op.outdata.claim_append(outdata); } if (ret < 0) { - return crimson::stateful_ec{ std::error_code(-ret, std::generic_category()) }; + return crimson::stateful_ec{ + std::error_code(-ret, std::generic_category()) }; + } else { + return seastar::now(); } - return seastar::now(); } ); } diff --git a/src/crimson/osd/ops_executer.h b/src/crimson/osd/ops_executer.h index 390bf9e23dc..93fb96dd6e4 100644 --- a/src/crimson/osd/ops_executer.h +++ b/src/crimson/osd/ops_executer.h @@ -42,7 +42,8 @@ class OpsExecuter { crimson::ct_error::invarg, crimson::ct_error::permission_denied, crimson::ct_error::operation_not_supported, - crimson::ct_error::input_output_error>; + crimson::ct_error::input_output_error, + crimson::ct_error::value_too_large>; using read_errorator = PGBackend::read_errorator; using get_attr_errorator = PGBackend::get_attr_errorator; using watch_errorator = crimson::errorator< @@ -79,6 +80,7 @@ private: }; ObjectContextRef obc; + const OpInfo* op_info; PG& pg; PGBackend& backend; Ref msg; @@ -163,14 +165,15 @@ private: } public: - OpsExecuter(ObjectContextRef obc, PG& pg, Ref msg) + OpsExecuter(ObjectContextRef obc, const OpInfo* op_info, PG& pg, Ref msg) : obc(std::move(obc)), + op_info(op_info), pg(pg), backend(pg.get_backend()), msg(std::move(msg)) { } OpsExecuter(PG& pg, Ref msg) - : OpsExecuter{ObjectContextRef(), pg, std::move(msg)} + : OpsExecuter{ObjectContextRef(), nullptr, pg, std::move(msg)} {} osd_op_errorator::future<> execute_osd_op(class OSDOp& osd_op); diff --git a/src/crimson/osd/osd_operations/client_request.cc b/src/crimson/osd/osd_operations/client_request.cc index 3661d042734..72f9f4396b3 100644 --- a/src/crimson/osd/osd_operations/client_request.cc +++ b/src/crimson/osd/osd_operations/client_request.cc @@ -114,7 +114,7 @@ seastar::future<> ClientRequest::process_op( [this, &pg](auto obc) { return with_blocking_future(handle.enter(pp(pg).process) ).then([this, &pg, obc]() { - return pg.do_osd_ops(m, obc); + return pg.do_osd_ops(m, obc, op_info); }).then([this](Ref reply) { return conn->send(reply); }); diff --git a/src/crimson/osd/pg.cc b/src/crimson/osd/pg.cc index fcef9c45aa1..dcfa7c75006 100644 --- a/src/crimson/osd/pg.cc +++ b/src/crimson/osd/pg.cc @@ -524,13 +524,14 @@ seastar::future<> PG::submit_transaction(ObjectContextRef&& obc, seastar::future> PG::do_osd_ops( Ref m, - ObjectContextRef obc) + ObjectContextRef obc, + const OpInfo &op_info) { using osd_op_errorator = OpsExecuter::osd_op_errorator; const auto oid = m->get_snapid() == CEPH_SNAPDIR ? m->get_hobj().get_head() : m->get_hobj(); auto ox = - std::make_unique(obc, *this/* as const& */, m); + std::make_unique(obc, &op_info, *this/* as const& */, m); return crimson::do_for_each( m->ops, [obc, m, ox = ox.get()](OSDOp& osd_op) { @@ -563,9 +564,17 @@ seastar::future> PG::do_osd_ops( std::move(osd_op_p)); } }); - }).safe_then([m, obc, this, ox_deleter = std::move(ox)] { - auto reply = make_message(m.get(), 0, get_osdmap_epoch(), - 0, false); + }).safe_then([this, + m, + obc, + ox_deleter = std::move(ox), + rvec = op_info.allows_returnvec()] { + auto result = m->ops.empty() || !rvec ? 0 : m->ops.back().rval.code; + auto reply = make_message(m.get(), + result, + get_osdmap_epoch(), + 0, + false); reply->add_flags(CEPH_OSD_FLAG_ACK | CEPH_OSD_FLAG_ONDISK); logger().debug( "do_osd_ops: {} - object {} sending reply", diff --git a/src/crimson/osd/pg.h b/src/crimson/osd/pg.h index aa5945f1abe..a8f6ba4e3fb 100644 --- a/src/crimson/osd/pg.h +++ b/src/crimson/osd/pg.h @@ -481,7 +481,8 @@ private: PeeringCtx &rctx); seastar::future> do_osd_ops( Ref m, - ObjectContextRef obc); + ObjectContextRef obc, + const OpInfo &op_info); seastar::future> do_pg_ops(Ref m); seastar::future<> do_osd_op( ObjectState& os, diff --git a/src/test/cls_hello/test_cls_hello.cc b/src/test/cls_hello/test_cls_hello.cc index 4f1d90de4d3..cb05bb77bf5 100644 --- a/src/test/cls_hello/test_cls_hello.cc +++ b/src/test/cls_hello/test_cls_hello.cc @@ -118,7 +118,7 @@ TEST(ClsHello, WriteReturnData) { return; } - // this will return nothing -- not flag set + // this will return nothing -- no flag is set bufferlist in, out; ASSERT_EQ(0, ioctx.exec("myobject", "hello", "write_return_data", in, out)); ASSERT_EQ(std::string(), std::string(out.c_str(), out.length())); -- 2.39.5