tools: Check for overlaps in internal "complete" table for DBObjectMap

Changed check to return an error count and fix tool error message Signed-off-by: David Zafman <dzafman@redhat.com>
dzafman · Mar 27, 2017 · e5e8eb9 · e5e8eb9
1 parent f410159
commit e5e8eb9
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 10 deletions.
diff --git a/src/os/ObjectMap.h b/src/os/ObjectMap.h
@@ -138,7 +138,7 @@ class ObjectMap {
     const SequencerPosition *spos=0   ///< [in] Sequencer
     ) { return 0; }
 
-  virtual bool check(std::ostream &out) { return true; }
+  virtual int check(std::ostream &out) { return 0; }
 
   typedef KeyValueDB::GenericIteratorImpl ObjectMapIteratorImpl;
   typedef ceph::shared_ptr<ObjectMapIteratorImpl> ObjectMapIterator;

diff --git a/src/os/filestore/DBObjectMap.cc b/src/os/filestore/DBObjectMap.cc
@@ -55,9 +55,9 @@ static void append_escaped(const string &in, string *out)
   }
 }
 
-bool DBObjectMap::check(std::ostream &out)
+int DBObjectMap::check(std::ostream &out)
 {
-  bool retval = true;
+  int errors = 0;
   map<uint64_t, uint64_t> parent_to_num_children;
   map<uint64_t, uint64_t> parent_to_actual_num_children;
   KeyValueDB::Iterator iter = db->get_iterator(HOBJECT_TO_SEQ);
@@ -69,6 +69,20 @@ bool DBObjectMap::check(std::ostream &out)
       header.decode(bliter);
       if (header.seq != 0)
 	parent_to_actual_num_children[header.seq] = header.num_children;
+
+      // Check complete table
+      boost::optional<string> prev;
+      KeyValueDB::Iterator complete_iter = db->get_iterator(USER_PREFIX + header_key(header.seq) + COMPLETE_PREFIX);
+      for (complete_iter->seek_to_first(); complete_iter->valid();
+           complete_iter->next()) {
+         if (prev && prev >= complete_iter->key()) {
+             out << "Bad complete for " << header.oid << std::endl;
+             errors++;
+             break;
+         }
+         prev = string(complete_iter->value().c_str(), complete_iter->value().length() - 1);
+      }
+
       if (header.parent == 0)
 	break;
 
@@ -84,7 +98,7 @@ bool DBObjectMap::check(std::ostream &out)
       db->get(sys_parent_prefix(header), to_get, &got);
       if (got.empty()) {
 	out << "Missing: seq " << header.parent << std::endl;
-	retval = false;
+	errors++;
 	break;
       } else {
 	bl = got.begin()->second;
@@ -101,11 +115,11 @@ bool DBObjectMap::check(std::ostream &out)
       out << "Invalid: seq " << i->first << " recorded children: "
 	  << parent_to_actual_num_children[i->first] << " found: "
 	  << i->second << std::endl;
-      retval = false;
+      errors++;
     }
     parent_to_actual_num_children.erase(i->first);
   }
-  return retval;
+  return errors;
 }
 
 string DBObjectMap::ghobject_key(const ghobject_t &oid)

diff --git a/src/os/filestore/DBObjectMap.h b/src/os/filestore/DBObjectMap.h
@@ -213,7 +213,7 @@ class DBObjectMap : public ObjectMap {
   int upgrade_to_v2();
 
   /// Consistency check, debug, there must be no parallel writes
-  bool check(std::ostream &out) override;
+  int check(std::ostream &out) override;
 
   /// Ensure that all previous operations are durable
   int sync(const ghobject_t *oid=0, const SequencerPosition *spos=0) override;

diff --git a/src/test/ObjectMap/test_object_map.cc b/src/test/ObjectMap/test_object_map.cc
@@ -542,7 +542,7 @@ class ObjectMapTest : public ::testing::Test {
 
   void TearDown() override {
     std::cerr << "Checking..." << std::endl;
-    assert(db->check(std::cerr));
+    assert(db->check(std::cerr) == 0);
   }
 };
 

diff --git a/src/tools/ceph_osdomap_tool.cc b/src/tools/ceph_osdomap_tool.cc
@@ -150,8 +150,8 @@ int main(int argc, char **argv) {
     }
   } else if (cmd == "check") {
     r = omap.check(std::cout);
-    if (!r) {
-      std::cerr << "check got: " << cpp_strerror(r) << std::endl;
+    if (r > 0) {
+      std::cerr << "check got " << r << " error(s)" << std::endl;
       goto done;
     }
     std::cout << "check succeeded" << std::endl;