From 4a3e4bcf40cd004fc53e7be467a29084dedc3e1c Mon Sep 17 00:00:00 2001 From: David Zafman Date: Wed, 15 Feb 2017 15:02:33 -0800 Subject: [PATCH] osd: Add automatic repair for DBObjectMap bug Add repair command to ceph-osdomap-tool too Under some situations the previous rm_keys() code would generated a corrupt complete table. There is no way to figure out what the table should look like now. By removing the entries we fix the corruption and aren't much worse off because the corruption caused some deleted keys to re-appear. This doesn't breaking the parent/child relationship during repair because some of the keys may still be contained in the parent. Signed-off-by: David Zafman (cherry picked from commit 4cd3c74c928a32e065ed9543d6c91d8718a6ae3d) Conflicts: src/os/filestore/DBObjectMap.h (trivial) --- src/os/ObjectMap.h | 2 +- src/os/filestore/DBObjectMap.cc | 19 ++++++++++++++++--- src/os/filestore/DBObjectMap.h | 2 +- src/tools/ceph_osdomap_tool.cc | 12 ++++++++---- 4 files changed, 26 insertions(+), 9 deletions(-) diff --git a/src/os/ObjectMap.h b/src/os/ObjectMap.h index a07142a09c481..961c3b24be9fc 100644 --- a/src/os/ObjectMap.h +++ b/src/os/ObjectMap.h @@ -137,7 +137,7 @@ public: const SequencerPosition *spos=0 ///< [in] Sequencer ) { return 0; } - virtual int check(std::ostream &out) { return 0; } + virtual int check(std::ostream &out, bool repair = false) { return 0; } typedef KeyValueDB::GenericIteratorImpl ObjectMapIteratorImpl; typedef ceph::shared_ptr ObjectMapIterator; diff --git a/src/os/filestore/DBObjectMap.cc b/src/os/filestore/DBObjectMap.cc index 49e36144c1c5a..25f6ea974f89d 100644 --- a/src/os/filestore/DBObjectMap.cc +++ b/src/os/filestore/DBObjectMap.cc @@ -54,7 +54,7 @@ static void append_escaped(const string &in, string *out) } } -int DBObjectMap::check(std::ostream &out) +int DBObjectMap::check(std::ostream &out, bool repair) { int errors = 0; map parent_to_num_children; @@ -77,18 +77,25 @@ int DBObjectMap::check(std::ostream &out) complete_iter->next()) { if (prev && prev >= complete_iter->key()) { out << "Bad complete for " << header.oid << std::endl; - errors++; complete_error = true; break; } prev = string(complete_iter->value().c_str(), complete_iter->value().length() - 1); } if (complete_error) { - out << "Complete mapping:" << std::endl; + out << "Complete mapping for " << header.seq << " :" << std::endl; for (complete_iter->seek_to_first(); complete_iter->valid(); complete_iter->next()) { out << complete_iter->key() << " -> " << string(complete_iter->value().c_str(), complete_iter->value().length() - 1) << std::endl; } + if (repair) { + KeyValueDB::Transaction t = db->get_transaction(); + t->rmkeys_by_prefix(USER_PREFIX + header_key(header.seq) + COMPLETE_PREFIX); + db->submit_transaction(t); + out << "Cleared complete mapping to repair" << std::endl; + } else { + errors++; // Only count when not repaired + } } if (header.parent == 0) @@ -1049,6 +1056,12 @@ int DBObjectMap::init(bool do_upgrade) state.v = 2; state.seq = 1; } + ostringstream ss; + int errors = check(ss, true); + if (errors) { + dout(5) << ss.str() << dendl; + return -EINVAL; + } dout(20) << "(init)dbobjectmap: seq is " << state.seq << dendl; return 0; } diff --git a/src/os/filestore/DBObjectMap.h b/src/os/filestore/DBObjectMap.h index 948e76b08a863..743db814ada03 100644 --- a/src/os/filestore/DBObjectMap.h +++ b/src/os/filestore/DBObjectMap.h @@ -212,7 +212,7 @@ public: int upgrade_to_v2(); /// Consistency check, debug, there must be no parallel writes - int check(std::ostream &out); + int check(std::ostream &out, bool repair = false); /// Ensure that all previous operations are durable int sync(const ghobject_t *oid=0, const SequencerPosition *spos=0); diff --git a/src/tools/ceph_osdomap_tool.cc b/src/tools/ceph_osdomap_tool.cc index b2f231641d5ff..11d984e330522 100644 --- a/src/tools/ceph_osdomap_tool.cc +++ b/src/tools/ceph_osdomap_tool.cc @@ -37,7 +37,7 @@ int main(int argc, char **argv) { ("debug", "Additional debug output from DBObjectMap") ("oid", po::value(&oid), "Restrict to this object id when dumping objects") ("command", po::value(&cmd), - "command arg is one of [dump-raw-keys, dump-raw-key-vals, dump-objects, dump-objects-with-keys, check, dump-headers], mandatory") + "command arg is one of [dump-raw-keys, dump-raw-key-vals, dump-objects, dump-objects-with-keys, check, dump-headers, repair], mandatory") ; po::positional_options_description p; p.add("command", 1); @@ -109,6 +109,9 @@ int main(int argc, char **argv) { std::cerr << "Output: " << out.str() << std::endl; goto done; } + // We don't call omap.init() here because it will repair + // the DBObjectMap which we might want to examine for diagnostic + // reasons. Instead use --command repair. r = 0; @@ -157,14 +160,15 @@ int main(int argc, char **argv) { j->value().hexdump(std::cout); } } - } else if (cmd == "check") { - r = omap.check(std::cout); + } else if (cmd == "check" || cmd == "repair") { + bool repair = (cmd == "repair"); + r = omap.check(std::cout, repair); if (r > 0) { std::cerr << "check got " << r << " error(s)" << std::endl; r = 1; goto done; } - std::cout << "check succeeded" << std::endl; + std::cout << (repair ? "repair" : "check") << " succeeded" << std::endl; } else if (cmd == "dump-headers") { vector headers; r = omap.list_object_headers(&headers); -- 2.39.5