OSD,ReplicatedPG: let PGBackend handle the temp collection

Signed-off-by: Samuel Just <sam.just@inktank.com>

7 files changed, 69 insertions, 77 deletions

diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index 1cdc232b064..85cf817adc5 100644
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -3402,16 +3402,16 @@ void OSD::RemoveWQ::_process(pair<PGRef, DeletingStateRef> item)
   if (!item.second->start_clearing())
     return;
 
-  if (pg->have_temp_coll()) {
+  list<coll_t> colls_to_remove;
+  pg->get_colls(&colls_to_remove);
+  for (list<coll_t>::iterator i = colls_to_remove.begin();
+       i != colls_to_remove.end();
+       ++i) {
     bool cont = remove_dir(
-      pg->cct, store, &mapper, &driver, pg->osr.get(), pg->get_temp_coll(), item.second);
+      pg->cct, store, &mapper, &driver, pg->osr.get(), *i, item.second);
     if (!cont)
       return;
   }
-  bool cont = remove_dir(
-      pg->cct, store, &mapper, &driver, pg->osr.get(), coll, item.second);
-  if (!cont)
-    return;
 
   if (!item.second->start_deleting())
     return;
@@ -3422,9 +3422,12 @@ void OSD::RemoveWQ::_process(pair<PGRef, DeletingStateRef> item)
     OSD::make_infos_oid(),
     pg->log_oid,
     t);
