Merge pull request #2735 from ceph/wip-pg-stat

mon: make 'pg stat -f json' not pg dump; make JSON dump of floats floats and not strings

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

PendingReleaseNotes

@@ -10,3 +10,17 @@ v0.89
 
 v0.90
 -----
+
+* Previously, the formatted output of 'ceph pg stat -f ...' was a full
+  pg dump that included all metadata about all PGs in the system.  It
+  is now a concise summary of high-level PG stats, just like the
+  unformatted 'ceph pg stat' command.
+
+
+
+
+* All JSON dumps of floating point values were incorrecting surrounding the
+  value with quotes.  These quotes have been removed.  Any consumer of structured
+  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.

src/common/Formatter.cc

@@ -252,9 +252,10 @@ void JSONFormatter::dump_int(const char *name, int64_t s)
 
 void JSONFormatter::dump_float(const char *name, double d)
 {
+  print_name(name);
   char foo[30];
   snprintf(foo, sizeof(foo), "%lf", d);
-  dump_string(name, foo);
+  m_ss << foo;
 }
 
 void JSONFormatter::dump_string(const char *name, std::string s)

src/mon/PGMap.cc

@@ -926,7 +926,7 @@ void PGMap::recovery_summary(Formatter *f, list<string> *psl,
     if (f) {
       f->dump_unsigned("degraded_objects", delta_sum.stats.sum.num_objects_degraded);
       f->dump_unsigned("degraded_total", delta_sum.stats.sum.num_object_copies);
-      f->dump_string("degraded_ratio", b);
+      f->dump_float("degraded_ratio", pc / 100.0);
     } else {
       ostringstream ss;
       ss << delta_sum.stats.sum.num_objects_degraded
@@ -942,7 +942,7 @@ void PGMap::recovery_summary(Formatter *f, list<string> *psl,
     if (f) {
       f->dump_unsigned("misplaced_objects", delta_sum.stats.sum.num_objects_misplaced);
       f->dump_unsigned("misplaced_total", delta_sum.stats.sum.num_object_copies);
-      f->dump_string("misplaced_ratio", b);
+      f->dump_float("misplaced_ratio", pc / 100.0);
     } else {
       ostringstream ss;
       ss << delta_sum.stats.sum.num_objects_misplaced
@@ -958,7 +958,7 @@ void PGMap::recovery_summary(Formatter *f, list<string> *psl,
     if (f) {
       f->dump_unsigned("unfound_objects", delta_sum.stats.sum.num_objects_unfound);
       f->dump_unsigned("unfound_total", delta_sum.stats.sum.num_objects);
-      f->dump_string("unfound_ratio", b);
+      f->dump_float("unfound_ratio", pc / 100.0);
     } else {
       ostringstream ss;
       ss << delta_sum.stats.sum.num_objects_unfound
@@ -1259,26 +1259,41 @@ void PGMap::print_summary(Formatter *f, ostream *out) const
     *out << "  client io " << ssr.str() << "\n";
 }
 
-void PGMap::print_oneline_summary(ostream *out) const
+void PGMap::print_oneline_summary(Formatter *f, ostream *out) const
 {
   std::stringstream ss;
 
+  if (f)
+    f->open_object_section("num_pg_by_state");
   for (ceph::unordered_map<int,int>::const_iterator p = num_pg_by_state.begin();
        p != num_pg_by_state.end();
        ++p) {
+    if (f)
+      f->dump_unsigned(pg_state_string(p->first).c_str(), p->second);
     if (p != num_pg_by_state.begin())
       ss << ", ";
     ss << p->second << " " << pg_state_string(p->first);
   }
+  if (f)
+    f->close_section();
 
   string states = ss.str();
-  *out << "v" << version << ": "
+  if (out)
-       << pg_stat.size() << " pgs: "
+    *out << "v" << version << ": "
-       << states << "; "
+	 << pg_stat.size() << " pgs: "
-       << prettybyte_t(pg_sum.stats.sum.num_bytes) << " data, "
+	 << states << "; "
-       << kb_t(osd_sum.kb_used) << " used, "
+	 << prettybyte_t(pg_sum.stats.sum.num_bytes) << " data, "
-       << kb_t(osd_sum.kb_avail) << " / "
+	 << kb_t(osd_sum.kb_used) << " used, "
-       << kb_t(osd_sum.kb) << " avail";
+	 << kb_t(osd_sum.kb_avail) << " / "
+	 << kb_t(osd_sum.kb) << " avail";
+  if (f) {
+    f->dump_unsigned("version", version);
+    f->dump_unsigned("num_pgs", pg_stat.size());
+    f->dump_unsigned("num_bytes", pg_sum.stats.sum.num_bytes);
+    f->dump_unsigned("raw_bytes_used", osd_sum.kb_used << 10);
+    f->dump_unsigned("raw_bytes_avail", osd_sum.kb_avail << 10);
+    f->dump_unsigned("raw_bytes", osd_sum.kb << 10);
+  }
 
   // make non-negative; we can get negative values if osds send
   // uncommitted stats and then "go backward" or if they are just
@@ -1287,26 +1302,37 @@ void PGMap::print_oneline_summary(ostream *out) const
   pos_delta.floor(0);
   if (pos_delta.stats.sum.num_rd ||
       pos_delta.stats.sum.num_wr) {
-    *out << "; ";
+    if (out)
+      *out << "; ";
     if (pos_delta.stats.sum.num_rd) {
       int64_t rd = (pos_delta.stats.sum.num_rd_kb << 10) / (double)stamp_delta;
-      *out << pretty_si_t(rd) << "B/s rd, ";
+      if (out)
+	*out << pretty_si_t(rd) << "B/s rd, ";
+      if (f)
+	f->dump_unsigned("read_bytes_sec", rd);
     }
     if (pos_delta.stats.sum.num_wr) {
       int64_t wr = (pos_delta.stats.sum.num_wr_kb << 10) / (double)stamp_delta;
-      *out << pretty_si_t(wr) << "B/s wr, ";
+      if (out)
+	*out << pretty_si_t(wr) << "B/s wr, ";
+      if (f)
+	f->dump_unsigned("write_bytes_sec", wr);
     }
     int64_t iops = (pos_delta.stats.sum.num_rd + pos_delta.stats.sum.num_wr) / (double)stamp_delta;
-    *out << pretty_si_t(iops) << "op/s";
+    if (out)
+      *out << pretty_si_t(iops) << "op/s";
+    if (f)
+      f->dump_unsigned("io_sec", iops);
   }
 
   list<string> sl;
-  overall_recovery_summary(NULL, &sl);
+  overall_recovery_summary(f, &sl);
-  for (list<string>::iterator p = sl.begin(); p != sl.end(); ++p)
+  if (out)
-    *out << "; " << *p;
+    for (list<string>::iterator p = sl.begin(); p != sl.end(); ++p)
+      *out << "; " << *p;
   std::stringstream ssr;
-  overall_recovery_rate_summary(NULL, &ssr);
+  overall_recovery_rate_summary(f, &ssr);
-  if (ssr.str().length())
+  if (out && ssr.str().length())
     *out << "; " << ssr.str() << " recovering";
 }
 

src/mon/PGMap.h

@@ -315,7 +315,7 @@ public:
                                    uint64_t poolid) const;
 
   void print_summary(Formatter *f, ostream *out) const;
-  void print_oneline_summary(ostream *out) const;
+  void print_oneline_summary(Formatter *f, ostream *out) const;
 
   epoch_t get_min_last_epoch_clean() const {
     if (!min_last_epoch_clean)
@@ -329,7 +329,7 @@ WRITE_CLASS_ENCODER_FEATURES(PGMap::Incremental)
 WRITE_CLASS_ENCODER_FEATURES(PGMap)
 
 inline ostream& operator<<(ostream& out, const PGMap& m) {
-  m.print_oneline_summary(&out);
+  m.print_oneline_summary(NULL, &out);
   return out;
 }
 

src/mon/PGMonitor.cc

@@ -1499,8 +1499,8 @@ bool PGMonitor::preprocess_command(MMonCommand *m)
 
   if (prefix == "pg stat") {
     if (f) {
-      f->open_object_section("pg_map");
+      f->open_object_section("pg_summary");
-      pg_map.dump(f.get());
+      pg_map.print_oneline_summary(f.get(), NULL);
       f->close_section();
       f->flush(ds);
     } else {

src/test/formatter.cc

@@ -45,7 +45,7 @@ TEST(JsonFormatter, Simple2) {
   fmt.close_section();
   fmt.flush(oss);
   ASSERT_EQ(oss.str(), "{\"bar\":{\"int\":263882790666240,\
-\"unsigned\":9223372036854775809,\"float\":\"1.234000\"},\
+\"unsigned\":9223372036854775809,\"float\":1.234000},\
 \"string\":\"str\"}");
 }