avoid blocking on HgProxyHash::getBatch() in hg importer threads

Summary:
D8065370 changed the HgImporter code to make a blocking `get()` call on a
future from the hg importer thread pool.  This caused deadlocks, since all of
the hg importer threads could become stuck waiting on these `get()` calls to
complete.  These would be waiting on RocksDbLocalStore threads which were in
turn all busy waiting to schedule operations on the HgImporter threads.

This fixes the code to use `Future::then()` rather than `Future::get()` to
avoid blocking the HgImporter threads on these operations.

Reviewed By: wez

Differential Revision: D8438777

fbshipit-source-id: a0d647b10ef5a182be2d19f636c2dbc24eab1b23

eden/fs/store/BackingStore.h

@@ -42,7 +42,7 @@ class BackingStore {
   virtual folly::Future<std::unique_ptr<Blob>> getBlob(const Hash& id) = 0;
   virtual folly::Future<std::unique_ptr<Tree>> getTreeForCommit(
       const Hash& commitID) = 0;
-  virtual folly::Future<folly::Unit> prefetchBlobs(
+  FOLLY_NODISCARD virtual folly::Future<folly::Unit> prefetchBlobs(
       const std::vector<Hash>& ids) const {
     return folly::unit;
   }

eden/fs/store/hg/HgBackingStore.cpp

@@ -21,6 +21,7 @@
 #include "eden/fs/store/LocalStore.h"
 #include "eden/fs/store/StoreResult.h"
 #include "eden/fs/store/hg/HgImporter.h"
+#include "eden/fs/store/hg/HgProxyHash.h"
 #include "eden/fs/utils/UnboundedQueueThreadPool.h"
 
 using folly::ByteRange;
@@ -153,14 +154,11 @@ Future<unique_ptr<Blob>> HgBackingStore::getBlob(const Hash& id) {
 
 folly::Future<folly::Unit> HgBackingStore::prefetchBlobs(
     const std::vector<Hash>& ids) const {
-  return folly::via(
-             importThreadPool_.get(),
-             [ids] {
-               getThreadLocalImporter().prefetchFiles(ids);
-               return folly::unit;
-             })
-      // Ensure that the control moves back to the main thread pool
-      // to process the caller-attached .then routine.
+  return HgProxyHash::getBatch(localStore_, ids)
+      .via(importThreadPool_.get())
+      .then([](std::vector<std::pair<RelativePath, Hash>>&& hgPathHashes) {
+        return getThreadLocalImporter().prefetchFiles(hgPathHashes);
+      })
       .via(serverThreadPool_);
 }
 

eden/fs/store/hg/HgBackingStore.h

@@ -54,7 +54,7 @@ class HgBackingStore : public BackingStore {
   folly::Future<std::unique_ptr<Blob>> getBlob(const Hash& id) override;
   folly::Future<std::unique_ptr<Tree>> getTreeForCommit(
       const Hash& commitID) override;
-  folly::Future<folly::Unit> prefetchBlobs(
+  FOLLY_NODISCARD folly::Future<folly::Unit> prefetchBlobs(
       const std::vector<Hash>& ids) const override;
 
  private:

eden/fs/store/hg/HgImporter.cpp

@@ -623,9 +623,8 @@ IOBuf HgImporter::importFileContents(Hash blobHash) {
   return buf;
 }
 
-void HgImporter::prefetchFiles(const std::vector<Hash>& blobHashes) {
-  auto files = HgProxyHash::getBatch(store_, blobHashes).get();
-
+void HgImporter::prefetchFiles(
+    const std::vector<std::pair<RelativePath, Hash>>& files) {
   sendPrefetchFilesRequest(files);
 
   // Read the response; throws if there was any error.

eden/fs/store/hg/HgImporter.h

@@ -69,7 +69,8 @@ class Importer {
    */
   virtual folly::IOBuf importFileContents(Hash blobHash) = 0;
 
-  virtual void prefetchFiles(const std::vector<Hash>& blobHashes) = 0;
+  virtual void prefetchFiles(
+      const std::vector<std::pair<RelativePath, Hash>>& files) = 0;
 };
 
 /**
@@ -134,7 +135,8 @@ class HgImporter : public Importer {
 
   std::unique_ptr<Tree> importTree(const Hash& id) override;
   folly::IOBuf importFileContents(Hash blobHash) override;
-  void prefetchFiles(const std::vector<Hash>& blobHashes) override;
+  void prefetchFiles(
+      const std::vector<std::pair<RelativePath, Hash>>& files) override;
 
   /**
    * Resolve the manifest node for the specified revision.

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

@@ -16,11 +16,13 @@
 #include "eden/fs/model/Tree.h"
 #include "eden/fs/model/TreeEntry.h"
 #include "eden/fs/store/MemoryLocalStore.h"
-#include "eden/fs/store/hg/HgImporter.h"
+#include "eden/fs/store/hg/HgBackingStore.h"
 #include "eden/fs/testharness/HgRepo.h"
 #include "eden/fs/utils/PathFuncs.h"
+#include "eden/fs/utils/UnboundedQueueThreadPool.h"
 
 using namespace facebook::eden;
+using namespace std::chrono_literals;
 using folly::StringPiece;
 using folly::test::TemporaryDirectory;
 using testing::HasSubstr;
@@ -169,14 +171,19 @@ usecunionstore=True
   EXPECT_THAT(
       catOutputs.second, HasSubstr("no remotefilelog server configured"));
 
+  // Build an HgBackingStore for this repository
+  UnboundedQueueThreadPool resultThreadPool(1, "ResultThread");
+  HgBackingStore store(clientRepo.path(), &localStore, &resultThreadPool);
+
   // Now test running prefetch
   // Build a list of file blob IDs to prefetch.
-  HgImporter importer(clientRepo.path(), &localStore);
-  auto rootTreeHash = importer.importManifest(commit2.toString());
-  auto rootTree = importer.importTree(rootTreeHash);
-  auto srcTree = importer.importTree(rootTree->getEntryAt("src"_pc).getHash());
-  auto edenTree = importer.importTree(srcTree->getEntryAt("eden"_pc).getHash());
-  auto fooTree = importer.importTree(rootTree->getEntryAt("foo"_pc).getHash());
+  auto rootTree = store.getTreeForCommit(commit2).get(10s);
+  auto srcTree =
+      store.getTree(rootTree->getEntryAt("src"_pc).getHash()).get(1s);
+  auto edenTree =
+      store.getTree(srcTree->getEntryAt("eden"_pc).getHash()).get(1s);
+  auto fooTree =
+      store.getTree(rootTree->getEntryAt("foo"_pc).getHash()).get(1s);
 
   std::vector<Hash> blobHashes;
   blobHashes.push_back(edenTree->getEntryAt("main.py"_pc).getHash());
@@ -187,8 +194,8 @@ usecunionstore=True
   blobHashes.push_back(fooTree->getEntryAt("bar.txt"_pc).getHash());
   blobHashes.push_back(fooTree->getEntryAt("test.txt"_pc).getHash());
 
-  // Call prefetch
-  importer.prefetchFiles(blobHashes);
+  // Call prefetchBlobs()
+  store.prefetchBlobs(blobHashes).get(10s);
 
   // Running "hg cat" with ssh disabled and no server repo should succeed now
   // that we have prefetched the data.