recover from a commit2tree entry existing without the corresponding tree

Summary:
When testing D8108649 I accidentally deleted all of my trees
but didn't delete my commit2tree mapping. This diff allows Eden to
recover from that situation.

Reviewed By: wez

Differential Revision: D8108728

fbshipit-source-id: 94a9393294ca259303026c297683dac4b3ecfac4

eden/fs/model/Tree.h

@@ -57,6 +57,15 @@ class Tree {
     return *entry;
   }
 
+  std::vector<PathComponent> getEntryNames() const {
+    std::vector<PathComponent> results;
+    results.reserve(entries_.size());
+    for (const auto& entry : entries_) {
+      results.emplace_back(entry.getName());
+    }
+    return results;
+  }
+
  private:
   const Hash hash_;
   const std::vector<TreeEntry> entries_;

eden/fs/service/EdenServer.cpp

@@ -17,6 +17,7 @@
 #include <folly/io/async/AsyncSignalHandler.h>
 #include <folly/logging/xlog.h>
 #include <gflags/gflags.h>
+#include <signal.h>
 #include <thrift/lib/cpp/concurrency/ThreadManager.h>
 #include <thrift/lib/cpp2/server/ThriftServer.h>
 

eden/fs/store/hg/HgBackingStore.cpp

@@ -20,6 +20,7 @@
 #include "eden/fs/model/Tree.h"
 #include "eden/fs/store/LocalStore.h"
 #include "eden/fs/store/StoreResult.h"
+#include "eden/fs/store/hg/HgImporter.h"
 #include "eden/fs/utils/UnboundedQueueThreadPool.h"
 
 using folly::ByteRange;
