Merge pull request ceph#2979 from ceph/wip-kill-category

osd: kill 'category' from pg_stats_t

Reviewed-by: Samuel Just <[email protected]>

PendingReleaseNotes

@@ -14,3 +14,17 @@ v0.90
   JSON output that was consuming the floating point values was previously having
   to interpret the quoted string and will most likely need to be fixed to take
   the unquoted number.
+
+* The 'category' field for objects has been removed.  This was originally added
+  to track PG stat summations over different categories of objects for use by
+  radosgw.  It is no longer has any known users and is prone to abuse because it
+  can lead to a pg_stat_t structure that is unbounded.  The librados API calls
+  that accept this field now ignore it, and the OSD no longers tracks the
+  per-category summations.
+
+* The output for 'rados df' has changed.  The 'category' level has been
+  eliminated, so there is now a single stat object per pool.  The structure of
+  the JSON output is different, and the plaintext output has one less column.
+
+* The 'rados create <objectname> [category]' optional category argument is no
+  longer supported or recognized.

src/include/rados/librados.h

@@ -2067,6 +2067,7 @@ CEPH_RADOS_API void rados_write_op_rmxattr(rados_write_op_t write_op,
  * @param exclusive set to either LIBRADOS_CREATE_EXCLUSIVE or
    LIBRADOS_CREATE_IDEMPOTENT
  * will error if the object already exists.
+ * @param category category string (DEPRECATED, HAS NO EFFECT)
  */
 CEPH_RADOS_API void rados_write_op_create(rados_write_op_t write_op,
                                           int exclusive,

src/include/rados/librados.hpp

@@ -308,7 +308,9 @@ namespace librados
     }
 
     void create(bool exclusive);
-    void create(bool exclusive, const std::string& category);
+    void create(bool exclusive,
+		const std::string& category); ///< NOTE: category is unused
+
     void write(uint64_t off, const bufferlist& bl);
     void write_full(const bufferlist& bl);
     void append(const bufferlist& bl);
@@ -584,7 +586,8 @@ namespace librados
 
     // create an object
     int create(const std::string& oid, bool exclusive);
-    int create(const std::string& oid, bool exclusive, const std::string& category);
+    int create(const std::string& oid, bool exclusive,
+	       const std::string& category); ///< category is unused
 
     /**
      * write bytes to an object at a specified offset
@@ -987,8 +990,12 @@ namespace librados
 
     /* listing objects */
     int pool_list(std::list<std::string>& v);
+    int get_pool_stats(std::list<std::string>& v,
+		       stats_map& result);
+    /// deprecated; use simpler form.  categories no longer supported.
     int get_pool_stats(std::list<std::string>& v,
 		       std::map<std::string, stats_map>& stats);
+    /// deprecated; categories no longer supported
     int get_pool_stats(std::list<std::string>& v,
                        std::string& category,
 		       std::map<std::string, stats_map>& stats);

src/librados/IoCtxImpl.cc

@@ -400,15 +400,6 @@ int librados::IoCtxImpl::create(const object_t& oid, bool exclusive)
   return operate(oid, &op, NULL);
 }
 
