src/osd: use unique_ptr for backend trasaction, move into submit_transaction

submit_transaction takes ownership of the transaction implicitely.  Make
this implicit using an rvalue ref to force usage of std::move and move
constructor.

Signed-off-by: Samuel Just <sjust@redhat.com>

diff --git a/src/osd/ECBackend.cc b/src/osd/ECBackend.cc
index feefcd7dc273dd98f22f504ea2cb962650ccb5a1..c87e6d22af0169dcaa80238f35e4b428743b95ef 100644 (file)
--- a/src/osd/ECBackend.cc
+++ b/src/osd/ECBackend.cc
@@ -858,7 +858,7 @@ void ECBackend::handle_sub_write(
     op.trim_to,
     op.trim_rollback_to,
     !(op.t.empty()),
-    &localt);
+    localt);
 
   ReplicatedPG *_rPG = dynamic_cast<ReplicatedPG *>(get_parent());
   if (_rPG && !_rPG->is_undersized() &&
@@ -1342,7 +1342,7 @@ struct MustPrependHashInfo : public ObjectModDesc::Visitor {
 void ECBackend::submit_transaction(
   const hobject_t &hoid,
   const eversion_t &at_version,
-  PGTransaction *_t,
+  PGTransactionUPtr &&_t,
   const eversion_t &trim_to,
   const eversion_t &trim_rollback_to,
   const vector<pg_log_entry_t> &log_entries,
@@ -1370,7 +1370,7 @@ void ECBackend::submit_transaction(
   op->reqid = reqid;
   op->client_op = client_op;
   
-  op->t = static_cast<ECTransaction*>(_t);
+  op->t.reset(static_cast<ECTransaction*>(_t.release()));
 
   set<hobject_t, hobject_t::BitwiseComparator> need_hinfos;
   op->t->get_append_objects(&need_hinfos);

diff --git a/src/osd/ECBackend.h b/src/osd/ECBackend.h
index bee32fd084685d57d533ca51a6e4bc12271760e5..b691658947b2ef78a83a6585d4f66def44361fe4 100644 (file)
--- a/src/osd/ECBackend.h
+++ b/src/osd/ECBackend.h
@@ -95,7 +95,7 @@ public:
   void submit_transaction(
     const hobject_t &hoid,
     const eversion_t &at_version,
-    PGTransaction *t,
+    PGTransactionUPtr &&t,
     const eversion_t &trim_to,
     const eversion_t &trim_rollback_to,
     const vector<pg_log_entry_t> &log_entries,
@@ -359,7 +359,7 @@ public:
     osd_reqid_t reqid;
     OpRequestRef client_op;
 
-    ECTransaction *t;
+    std::unique_ptr<ECTransaction> t;
 
     set<hobject_t, hobject_t::BitwiseComparator> temp_added;
     set<hobject_t, hobject_t::BitwiseComparator> temp_cleared;
@@ -369,7 +369,6 @@ public:
 
     map<hobject_t, ECUtil::HashInfoRef, hobject_t::BitwiseComparator> unstable_hash_infos;
     ~Op() {
-      delete t;
       delete on_local_applied_sync;
       delete on_all_applied;
       delete on_all_commit;

diff --git a/src/osd/PGBackend.h b/src/osd/PGBackend.h
index 829c810d6510c7afc3ac440461d3c7790b1daf01..1af8a0c4c867aad8befd79ed2b38afb9c7de377f 100644 (file)
--- a/src/osd/PGBackend.h
+++ b/src/osd/PGBackend.h
@@ -186,7 +186,7 @@ struct shard_info_wrapper;
        const eversion_t &trim_to,
        const eversion_t &trim_rollback_to,
        bool transaction_applied,
-       ObjectStore::Transaction *t) = 0;
+       ObjectStore::Transaction &t) = 0;
 
      virtual void update_peer_last_complete_ondisk(
        pg_shard_t fromosd,
@@ -472,6 +472,8 @@ struct shard_info_wrapper;
      virtual uint64_t get_bytes_written() const = 0;
      virtual ~PGTransaction() {}
    };
+   using PGTransactionUPtr = std::unique_ptr<PGTransaction>;
+
    /// Get implementation specific empty transaction
    virtual PGTransaction *get_transaction() = 0;
 
@@ -479,7 +481,7 @@ struct shard_info_wrapper;
    virtual void submit_transaction(
      const hobject_t &hoid,               ///< [in] object
      const eversion_t &at_version,        ///< [in] version
-     PGTransaction *t,                    ///< [in] trans to execute
+     PGTransactionUPtr &&t,               ///< [in] trans to execute (move)
      const eversion_t &trim_to,           ///< [in] trim log to here
      const eversion_t &trim_rollback_to,  ///< [in] trim rollback info to here
      const vector<pg_log_entry_t> &log_entries, ///< [in] log entries for t

diff --git a/src/osd/ReplicatedBackend.cc b/src/osd/ReplicatedBackend.cc
index bef96d68747fbf9a1d9491588748ade15c04940d..4cb48ab72a0fd211e8ff86e28e271dd287b0b045 100644 (file)
--- a/src/osd/ReplicatedBackend.cc
+++ b/src/osd/ReplicatedBackend.cc
@@ -549,7 +549,7 @@ public:
 void ReplicatedBackend::submit_transaction(
   const hobject_t &soid,
   const eversion_t &at_version,
-  PGTransaction *_t,
+  PGTransactionUPtr &&_t,
   const eversion_t &trim_to,
   const eversion_t &trim_rollback_to,
   const vector<pg_log_entry_t> &log_entries,
@@ -561,7 +561,8 @@ void ReplicatedBackend::submit_transaction(
   osd_reqid_t reqid,
   OpRequestRef orig_op)
 {
-  RPGTransaction *t = dynamic_cast<RPGTransaction*>(_t);
+  std::unique_ptr<RPGTransaction> t(
+    static_cast<RPGTransaction*>(_t.release()));
   assert(t);
   ObjectStore::Transaction op_t = t->get_transaction();
 
@@ -599,7 +600,7 @@ void ReplicatedBackend::submit_transaction(
     log_entries,
     hset_history,
     &op,
-    &op_t);
+    op_t);
 
   if (!(t->get_temp_added().empty())) {
     add_temp_objs(t->get_temp_added());
@@ -612,7 +613,7 @@ void ReplicatedBackend::submit_transaction(
     trim_to,
     trim_rollback_to,
     true,
-    &op_t);
+    op_t);
   
   op_t.register_on_applied_sync(on_local_applied_sync);
   op_t.register_on_applied(
@@ -621,10 +622,11 @@ void ReplicatedBackend::submit_transaction(
   op_t.register_on_commit(
     parent->bless_context(
       new C_OSD_OnOpCommit(this, &op)));
+
   vector<ObjectStore::Transaction> tls;
   tls.push_back(std::move(op_t));
+
   parent->queue_transactions(tls, op.op);
-  delete t;
 }
 
 void ReplicatedBackend::op_applied(
@@ -993,7 +995,7 @@ Message * ReplicatedBackend::generate_subop(
   const vector<pg_log_entry_t> &log_entries,
   boost::optional<pg_hit_set_history_t> &hset_hist,
   InProgressOp *op,
-  ObjectStore::Transaction *op_t,
+  ObjectStore::Transaction &op_t,
   pg_shard_t peer,
   const pg_info_t &pinfo)
 {
@@ -1015,10 +1017,10 @@ Message * ReplicatedBackend::generate_subop(
             << ", pinfo.last_backfill "
             << pinfo.last_backfill << ")" << dendl;
     ObjectStore::Transaction t;
-    t.set_use_tbl(op_t->get_use_tbl());
+    t.set_use_tbl(op_t.get_use_tbl());
     ::encode(t, wr->get_data());
   } else {
-    ::encode(*op_t, wr->get_data());
+    ::encode(op_t, wr->get_data());
   }
 
   ::encode(log_entries, wr->logbl);
@@ -1049,7 +1051,7 @@ void ReplicatedBackend::issue_op(
   const vector<pg_log_entry_t> &log_entries,
   boost::optional<pg_hit_set_history_t> &hset_hist,
   InProgressOp *op,
-  ObjectStore::Transaction *op_t)
+  ObjectStore::Transaction &op_t)
 {
 
   if (parent->get_actingbackfill_shards().size() > 1) {
@@ -1198,7 +1200,7 @@ void ReplicatedBackend::sub_op_modify_impl(OpRequestRef op)
     m->pg_trim_to,
     m->pg_trim_rollback_to,
     update_snaps,
-    &(rm->localt));
+    rm->localt);
 
   rm->opt.register_on_commit(
     parent->bless_context(

diff --git a/src/osd/ReplicatedBackend.h b/src/osd/ReplicatedBackend.h
index 1f04150e476b0e1830f31118c3fea1ec7efa4704..7961aa4acffe62350d832631322c5119ea7fc224 100644 (file)
--- a/src/osd/ReplicatedBackend.h
+++ b/src/osd/ReplicatedBackend.h
@@ -347,7 +347,7 @@ public:
   void submit_transaction(
     const hobject_t &hoid,
     const eversion_t &at_version,
-    PGTransaction *t,
+    PGTransactionUPtr &&t,
     const eversion_t &trim_to,
     const eversion_t &trim_rollback_to,
     const vector<pg_log_entry_t> &log_entries,
@@ -374,7 +374,7 @@ private:
     const vector<pg_log_entry_t> &log_entries,
     boost::optional<pg_hit_set_history_t> &hset_history,
     InProgressOp *op,
-    ObjectStore::Transaction *op_t,
+    ObjectStore::Transaction &op_t,
     pg_shard_t peer,
     const pg_info_t &pinfo);
   void issue_op(
@@ -389,7 +389,7 @@ private:
     const vector<pg_log_entry_t> &log_entries,
     boost::optional<pg_hit_set_history_t> &hset_history,
     InProgressOp *op,
-    ObjectStore::Transaction *op_t);
+    ObjectStore::Transaction &op_t);
   void op_applied(InProgressOp *op);
   void op_commit(InProgressOp *op);
   template<typename T, int MSGTYPE>

diff --git a/src/osd/ReplicatedPG.cc b/src/osd/ReplicatedPG.cc
index 19056260e577a1e39fd26a693d90035ed1226b6e..a06c5d1a7d40421a32f8237795594d3bee589a0d 100644 (file)
--- a/src/osd/ReplicatedPG.cc
+++ b/src/osd/ReplicatedPG.cc
@@ -2810,8 +2810,7 @@ void ReplicatedPG::execute_ctx(OpContext *ctx)
 
   // this method must be idempotent since we may call it several times
   // before we finally apply the resulting transaction.
-  delete ctx->op_t;
-  ctx->op_t = pgbackend->get_transaction();
+  ctx->op_t.reset(pgbackend->get_transaction());
 
   if (op->may_write() || op->may_cache()) {
     // snap
@@ -3364,7 +3363,7 @@ ReplicatedPG::OpContextUPtr ReplicatedPG::trim_object(const hobject_t &coid)
   ctx->release_snapset_obc = true;
   ctx->at_version = get_next_version();
 
-  PGBackend::PGTransaction *t = ctx->op_t;
+  PGBackend::PGTransaction *t = ctx->op_t.get();
  
   if (new_snaps.empty()) {
     // remove clone
@@ -4035,7 +4034,7 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
 
   bool first_read = true;
 
-  PGBackend::PGTransaction* t = ctx->op_t;
+  PGBackend::PGTransaction* t = ctx->op_t.get();
 
   dout(10) << "do_osd_op " << soid << " " << ops << dendl;
 
@@ -5955,7 +5954,7 @@ inline int ReplicatedPG::_delete_oid(OpContext *ctx, bool no_whiteout)
   ObjectState& obs = ctx->new_obs;
   object_info_t& oi = obs.oi;
   const hobject_t& soid = oi.soid;
-  PGBackend::PGTransaction* t = ctx->op_t;
+  PGBackend::PGTransaction* t = ctx->op_t.get();
 
   if (!obs.exists || (obs.oi.is_whiteout() && !no_whiteout))
     return -ENOENT;
@@ -6029,7 +6028,7 @@ int ReplicatedPG::_rollback_to(OpContext *ctx, ceph_osd_op& op)
   ObjectState& obs = ctx->new_obs;
   object_info_t& oi = obs.oi;
   const hobject_t& soid = oi.soid;
-  PGBackend::PGTransaction* t = ctx->op_t;
+  PGBackend::PGTransaction* t = ctx->op_t.get();
   snapid_t snapid = (uint64_t)op.snap.snapid;
   hobject_t missing_oid;
 
@@ -6276,9 +6275,8 @@ void ReplicatedPG::make_writeable(OpContext *ctx)
     // prepend transaction to op_t
     PGBackend::PGTransaction *t = pgbackend->get_transaction();
     _make_clone(ctx, t, ctx->clone_obc, soid, coid, snap_oi);
-    t->append(ctx->op_t);
-    delete ctx->op_t;
-    ctx->op_t = t;
+    t->append(ctx->op_t.get());
+    ctx->op_t.reset(t);
     
     ctx->delta_stats.num_objects++;
     if (snap_oi->is_dirty()) {
@@ -6628,7 +6626,7 @@ void ReplicatedPG::finish_ctx(OpContext *ctx, int log_op_type, bool maintain_ssc
       ctx->op_t->touch(snapoid);
       attrs[OI_ATTR].claim(bv);
       attrs[SS_ATTR].claim(bss);
-      setattrs_maybe_cache(ctx->snapset_obc, ctx, ctx->op_t, attrs);
+      setattrs_maybe_cache(ctx->snapset_obc, ctx, ctx->op_t.get(), attrs);
       if (pool.info.require_rollback()) {
        map<string, boost::optional<bufferlist> > to_set;
        to_set[SS_ATTR];
@@ -6680,7 +6678,7 @@ void ReplicatedPG::finish_ctx(OpContext *ctx, int log_op_type, bool maintain_ssc
     } else {
       dout(10) << " no snapset (this is a clone)" << dendl;
     }
-    setattrs_maybe_cache(ctx->obc, ctx, ctx->op_t, attrs);
+    setattrs_maybe_cache(ctx->obc, ctx, ctx->op_t.get(), attrs);
 
     if (pool.info.require_rollback()) {
       set<string> changing;
@@ -7216,7 +7214,7 @@ void ReplicatedPG::process_copy_chunk(hobject_t oid, ceph_tid_t tid, int r)
     if (cop->temp_cursor.is_initial()) {
       ctx->new_temp_oid = cop->results.temp_oid;
     }
-    _write_copy_chunk(cop, ctx->op_t);
+    _write_copy_chunk(cop, ctx->op_t.get());
     simple_opc_submit(std::move(ctx));
     dout(10) << __func__ << " fetching more" << dendl;
     _copy_some(cobc, cop);
@@ -8403,7 +8401,7 @@ void ReplicatedPG::issue_repop(RepGather *repop)
   pgbackend->submit_transaction(
     soid,
     repop->ctx->at_version,
-    repop->ctx->op_t,
+    std::move(repop->ctx->op_t),
     pg_trim_to,
     min_last_complete_ondisk,
     repop->ctx->log,
@@ -8414,7 +8412,6 @@ void ReplicatedPG::issue_repop(RepGather *repop)
     repop->rep_tid,
     repop->ctx->reqid,
     repop->ctx->op);
-  repop->ctx->op_t = NULL;
 }
 
 ReplicatedPG::RepGather *ReplicatedPG::new_repop(
@@ -8467,7 +8464,7 @@ ReplicatedPG::OpContextUPtr ReplicatedPG::simple_opc_create(ObjectContextRef obc
   ceph_tid_t rep_tid = osd->get_tid();
   osd_reqid_t reqid(osd->get_cluster_msgr_name(), 0, rep_tid);
   OpContextUPtr ctx(new OpContext(OpRequestRef(), reqid, ops, obc, this));
-  ctx->op_t = pgbackend->get_transaction();
+  ctx->op_t.reset(pgbackend->get_transaction());
   ctx->mtime = ceph_clock_now(g_ceph_context);
   return ctx;
 }
@@ -8618,7 +8615,7 @@ void ReplicatedPG::handle_watch_timeout(WatchRef watch)
     });
 
 
-  PGBackend::PGTransaction *t = ctx->op_t;
+  PGBackend::PGTransaction *t = ctx->op_t.get();
   ctx->log.push_back(pg_log_entry_t(pg_log_entry_t::MODIFY, obc->obs.oi.soid,
                                    ctx->at_version,
                                    oi.version,
@@ -11399,7 +11396,7 @@ void ReplicatedPG::hit_set_persist()
   map <string, bufferlist> attrs;
   attrs[OI_ATTR].claim(boi);
   attrs[SS_ATTR].claim(bss);
-  setattrs_maybe_cache(ctx->obc, ctx.get(), ctx->op_t, attrs);
+  setattrs_maybe_cache(ctx->obc, ctx.get(), ctx->op_t.get(), attrs);
   ctx->log.push_back(
     pg_log_entry_t(
       pg_log_entry_t::MODIFY,

diff --git a/src/osd/ReplicatedPG.h b/src/osd/ReplicatedPG.h
index 9aa57dc4cb4fc12822b232e08af6b4221be2198b..60558f7be38af5a34e8c77bc1036f8a181bfb12c 100644 (file)
--- a/src/osd/ReplicatedPG.h
+++ b/src/osd/ReplicatedPG.h
@@ -389,12 +389,12 @@ public:
     const eversion_t &trim_to,
     const eversion_t &trim_rollback_to,
     bool transaction_applied,
-    ObjectStore::Transaction *t) {
+    ObjectStore::Transaction &t) {
     if (hset_history) {
       info.hit_set = *hset_history;
       dirty_info = true;
     }
-    append_log(logv, trim_to, trim_rollback_to, *t, transaction_applied);
+    append_log(logv, trim_to, trim_rollback_to, t, transaction_applied);
   }
 
   void op_applied(
@@ -531,7 +531,7 @@ public:
 
     int current_osd_subop_num;
 
-    PGBackend::PGTransaction *op_t;
+    PGBackend::PGTransactionUPtr op_t;
     vector<pg_log_entry_t> log;
     boost::optional<pg_hit_set_history_t> updated_hset_history;
 
@@ -640,7 +640,6 @@ public:
       ignore_cache(false), ignore_log_op_stats(false),
       bytes_written(0), bytes_read(0), user_at_version(0),
       current_osd_subop_num(0),
-      op_t(NULL),
       obc(obc),
       data_off(0), reply(NULL), pg(_pg),
       num_read(0),
@@ -663,7 +662,6 @@ public:
       ignore_cache(false), ignore_log_op_stats(false),
       bytes_written(0), bytes_read(0), user_at_version(0),
       current_osd_subop_num(0),
-      op_t(NULL),
       data_off(0), reply(NULL), pg(_pg),
       num_read(0),
       num_write(0),
@@ -808,8 +806,7 @@ protected:
    */
   void close_op_ctx(OpContext *ctx, int r) {
     release_op_ctx_locks(ctx);
-    delete ctx->op_t;
-    ctx->op_t = NULL;
+    ctx->op_t.reset();
     for (auto p = ctx->on_finish.begin();
         p != ctx->on_finish.end();
         ctx->on_finish.erase(p++)) {