@@ -45,12 +46,12 @@ namespace eden {
 
 namespace {
 // Thread local HgImporter. This is only initialized on HgImporter threads.
-static folly::ThreadLocalPtr<HgImporter> threadLocalImporter;
+static folly::ThreadLocalPtr<Importer> threadLocalImporter;
 
 /**
  * Checks that the thread local HgImporter is present and returns it.
  */
-HgImporter& getThreadLocalImporter() {
+Importer& getThreadLocalImporter() {
   if (!threadLocalImporter) {
     throw std::logic_error(
         "Attempting to get HgImporter from non-HgImporter thread");
@@ -81,6 +82,21 @@ class HgImporterThreadFactory : public folly::ThreadFactory {
   AbsolutePath repository_;
   LocalStore* localStore_;
 };
+
+/**
+ * An inline executor that, while it exists, keeps a thread-local HgImporter
+ * instance.
+ */
+class HgImporterTestExecutor : public folly::InlineExecutor {
+ public:
+  explicit HgImporterTestExecutor(Importer* importer) {
+    threadLocalImporter.reset(importer);
+  }
+
+  ~HgImporterTestExecutor() {
+    threadLocalImporter.release();
+  }
+};
 } // namespace
 
 HgBackingStore::HgBackingStore(
@@ -102,6 +118,16 @@ HgBackingStore::HgBackingStore(
           std::make_shared<HgImporterThreadFactory>(repository, localStore))),
       serverThreadPool_(serverThreadPool) {}
 
+/**
+ * Create an HgBackingStore suitable for use in unit tests. It uses an inline
+ * executor to process loaded objects rather than the thread pools used in
+ * production Eden.
+ */
+HgBackingStore::HgBackingStore(Importer* importer, LocalStore* localStore)
+    : localStore_{localStore},
+      importThreadPool_{std::make_unique<HgImporterTestExecutor>(importer)},
+      serverThreadPool_{importThreadPool_.get()} {}
+
 HgBackingStore::~HgBackingStore() {}
 
 Future<unique_ptr<Tree>> HgBackingStore::getTree(const Hash& id) {
@@ -150,22 +176,54 @@ Future<unique_ptr<Tree>> HgBackingStore::getTreeForCommit(
 
 folly::Future<unique_ptr<Tree>> HgBackingStore::getTreeForCommitImpl(
     const Hash& commitID) {
-  Hash rootTreeHash;
-  auto result = localStore_->get(KeySpace::HgCommitToTreeFamily, commitID);
-  if (result.isValid()) {
-    rootTreeHash = Hash{result.bytes()};
-    XLOG(DBG5) << "found existing tree " << rootTreeHash.toString()
-               << " for mercurial commit " << commitID.toString();
-  } else {
-    rootTreeHash = getThreadLocalImporter().importManifest(commitID.toString());
-    XLOG(DBG1) << "imported mercurial commit " << commitID.toString()
-               << " as tree " << rootTreeHash.toString();
+  return localStore_
+      ->getFuture(KeySpace::HgCommitToTreeFamily, commitID.getBytes())
+      .then(
+          [this,
+           commitID](StoreResult result) -> folly::Future<unique_ptr<Tree>> {
+            if (!result.isValid()) {
+              return nullptr;
+            }
 
-    localStore_->put(
-        KeySpace::HgCommitToTreeFamily, commitID, rootTreeHash.getBytes());
-  }
+            auto rootTreeHash = Hash{result.bytes()};
+            XLOG(DBG5) << "found existing tree " << rootTreeHash.toString()
+                       << " for mercurial commit " << commitID.toString();
 
-  return localStore_->getTree(rootTreeHash);
+            return localStore_->getTree(rootTreeHash)
+                .then(
+                    [rootTreeHash, commitID](std::unique_ptr<Tree> tree)
+                        -> folly::Future<unique_ptr<Tree>> {
+                      if (tree) {
+                        return tree;
+                      }
+                      // No corresponding tree for this commit ID! Must
+                      // re-import. This could happen if RocksDB is corrupted
+                      // in some way or deleting entries races with
+                      // population.
+                      XLOG(WARN) << "No corresponding tree " << rootTreeHash
+                                 << " for commit " << commitID
+                                 << "; will import again";
+                      return nullptr;
+                    });
+          })
+      .then(
+          [this,
+           commitID](unique_ptr<Tree> tree) -> folly::Future<unique_ptr<Tree>> {
+            if (tree) {
+              return tree;
+            } else {
+              auto rootTreeHash =
+                  getThreadLocalImporter().importManifest(commitID.toString());
+              XLOG(DBG1) << "imported mercurial commit " << commitID.toString()
+                         << " as tree " << rootTreeHash.toString();
+
+              localStore_->put(
+                  KeySpace::HgCommitToTreeFamily,
+                  commitID,
+                  rootTreeHash.getBytes());
+              return localStore_->getTree(rootTreeHash);
+            }
+          });
 }
 } // namespace eden
 } // namespace facebook

eden/fs/store/hg/HgBackingStore.h

@@ -10,7 +10,6 @@
 #pragma once
 
 #include "eden/fs/store/BackingStore.h"
-#include "eden/fs/store/hg/HgImporter.h"
 #include "eden/fs/utils/PathFuncs.h"
 #include "eden/fs/utils/UnboundedQueueThreadPool.h"
 
@@ -23,6 +22,7 @@ namespace eden {
 
 class LocalStore;
 class UnboundedQueueThreadPool;
+class Importer;
 
 /**
  * A BackingStore implementation that loads data out of a mercurial repository.
@@ -40,6 +40,14 @@ class HgBackingStore : public BackingStore {
       AbsolutePathPiece repository,
       LocalStore* localStore,
       UnboundedQueueThreadPool* serverThreadPool);
+
+  /**
+   * Create an HgBackingStore suitable for use in unit tests. It uses an inline
+   * executor to process loaded objects rather than the thread pools used in
+   * production Eden.
+   */
+  HgBackingStore(Importer* importer, LocalStore* localStore);
+
   ~HgBackingStore() override;
 
   folly::Future<std::unique_ptr<Tree>> getTree(const Hash& id) override;
@@ -65,7 +73,7 @@ class HgBackingStore : public BackingStore {
   // the importer pool. Queuing in this pool can never block (which would risk
   // deadlock) or throw an exception when full (which would incorrectly fail the
   // load).
-  UnboundedQueueThreadPool* serverThreadPool_;
+  folly::Executor* serverThreadPool_;
 };
 } // namespace eden
 } // namespace facebook

eden/fs/store/hg/HgImporter.h

@@ -37,6 +37,41 @@ class HgManifestImporter;
 class StoreResult;
 class Tree;
 
+class Importer {
+ public:
+  virtual ~Importer() {}
+
+  /**
+   * Import the manifest for the specified revision.
+   *
+   * Returns a Hash identifying the root Tree for the imported revision.
+   */
+  virtual Hash importManifest(folly::StringPiece revName) = 0;
+
+  /**
+   * Import the tree with the specified tree manifest hash.
+   *
+   * @param id The Tree ID.  Note that this is eden's Tree ID, and does not
+   *   correspond to the mercurial manifest node ID for this path.
+   *
+   * Returns the Tree, or throws on error.
+   * Requires that tree manifest data be available.
+   */
+  virtual std::unique_ptr<Tree> importTree(const Hash& id) = 0;
+
+  /**
+   * Import file information
+   *
+   * Takes a hash identifying the requested blob.  (For instance, blob hashes
+   * can be found in the TreeEntry objects generated by importManifest().)
+   *
+   * Returns an IOBuf containing the file contents.
+   */
+  virtual folly::IOBuf importFileContents(Hash blobHash) = 0;
+
+  virtual void prefetchFiles(const std::vector<Hash>& blobHashes) = 0;
+};
+
 /**
  * HgImporter provides an API for extracting data out of a mercurial
  * repository.
@@ -50,7 +85,7 @@ class Tree;
  * multiple HgImporter objects can be created for the same repository and used
  * simultaneously.
  */
-class HgImporter {
+class HgImporter : public Importer {
  public:
   /**
    * Create a new HgImporter object that will import data from the specified
@@ -62,11 +97,6 @@ class HgImporter {
   HgImporter(AbsolutePathPiece repoPath, LocalStore* store);
   virtual ~HgImporter();
 
-  /**
-   * Import the manifest for the specified revision.
-   *
-   * Returns a Hash identifying the root Tree for the imported revision.
-   */
   Hash importManifest(folly::StringPiece revName);
 
 #if EDEN_HAVE_HG_TREEMANIFEST
@@ -102,28 +132,9 @@ class HgImporter {
    */
   static Hash importFlatManifest(int manifestDataFd, LocalStore* store);
 
-  /**
-   * Import the tree with the specified tree manifest hash.
-   *
-   * @param id The Tree ID.  Note that this is eden's Tree ID, and does not
-   *   correspond to the mercurial manifest node ID for this path.
-   *
-   * Returns the Tree, or throws on error.
-   * Requires that tree manifest data be available.
-   */
-  std::unique_ptr<Tree> importTree(const Hash& id);
-
-  /**
-   * Import file information
-   *
-   * Takes a hash identifying the requested blob.  (For instance, blob hashes
-   * can be found in the TreeEntry objects generated by importManifest().)
-   *
-   * Returns an IOBuf containing the file contents.
-   */
-  folly::IOBuf importFileContents(Hash blobHash);
-
-  void prefetchFiles(const std::vector<Hash>& blobHashes);
+  std::unique_ptr<Tree> importTree(const Hash& id) override;
+  folly::IOBuf importFileContents(Hash blobHash) override;
+  void prefetchFiles(const std::vector<Hash>& blobHashes) override;
 
   /**
    * Resolve the manifest node for the specified revision.

eden/fs/store/hg/test/HgBackingStoreTest.cpp

@@ -0,0 +1,69 @@
+/*
+ *  Copyright (c) 2018-present, Facebook, Inc.
+ *  All rights reserved.
+ *
+ *  This source code is licensed under the BSD-style license found in the
+ *  LICENSE file in the root directory of this source tree. An additional grant
+ *  of patent rights can be found in the PATENTS file in the same directory.
+ *
+ */
+#include <folly/experimental/TestUtil.h>
+#include <folly/test/TestUtils.h>
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include "eden/fs/model/Tree.h"
+#include "eden/fs/store/MemoryLocalStore.h"
+#include "eden/fs/store/ObjectStore.h"
+#include "eden/fs/store/hg/HgBackingStore.h"
+#include "eden/fs/store/hg/HgImporter.h"
+#include "eden/fs/testharness/HgRepo.h"
+
+using namespace facebook::eden;
+using namespace std::chrono_literals;
+
+struct TestRepo {
+  folly::test::TemporaryDirectory testDir{"eden_hg_backing_store_test"};
+  AbsolutePath testPath{testDir.path().string()};
+  HgRepo repo{testPath + "repo"_pc};
+  Hash commit1;
+
+  TestRepo() {
+    repo.hgInit();
+
+    repo.mkdir("foo");
+    repo.writeFile("foo/bar.txt", "bar\n");
+    repo.mkdir("src");
+    repo.writeFile("src/hello.txt", "world\n");
+    repo.hg("add");
+    commit1 = repo.commit("Initial commit");
+  }
+};
+
+struct HgBackingStoreTest : TestRepo, ::testing::Test {
+  HgBackingStoreTest() {}
+
+  std::shared_ptr<MemoryLocalStore> localStore{
+      std::make_shared<MemoryLocalStore>()};
+  HgImporter importer{repo.path(), localStore.get()};
+  std::shared_ptr<HgBackingStore> backingStore{
+      std::make_shared<HgBackingStore>(&importer, localStore.get())};
+  ObjectStore objectStore{localStore, backingStore};
+};
+
+TEST_F(
+    HgBackingStoreTest,
+    getTreeForCommit_reimports_tree_if_it_was_deleted_after_import) {
+  auto tree1 = objectStore.getTreeForCommit(commit1).get(0ms);
+  EXPECT_TRUE(tree1);
+  ASSERT_THAT(
+      tree1->getEntryNames(),
+      ::testing::ElementsAre(PathComponent{"foo"}, PathComponent{"src"}));
+
+  localStore->clearKeySpace(LocalStore::TreeFamily);
+  auto tree2 = objectStore.getTreeForCommit(commit1).get(0ms);
+  EXPECT_TRUE(tree2);
+  ASSERT_THAT(
+      tree1->getEntryNames(),
+      ::testing::ElementsAre(PathComponent{"foo"}, PathComponent{"src"}));
+}

eden/fs/store/hg/test/HgImportTest.cpp

@@ -9,9 +9,6 @@
  */
 #include <folly/experimental/TestUtil.h>
 #include <folly/futures/Future.h>
-#include <folly/init/Init.h>
-#include <folly/logging/Init.h>
-#include <folly/logging/xlog.h>
 #include <folly/test/TestUtils.h>
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
@@ -35,14 +32,6 @@ using std::vector;
 using testing::ElementsAre;
 
 namespace {
-vector<PathComponent> getTreeEntryNames(const Tree* tree) {
-  vector<PathComponent> results;
-  for (const auto& entry : tree->getTreeEntries()) {
-    results.push_back(entry.getName());
-  }
-  return results;
-}
-
 enum class RepoType {
   FLAT_MANIFEST,
   TREE_MANIFEST,
@@ -74,7 +63,7 @@ class HgImportTest : public ::testing::TestWithParam<RepoType> {
   }
 
  protected:
-  TemporaryDirectory testDir_{"eden_test"};
+  TemporaryDirectory testDir_{"eden_hg_import_test"};
   AbsolutePath testPath_{testDir_.path().string()};
   HgRepo repo_{testPath_ + "repo"_pc};
   MemoryLocalStore localStore_;
@@ -108,7 +97,7 @@ TEST_P(HgImportTest, importTest) {
   EXPECT_EQ(rootTreeHash, rootTree->getHash());
   EXPECT_EQ(rootTreeHash, rootTree->getHash());
   ASSERT_THAT(
-      getTreeEntryNames(rootTree.get()),
+      rootTree->getEntryNames(),
       ElementsAre(PathComponent{"foo"}, PathComponent{"src"}));
 
   // Get the "foo" tree.
@@ -122,7 +111,7 @@ TEST_P(HgImportTest, importTest) {
       : localStore_.getTree(fooEntry.getHash()).get(10s);
   ASSERT_TRUE(fooTree);
   ASSERT_THAT(
-      getTreeEntryNames(fooTree.get()),
+      fooTree->getEntryNames(),
       ElementsAre(PathComponent{"bar.txt"}, PathComponent{"test.txt"}));
   if (treemanifest) {
     // HgImporter::importTree() is currently responsible for inserting the tree
@@ -149,7 +138,7 @@ TEST_P(HgImportTest, importTest) {
       : localStore_.getTree(srcEntry.getHash()).get(10ms);
   ASSERT_TRUE(srcTree);
   ASSERT_THAT(
-      getTreeEntryNames(srcTree.get()),
+      srcTree->getEntryNames(),
       ElementsAre(PathComponent{"eden"}, PathComponent{"somelink"}));
   if (treemanifest) {
     auto srcTree2 = localStore_.getTree(srcEntry.getHash()).get(10ms);
@@ -167,8 +156,7 @@ TEST_P(HgImportTest, importTest) {
       ? importer.importTree(edenEntry.getHash())
       : localStore_.getTree(edenEntry.getHash()).get(10s);
   ASSERT_TRUE(edenTree);
-  ASSERT_THAT(
-      getTreeEntryNames(edenTree.get()), ElementsAre(PathComponent{"main.py"}));
+  ASSERT_THAT(edenTree->getEntryNames(), ElementsAre(PathComponent{"main.py"}));
   if (treemanifest) {
     auto edenTree2 = localStore_.getTree(edenEntry.getHash()).get(10ms);
     ASSERT_TRUE(edenTree2);