From 325aae365244f5806fde310dae2a286abf8e6ac2 Mon Sep 17 00:00:00 2001 From: Greg Farnum Date: Wed, 9 Oct 2013 16:16:36 -0700 Subject: [PATCH] ReplicatedPG: copy: switch out the CopyCallback interface The tuple was already unwieldy with 4 members; I didn't want to add more. Instead, create a new CopyResults struct which contains all the object info and completion data, and pass the retval and a CopyResults* in the CopyCallbackResults tuple. Signed-off-by: Greg Farnum Reviewed-by: Sage Weil --- src/osd/ReplicatedPG.cc | 29 +++++++-------- src/osd/ReplicatedPG.h | 80 +++++++++++++++++++++++------------------ 2 files changed, 58 insertions(+), 51 deletions(-) diff --git a/src/osd/ReplicatedPG.cc b/src/osd/ReplicatedPG.cc index d94668a248fdf..5e7745a0bd550 100644 --- a/src/osd/ReplicatedPG.cc +++ b/src/osd/ReplicatedPG.cc @@ -4424,15 +4424,16 @@ void ReplicatedPG::_copy_some(ObjectContextRef obc, CopyOpRef cop) { dout(10) << __func__ << " " << obc << " " << cop << dendl; ObjectOperation op; - if (cop->user_version) { - op.assert_version(cop->user_version); + if (cop->results->user_version) { + op.assert_version(cop->results->user_version); } else { // we should learn the version after the first chunk, if we didn't know // it already! assert(cop->cursor.is_initial()); } op.copy_get(&cop->cursor, cct->_conf->osd_copyfrom_max_chunk, - &cop->size, &cop->mtime, &cop->category, + &cop->results->object_size, &cop->results->mtime, + &cop->results->category, &cop->attrs, &cop->data, &cop->omap, &cop->rval); @@ -4444,7 +4445,7 @@ void ReplicatedPG::_copy_some(ObjectContextRef obc, CopyOpRef cop) new C_OnFinisher(fin, &osd->objecter_finisher), // discover the object version if we don't know it yet - cop->user_version ? NULL : &cop->user_version); + cop->results->user_version ? NULL : &cop->results->user_version); fin->tid = tid; cop->objecter_tid = tid; osd->objecter_lock.Unlock(); @@ -4467,7 +4468,6 @@ void ReplicatedPG::process_copy_chunk(hobject_t oid, tid_t tid, int r) cop->objecter_tid = 0; ObjectContextRef& cobc = cop->obc; - CopyResults results; if (r >= 0) { assert(cop->rval >= 0); @@ -4486,13 +4486,11 @@ void ReplicatedPG::process_copy_chunk(hobject_t oid, tid_t tid, int r) _copy_some(cobc, cop); return; } - _build_finish_copy_transaction(cop, results.get<3>()); - results.get<1>() = cop->temp_cursor.data_offset; + _build_finish_copy_transaction(cop, cop->results->final_tx); } dout(20) << __func__ << " complete; committing" << dendl; - results.get<0>() = r; - results.get<4>() = false; + CopyCallbackResults results(r, cop->results); cop->cb->complete(results); copy_ops.erase(cobc->obs.oi.soid); @@ -4559,8 +4557,8 @@ int ReplicatedPG::finish_copyfrom(OpContext *ctx) if (cb->is_temp_obj_used()) { ctx->discard_temp_oid = cb->temp_obj; } - ctx->op_t.swap(cb->results.get<3>()); - ctx->op_t.append(cb->results.get<3>()); + ctx->op_t.swap(cb->results->final_tx); + ctx->op_t.append(cb->results->final_tx); interval_set ch; if (obs.oi.size > 0) @@ -4581,8 +4579,8 @@ int ReplicatedPG::finish_copyfrom(OpContext *ctx) void ReplicatedPG::cancel_copy(CopyOpRef cop, bool requeue) { dout(10) << __func__ << " " << cop->obc->obs.oi.soid - << " from " << cop->src << " " << cop->oloc << " v" << cop->user_version - << dendl; + << " from " << cop->src << " " << cop->oloc + << " v" << cop->results->user_version << dendl; // cancel objecter op, if we can if (cop->objecter_tid) { @@ -4594,9 +4592,8 @@ void ReplicatedPG::cancel_copy(CopyOpRef cop, bool requeue) --cop->obc->copyfrom_readside; kick_object_context_blocked(cop->obc); - bool temp_obj_created = !cop->cursor.is_initial(); - CopyResults result(-ECANCELED, 0, temp_obj_created, - ObjectStore::Transaction(), requeue); + cop->results->should_requeue = requeue; + CopyCallbackResults result(-ECANCELED, cop->results); cop->cb->complete(result); } diff --git a/src/osd/ReplicatedPG.h b/src/osd/ReplicatedPG.h index 0b0f0591fcab0..57be318331ed7 100644 --- a/src/osd/ReplicatedPG.h +++ b/src/osd/ReplicatedPG.h @@ -97,19 +97,36 @@ public: struct OpContext; class CopyCallback; + /** + * CopyResults stores the object metadata of interest to a copy initiator. + */ + struct CopyResults { + utime_t mtime; ///< the copy source's mtime + uint64_t object_size; ///< the copied object's size + bool started_temp_obj; ///< true if the callback needs to delete temp object + /** + * Final transaction; if non-empty the callback must execute it before any + * other accesses to the object (in order to complete the copy). + */ + ObjectStore::Transaction final_tx; + string category; ///< The copy source's category + version_t user_version; ///< The copy source's user version + bool should_requeue; ///< op should be requeued on cancel + CopyResults() : object_size(0), started_temp_obj(false), + user_version(0), should_requeue(false) {} + }; + struct CopyOp { CopyCallback *cb; ObjectContextRef obc; hobject_t src; object_locator_t oloc; - version_t user_version; + + CopyResults *results; tid_t objecter_tid; object_copy_cursor_t cursor; - uint64_t size; - utime_t mtime; - string category; map attrs; bufferlist data; map omap; @@ -121,12 +138,15 @@ public: CopyOp(CopyCallback *cb_, ObjectContextRef _obc, hobject_t s, object_locator_t l, version_t v, const hobject_t& dest) - : cb(cb_), obc(_obc), src(s), oloc(l), user_version(v), + : cb(cb_), obc(_obc), src(s), oloc(l), + results(NULL), objecter_tid(0), - size(0), rval(-1), temp_oid(dest) - {} + { + results = new CopyResults(); + results->user_version = v; + } }; typedef boost::shared_ptr CopyOpRef; @@ -136,32 +156,19 @@ public: * one and give an instance of the class to start_copy. * * The implementer is responsible for making sure that the CopyCallback - * can associate itself with the correct copy operation. The presence - * of the closing Transaction ensures that write operations can be performed - * atomically with the copy being completed (which doing them in separate - * transactions would not allow); if you are doing the copy for a read - * op you will have to generate a separate op to finish the copy with. + * can associate itself with the correct copy operation. */ - /// return code, total object size, data in temp object?, final Transaction, should requeue Op - typedef boost::tuple CopyResults; - class CopyCallback : public GenContext { + typedef boost::tuple CopyCallbackResults; + class CopyCallback : public GenContext { protected: CopyCallback() {} /** * results.get<0>() is the return code: 0 for success; -ECANCELLED if * the operation was cancelled by the local OSD; -errno for other issues. - * results.get<1>() is the total size of the object (for updating pg stats) - * results.get<2>() indicates whether we have already written data to - * the temp object (so it needs to get cleaned up, if the return code - * indicates a failure) - * results.get<3>() is a Transaction; if non-empty you need to perform - * its results before any other accesses to the object in order to - * complete the copy. - * results.get<4>() is a bool; if true you must requeue the client Op - * after processing the rest of the results (this will only be true - * in conjunction with an ECANCELED return code). + * results.get<1>() is a pointer to a CopyResults object, which you are + * responsible for deleting. */ - virtual void finish(CopyResults& results_) = 0; + virtual void finish(CopyCallbackResults results_) = 0; public: /// Provide the final size of the copied object to the CopyCallback @@ -170,16 +177,18 @@ public: class CopyFromCallback: public CopyCallback { public: - CopyResults results; + CopyResults *results; + int retval; OpContext *ctx; hobject_t temp_obj; CopyFromCallback(OpContext *ctx_, const hobject_t& temp_obj_) : - ctx(ctx_), temp_obj(temp_obj_) {} + results(NULL), retval(0), ctx(ctx_), temp_obj(temp_obj_) {} ~CopyFromCallback() {} - virtual void finish(CopyResults& results_) { - results = results_; - int r = results.get<0>(); + virtual void finish(CopyCallbackResults results_) { + results = results_.get<1>(); + int r = results_.get<0>(); + retval = r; if (r >= 0) { ctx->pg->execute_ctx(ctx); } @@ -187,16 +196,17 @@ public: if (r < 0) { if (r != -ECANCELED) { // on cancel just toss it out; client resends ctx->pg->osd->reply_op_error(ctx->op, r); - } else if (results_.get<4>()) { + } else if (results->should_requeue) { ctx->pg->requeue_op(ctx->op); } ctx->pg->close_op_ctx(ctx); } + delete results; } - bool is_temp_obj_used() { return results.get<2>(); } - uint64_t get_data_size() { return results.get<1>(); } - int get_result() { return results.get<0>(); } + bool is_temp_obj_used() { return results->started_temp_obj; } + uint64_t get_data_size() { return results->object_size; } + int get_result() { return retval; } }; friend class CopyFromCallback; -- 2.39.5