-  if (pg->have_temp_coll())
-    t->remove_collection(pg->get_temp_coll());
-  t->remove_collection(coll);
+
+  for (list<coll_t>::iterator i = colls_to_remove.begin();
+       i != colls_to_remove.end();
+       ++i) {
+    t->remove_collection(*i);
+  }
 
   // We need the sequencer to stick around until the op is complete
   store->queue_transaction(
@@ -5879,22 +5882,11 @@ void OSD::split_pgs(
     dout(10) << "m_seed " << i->ps() << dendl;
     dout(10) << "split_bits is " << split_bits << dendl;
 
-    rctx->transaction->create_collection(
-      coll_t(*i));
-    rctx->transaction->split_collection(
-      coll_t(parent->info.pgid),
+    parent->split_colls(
+      *i,
       split_bits,
       i->m_seed,
-      coll_t(*i));
-    if (parent->have_temp_coll()) {
-      rctx->transaction->create_collection(
-	coll_t::make_temp_coll(*i));
-      rctx->transaction->split_collection(
-	coll_t::make_temp_coll(parent->info.pgid),
-	split_bits,
-	i->m_seed,
-	coll_t::make_temp_coll(*i));
-    }
+      rctx->transaction);
     parent->split_into(
       *i,
       child,
diff --git a/src/osd/PG.h b/src/osd/PG.h
index cdbe827a4a9..b869a0e5e23 100644
--- a/src/osd/PG.h
+++ b/src/osd/PG.h
@@ -870,8 +870,12 @@ public:
   virtual void _scrub(ScrubMap &map) { }
   virtual void _scrub_clear_state() { }
   virtual void _scrub_finish() { }
-  virtual coll_t get_temp_coll() = 0;
-  virtual bool have_temp_coll() = 0;
+  virtual void get_colls(list<coll_t> *out) = 0;
+  virtual void split_colls(
+    pg_t child,
+    int split_bits,
+    int seed,
+    ObjectStore::Transaction *t) = 0;
   virtual bool _report_snap_collection_errors(
     const hobject_t &hoid,
     const map<string, bufferptr> &attrs,
diff --git a/src/osd/PGBackend.h b/src/osd/PGBackend.h
index 6a77c72438d..b17f0542d55 100644
--- a/src/osd/PGBackend.h
+++ b/src/osd/PGBackend.h
@@ -188,6 +188,10 @@
 
    virtual void dump_recovery_info(Formatter *f) const = 0;
 
+   virtual coll_t get_temp_coll(ObjectStore::Transaction *t) = 0;
+   virtual void add_temp_obj(const hobject_t &oid) = 0;
+   virtual void clear_temp_obj(const hobject_t &oid) = 0;
+
    virtual ~PGBackend() {}
  };
 
diff --git a/src/osd/ReplicatedBackend.cc b/src/osd/ReplicatedBackend.cc
index ecbfea9149b..d020b18d901 100644
--- a/src/osd/ReplicatedBackend.cc
+++ b/src/osd/ReplicatedBackend.cc
@@ -28,7 +28,9 @@ static ostream& _prefix(std::ostream *_dout, ReplicatedBackend *pgb) {
 
 ReplicatedBackend::ReplicatedBackend(
   PGBackend::Listener *pg, coll_t coll, OSDService *osd) :
-  PGBackend(pg), temp_created(false), coll(coll), osd(osd) {}
+  PGBackend(pg), temp_created(false),
+  temp_coll(coll_t::make_temp_coll(pg->get_info().pgid)),
+  coll(coll), osd(osd), cct(osd->cct) {}
 
 void ReplicatedBackend::run_recovery_op(
   PGBackend::RecoveryHandle *h,
diff --git a/src/osd/ReplicatedBackend.h b/src/osd/ReplicatedBackend.h
index f2a7e4ca9e0..bcd1239c626 100644
--- a/src/osd/ReplicatedBackend.h
+++ b/src/osd/ReplicatedBackend.h
@@ -27,8 +27,7 @@ class ReplicatedBackend : public PGBackend {
   };
 private:
   bool temp_created;
-  coll_t temp_coll;
-  coll_t get_temp_coll(ObjectStore::Transaction *t);
+  const coll_t temp_coll;
   coll_t get_temp_coll() const {
     return temp_coll;
   }
@@ -39,6 +38,7 @@ private:
 public:
   coll_t coll;
   OSDService *osd;
+  CephContext *cct;
 
   ReplicatedBackend(PGBackend::Listener *pg, coll_t coll, OSDService *osd);
 
@@ -78,14 +78,15 @@ public:
     int split_bits,
     int seed,
     ObjectStore::Transaction *t) {
+    coll_t target = coll_t::make_temp_coll(child);
     if (!temp_created)
       return;
-    t->create_collection(temp_coll);
+    t->create_collection(target);
     t->split_collection(
       temp_coll,
       split_bits,
       seed,
-      coll_t::make_temp_coll(child));
+      target);
   }
 
   virtual void dump_recovery_info(Formatter *f) const {
@@ -186,6 +187,15 @@ private:
       return recovery_progress.is_complete(recovery_info);
     }
   };
+
+  coll_t get_temp_coll(ObjectStore::Transaction *t);
+  void add_temp_obj(const hobject_t &oid) {
+    temp_contents.insert(oid);
+  }
+  void clear_temp_obj(const hobject_t &oid) {
+    temp_contents.erase(oid);
+  }
+
   map<hobject_t, PullInfo> pulling;
 
   // Reverse mapping from osd peer to objects beging pulled from that peer
diff --git a/src/osd/ReplicatedPG.cc b/src/osd/ReplicatedPG.cc
index e2f2423d792..85ed07845b0 100644
--- a/src/osd/ReplicatedPG.cc
+++ b/src/osd/ReplicatedPG.cc
@@ -630,8 +630,6 @@ ReplicatedPG::ReplicatedPG(OSDService *o, OSDMapRef curmap,
   PG(o, curmap, _pool, p, oid, ioid),
   pgbackend(new ReplicatedBackend(this, coll_t(p), o)),
   snapset_contexts_lock("ReplicatedPG::snapset_contexts"),
-  temp_created(false),
-  temp_coll(coll_t::make_temp_coll(p)),
   temp_seq(0),
   snap_trimmer_machine(this)
 { 
@@ -3968,19 +3966,9 @@ void ReplicatedPG::do_osd_op_effects(OpContext *ctx)
   }
 }
 
-bool ReplicatedPG::have_temp_coll()
-{
-  return temp_created || osd->store->collection_exists(temp_coll);
-}
-
 coll_t ReplicatedPG::get_temp_coll(ObjectStore::Transaction *t)
 {
-  if (temp_created)
-    return temp_coll;
-  if (!osd->store->collection_exists(temp_coll))
-      t->create_collection(temp_coll);
-  temp_created = true;
-  return temp_coll;
+  return pgbackend->get_temp_coll(t);
 }
 
 hobject_t ReplicatedPG::generate_temp_object()
@@ -3988,6 +3976,7 @@ hobject_t ReplicatedPG::generate_temp_object()
   ostringstream ss;
   ss << "temp_" << info.pgid << "_" << get_role() << "_" << osd->monc->get_global_id() << "_" << (++temp_seq);
   hobject_t hoid(object_t(ss.str()), "", CEPH_NOSNAP, 0, -1, "");
+  pgbackend->add_temp_obj(hoid);
   dout(20) << __func__ << " " << hoid << dendl;
   return hoid;
 }
@@ -4253,7 +4242,6 @@ void ReplicatedPG::process_copy_chunk(hobject_t oid, tid_t tid, int r)
     if (cop->temp_cursor.is_initial()) {
       cop->temp_coll = get_temp_coll(&tctx->local_t);
       cop->temp_oid = generate_temp_object();
-      temp_contents.insert(cop->temp_oid);
       repop->ctx->new_temp_oid = cop->temp_oid;
     }
 
@@ -4322,7 +4310,7 @@ int ReplicatedPG::finish_copy(OpContext *ctx)
     // finish writing to temp object, then move into place
     _write_copy_chunk(cop, &t);
     t.collection_move_rename(cop->temp_coll, cop->temp_oid, coll, obs.oi.soid);
-    temp_contents.erase(cop->temp_oid);
+    pgbackend->clear_temp_obj(cop->temp_oid);
     ctx->discard_temp_oid = cop->temp_oid;
   }
 