-int librados::IoCtxImpl::create(const object_t& oid, bool exclusive,
-				const std::string& category)
-{
-  ::ObjectOperation op;
-  prepare_assert_ops(&op);
-  op.create(exclusive, category);
-  return operate(oid, &op, NULL);
-}
-
 /*
  * add any version assert operations that are appropriate given the
  * stat in the IoCtx, either the target version assert or any src

src/librados/IoCtxImpl.h

@@ -115,7 +115,6 @@ struct librados::IoCtxImpl {
   int list(Objecter::ListContext *context, int max_entries);
   uint32_t list_seek(Objecter::ListContext *context, uint32_t pos);
   int create(const object_t& oid, bool exclusive);
-  int create(const object_t& oid, bool exclusive, const std::string& category);
   int write(const object_t& oid, bufferlist& bl, size_t len, uint64_t off);
   int append(const object_t& oid, bufferlist& bl, size_t len);
   int write_full(const object_t& oid, bufferlist& bl);

src/librados/librados.cc

@@ -314,10 +314,11 @@ void librados::ObjectWriteOperation::create(bool exclusive)
   o->create(exclusive);
 }
 
-void librados::ObjectWriteOperation::create(bool exclusive, const std::string& category)
+void librados::ObjectWriteOperation::create(bool exclusive,
+					    const std::string& category) // unused
 {
   ::ObjectOperation *o = (::ObjectOperation *)impl;
-  o->create(exclusive, category);
+  o->create(exclusive);
 }
 
 void librados::ObjectWriteOperation::write(uint64_t off, const bufferlist& bl)
@@ -1021,10 +1022,11 @@ int librados::IoCtx::create(const std::string& oid, bool exclusive)
   return io_ctx_impl->create(obj, exclusive);
 }
 
-int librados::IoCtx::create(const std::string& oid, bool exclusive, const std::string& category)
+int librados::IoCtx::create(const std::string& oid, bool exclusive,
+			    const std::string& category) // unused
 {
   object_t obj(oid);
-  return io_ctx_impl->create(obj, exclusive, category);
+  return io_ctx_impl->create(obj, exclusive);
 }
 
 int librados::IoCtx::write(const std::string& oid, bufferlist& bl, size_t len, uint64_t off)
@@ -1964,69 +1966,54 @@ void librados::Rados::test_blacklist_self(bool set)
   client->blacklist_self(set);
 }
 
-int librados::Rados::get_pool_stats(std::list<string>& v, std::map<string, stats_map>& result)
+int librados::Rados::get_pool_stats(std::list<string>& v,
+				    stats_map& result)
 {
-  string category;
-  return get_pool_stats(v, category, result);
+  map<string,::pool_stat_t> rawresult;
+  int r = client->get_pool_stats(v, rawresult);
+  for (map<string,::pool_stat_t>::iterator p = rawresult.begin();
+       p != rawresult.end();
+       ++p) {
+    pool_stat_t& pv = result[p->first];
+    object_stat_sum_t *sum = &p->second.stats.sum;
+    pv.num_kb = SHIFT_ROUND_UP(sum->num_bytes, 10);
+    pv.num_bytes = sum->num_bytes;
+    pv.num_objects = sum->num_objects;
+    pv.num_object_clones = sum->num_object_clones;
+    pv.num_object_copies = sum->num_object_copies;
+    pv.num_objects_missing_on_primary = sum->num_objects_missing_on_primary;
+    pv.num_objects_unfound = sum->num_objects_unfound;
+    pv.num_objects_degraded = sum->num_objects_degraded;
+    pv.num_rd = sum->num_rd;
+    pv.num_rd_kb = sum->num_rd_kb;
+    pv.num_wr = sum->num_wr;
+    pv.num_wr_kb = sum->num_wr_kb;
+  }
+  return r;
 }
 
-int librados::Rados::get_pool_stats(std::list<string>& v, string& category,
+int librados::Rados::get_pool_stats(std::list<string>& v,
 				    std::map<string, stats_map>& result)
 {
-  map<string,::pool_stat_t> rawresult;
-  int r = client->get_pool_stats(v, rawresult);
+  stats_map m;
+  int r = get_pool_stats(v, m);
   if (r < 0)
     return r;
-  for (map<string,::pool_stat_t>::iterator p = rawresult.begin();
-       p != rawresult.end();
+  for (map<string,pool_stat_t>::iterator p = m.begin();
+       p != m.end();
        ++p) {
-    stats_map& c = result[p->first];
-
-    string cat;
-    vector<string> cats;
-
-    if (!category.size()) {
-      cats.push_back(cat);
-      map<string,object_stat_sum_t>::iterator iter;
-      for (iter = p->second.stats.cat_sum.begin(); iter != p->second.stats.cat_sum.end(); ++iter) {
-        cats.push_back(iter->first);
-      }
-    } else {
-      cats.push_back(category);
-    }
-
-    vector<string>::iterator cat_iter;
-    for (cat_iter = cats.begin(); cat_iter != cats.end(); ++cat_iter) {
-      string& cur_category = *cat_iter;
-      object_stat_sum_t *sum;
-
-      if (!cur_category.size()) {
-         sum = &p->second.stats.sum;
-      } else {
-        map<string,object_stat_sum_t>::iterator iter = p->second.stats.cat_sum.find(cur_category);
-        if (iter == p->second.stats.cat_sum.end())
-          continue;
-        sum = &iter->second;
-      }
-
-      pool_stat_t& pv = c[cur_category];
-      pv.num_kb = SHIFT_ROUND_UP(sum->num_bytes, 10);
-      pv.num_bytes = sum->num_bytes;
-      pv.num_objects = sum->num_objects;
-      pv.num_object_clones = sum->num_object_clones;
-      pv.num_object_copies = sum->num_object_copies;
-      pv.num_objects_missing_on_primary = sum->num_objects_missing_on_primary;
-      pv.num_objects_unfound = sum->num_objects_unfound;
-      pv.num_objects_degraded = sum->num_objects_degraded;
-      pv.num_rd = sum->num_rd;
-      pv.num_rd_kb = sum->num_rd_kb;
-      pv.num_wr = sum->num_wr;
-      pv.num_wr_kb = sum->num_wr_kb;
-    }
+    result[p->first][string()] = p->second;
   }
   return r;
 }
 
+int librados::Rados::get_pool_stats(std::list<string>& v,
+				    string& category, // unused
+				    std::map<string, stats_map>& result)
+{
+  return -EOPNOTSUPP;
+}
+
 int librados::Rados::cluster_stat(cluster_stat_t& result)
 {
   ceph_statfs stats;
@@ -4008,15 +3995,11 @@ extern "C" void rados_write_op_rmxattr(rados_write_op_t write_op,
 
 extern "C" void rados_write_op_create(rados_write_op_t write_op,
                                       int exclusive,
-				      const char* category)
+				      const char* category) // unused
 {
-  tracepoint(librados, rados_write_op_create_enter, write_op, exclusive, category);
+  tracepoint(librados, rados_write_op_create_enter, write_op, exclusive);
   ::ObjectOperation *oo = (::ObjectOperation *) write_op;
-  if(category) {
-    oo->create(exclusive, category);
-  } else {
-    oo->create(!!exclusive);
-  }
+  oo->create(!!exclusive);
   tracepoint(librados, rados_write_op_create_exit);
 }
 

src/mon/PGMonitor.cc

@@ -1384,28 +1384,6 @@ void PGMonitor::dump_pool_stats(stringstream &ss, Formatter *f, bool verbose)
     else
       tbl << TextTable::endrow;
 
-    if (verbose) {
-      if (f)
-        f->open_array_section("categories");
-
-      for (map<string,object_stat_sum_t>::iterator it = stat.stats.cat_sum.begin();
-          it != stat.stats.cat_sum.end(); ++it) {
-        if (f) {
-          f->open_object_section(it->first.c_str());
-        } else {
-          tbl << ""
-              << ""
-              << it->first;
-        }
-        dump_object_stat_sum(tbl, f, it->second, avail, verbose);
-        if (f)
-          f->close_section(); // category name
-        else
-          tbl << TextTable::endrow;
-      }
-      if (f)
-        f->close_section(); // categories
-    }
     if (f)
       f->close_section(); // pool
   }

src/osd/ReplicatedPG.cc

@@ -2649,7 +2649,7 @@ ReplicatedPG::RepGather *ReplicatedPG::trim_object(const hobject_t &coid)
       delta.num_whiteouts--;
     }
     delta.num_object_clones--;
-    info.stats.stats.add(delta, obc->obs.oi.category);
+    info.stats.stats.add(delta);
     obc->obs.exists = false;
 
     snapset.clones.erase(p);
@@ -4166,14 +4166,7 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
 	      result = -EINVAL;
 	      goto fail;
 	    }
-	    if (category.size()) {
-	      if (obs.exists && !oi.is_whiteout()) {
-		if (obs.oi.category != category)
-		  result = -EEXIST;  // category cannot be reset
-	      } else {
-		obs.oi.category = category;
-	      }
-	    }
+	    // category is no longer implemented.
 	  }
           if (result >= 0) {
             if (!obs.exists)
@@ -5480,7 +5473,7 @@ int ReplicatedPG::prepare_transaction(OpContext *ctx)
 
   // read-op?  done?
   if (ctx->op_t->empty() && !ctx->modify) {
-    unstable_stats.add(ctx->delta_stats, ctx->obc->obs.oi.category);
+    unstable_stats.add(ctx->delta_stats);
     return result;
   }
 
@@ -5680,24 +5673,23 @@ void ReplicatedPG::finish_ctx(OpContext *ctx, int log_op_type, bool maintain_ssc
     ctx->obc->ssc->snapset = ctx->new_snapset;
   }
 
-  info.stats.stats.add(ctx->delta_stats, ctx->obs->oi.category);
+  info.stats.stats.add(ctx->delta_stats);
 
   for (set<pg_shard_t>::iterator i = backfill_targets.begin();
        i != backfill_targets.end();
        ++i) {
     pg_shard_t bt = *i;
     pg_info_t& pinfo = peer_info[bt];
     if (soid <= pinfo.last_backfill)
-      pinfo.stats.stats.add(ctx->delta_stats, ctx->obs->oi.category);
+      pinfo.stats.stats.add(ctx->delta_stats);
     else if (soid <= last_backfill_started)
-      pending_backfill_updates[soid].stats.add(ctx->delta_stats,
-					       ctx->obs->oi.category);
+      pending_backfill_updates[soid].stats.add(ctx->delta_stats);
   }
 
   if (scrubber.active) {
     assert(soid < scrubber.start || soid >= scrubber.end);
     if (soid < scrubber.start)
-      scrub_cstat.add(ctx->delta_stats, ctx->obs->oi.category);
+      scrub_cstat.add(ctx->delta_stats);
   }
 }
 
@@ -5809,7 +5801,6 @@ int ReplicatedPG::fill_in_copy_get(
   // size, mtime
   reply_obj.size = oi.size;
   reply_obj.mtime = oi.mtime;
-  reply_obj.category = oi.category;
   if (soid.snap < CEPH_NOSNAP) {
     reply_obj.snaps = oi.snaps;
   } else {
@@ -5983,7 +5974,6 @@ void ReplicatedPG::_copy_some(ObjectContextRef obc, CopyOpRef cop)
   }
   op.copy_get(&cop->cursor, get_copy_chunk_size(),
 	      &cop->results.object_size, &cop->results.mtime,
-	      &cop->results.category,
 	      &cop->attrs, &cop->data, &cop->omap_header, &cop->omap,
 	      &cop->results.snaps, &cop->results.snap_seq,
 	      &cop->rval);
@@ -6353,7 +6343,6 @@ void ReplicatedPG::finish_promote(int r, OpRequestRef op,
       tctx->discard_temp_oid = results->temp_oid;
     }
     tctx->new_obs.oi.size = results->object_size;
-    tctx->new_obs.oi.category = results->category;
     tctx->new_obs.oi.user_version = results->user_version;
 
     if (soid.snap != CEPH_NOSNAP) {
@@ -7851,8 +7840,6 @@ void ReplicatedPG::add_object_context_to_pg_stat(ObjectContextRef obc, pg_stat_t
 
   // add it in
   pgstat->stats.sum.add(stat);
-  if (oi.category.length())
-    pgstat->stats.cat_sum[oi.category].add(stat);
 }
 
 void ReplicatedPG::kick_object_context_blocked(ObjectContextRef obc)
@@ -11340,7 +11327,7 @@ void ReplicatedPG::hit_set_persist()
 
   hit_set_trim(repop, max);
 
-  info.stats.stats.add(ctx->delta_stats, string());
+  info.stats.stats.add(ctx->delta_stats);
 
   simple_repop_submit(repop);
 }
@@ -12285,8 +12272,7 @@ void ReplicatedPG::_scrub(ScrubMap& scrubmap)
       next_clone = hobject_t();
     }
 
-    string cat; // fixme
-    scrub_cstat.add(stat, cat);
+    scrub_cstat.add(stat);
   }
 
   if (!next_clone.is_min() &&

src/osd/ReplicatedPG.h

@@ -115,7 +115,6 @@ class ReplicatedPG : public PG, public PGBackend::Listener {
      * other accesses to the object (in order to complete the copy).
      */
     PGBackend::PGTransaction *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
     vector<snapid_t> snaps;  ///< src's snaps (if clone)