@@ -5372,12 +5360,12 @@ void ReplicatedPG::sub_op_modify(OpRequestRef op)
 
     if (m->new_temp_oid != hobject_t()) {
       dout(20) << __func__ << " start tracking temp " << m->new_temp_oid << dendl;
-      temp_contents.insert(m->new_temp_oid);
+      pgbackend->add_temp_obj(m->new_temp_oid);
       get_temp_coll(&rm->localt);
     }
     if (m->discard_temp_oid != hobject_t()) {
       dout(20) << __func__ << " stop tracking temp " << m->discard_temp_oid << dendl;
-      temp_contents.erase(m->discard_temp_oid);
+      pgbackend->clear_temp_obj(m->discard_temp_oid);
     }
 
     ::decode(rm->opt, p);
@@ -7146,20 +7134,6 @@ void ReplicatedPG::on_shutdown()
   cancel_recovery();
 }
 
-void ReplicatedPG::on_flushed()
-{
-  assert(object_contexts.empty());
-  if (have_temp_coll() &&
-      !osd->store->collection_empty(get_temp_coll())) {
-    vector<hobject_t> objects;
-    osd->store->collection_list(get_temp_coll(), objects);
-    derr << __func__ << ": found objects in the temp collection: "
-	 << objects << ", crashing now"
-	 << dendl;
-    assert(0 == "found garbage in the temp collection");
-  }
-}
-
 void ReplicatedPG::on_activate()
 {
   for (unsigned i = 1; i<acting.size(); i++) {
@@ -7228,16 +7202,6 @@ void ReplicatedPG::on_change(ObjectStore::Transaction *t)
   pulling.clear();
   pull_from_peer.clear();
 
-  // clear temp
-  for (set<hobject_t>::iterator i = temp_contents.begin();
-       i != temp_contents.end();
-       ++i) {
-    dout(10) << __func__ << ": Removing oid "
-	     << *i << " from the temp collection" << dendl;
-    t->remove(get_temp_coll(t), *i);
-  }
-  temp_contents.clear();
-
   // clear snap_trimmer state
   snap_trimmer_machine.process_event(Reset());
 
diff --git a/src/osd/ReplicatedPG.h b/src/osd/ReplicatedPG.h
index 595fec5eed4..427e50e283c 100644
--- a/src/osd/ReplicatedPG.h
+++ b/src/osd/ReplicatedPG.h
@@ -572,6 +572,9 @@ protected:
   void submit_push_complete(ObjectRecoveryInfo &recovery_info,
 			    ObjectStore::Transaction *t);
 
+  // Track contents of temp collection, clear on reset
+  set<hobject_t> temp_contents;
+
   /*
    * Backfill
    *
@@ -966,17 +969,30 @@ public:
 
   void do_osd_op_effects(OpContext *ctx);
 private:
-  bool temp_created;
-  coll_t temp_coll;
-  set<hobject_t> temp_contents;   ///< contents of temp collection, clear on reset
   uint64_t temp_seq; ///< last id for naming temp objects
   coll_t get_temp_coll(ObjectStore::Transaction *t);
   hobject_t generate_temp_object();  ///< generate a new temp object name
 public:
-  bool have_temp_coll();
-  coll_t get_temp_coll() {
-    return temp_coll;
+  void get_colls(list<coll_t> *out) {
+    out->push_back(coll);
+    return pgbackend->temp_colls(out);
+  }
+  void split_colls(
+    pg_t child,
+    int split_bits,
+    int seed,
+    ObjectStore::Transaction *t) {
+    coll_t target = coll_t(child);
+    t->create_collection(target);
+    t->split_collection(
+      coll,
+      split_bits,
+      seed,
+      target);
+    pgbackend->split_colls(child, split_bits, seed, t);
   }
+  /// TODOXXX: remove this one, stub
+  coll_t get_temp_coll(ObjectStore::Transaction *t) { return coll_t(); }
 private:
   struct NotTrimming;
   struct SnapTrim : boost::statechart::event< SnapTrim > {