allow loading unlinked inodes

Summary:
This implements a TODO/FATAL that is important for
graceful restarts to be useful in my "acid test" scenario,
which is to perform a graceful restart while buck build is
running.

Reviewed By: simpkins

Differential Revision: D6700189

fbshipit-source-id: dec1b818ebc9e907841bc127ee08c953b59d6487

eden/fs/fuse/handlemap.thrift

@@ -16,6 +16,8 @@ struct SerializedInodeMapEntry {
   3: string name,
   4: bool isUnlinked,
   5: i64 numFuseReferences,
+  6: string hash,
+  7: i32 mode,
 }
 
 struct SerializedInodeMap {

eden/fs/inodes/InodeBase.cpp

@@ -191,6 +191,12 @@ std::string InodeBase::getLogPath() const {
   return std::move(path).value();
 }
 
+void InodeBase::markUnlinkedAfterLoad() {
+  auto loc = location_.wlock();
+  DCHECK(!loc->unlinked);
+  loc->unlinked = true;
+}
+
 std::unique_ptr<InodeBase> InodeBase::markUnlinked(
     TreeInode* parent,
     PathComponentPiece name,

eden/fs/inodes/InodeBase.h

@@ -204,6 +204,14 @@ class InodeBase {
       PathComponentPiece name,
       const RenameLock& renameLock);
 
+  /**
+   * This method should only be called by TreeInode::loadUnlinkedChildInode().
+   * Its purpose is to set the unlinked flag to true for inodes that have
+   * been unlinked and passed over to the current process as part of a
+   * graceful restart procedure.
+   */
+  void markUnlinkedAfterLoad();
+
   /**
    * updateLocation() should only be invoked by TreeInode.
    *

eden/fs/inodes/InodeMap.cpp

@@ -18,6 +18,7 @@
 #include "eden/fs/inodes/Overlay.h"
 #include "eden/fs/inodes/ParentInodeInfo.h"
 #include "eden/fs/inodes/TreeInode.h"
+#include "eden/fs/service/ThriftUtil.h"
 #include "eden/fs/utils/Bug.h"
 
 using folly::Future;
@@ -126,11 +127,18 @@ Future<InodePtr> InodeMap::lookupInode(fusell::InodeNumber number) {
       InodePtr firstLoadedParent = InodePtr::newPtrLocked(loadedIter->second);
       PathComponent requiredChildName = unloadedData->name;
       bool isUnlinked = unloadedData->isUnlinked;
+      auto optionalHash = unloadedData->hash;
+      auto mode = unloadedData->mode;
       // Unlock the data before starting the child lookup
       data.unlock();
       // Trigger the lookup, then return to our caller.
       startChildLookup(
-          firstLoadedParent, requiredChildName, isUnlinked, childInodeNumber);
+          firstLoadedParent,
+          requiredChildName,
+          isUnlinked,
+          childInodeNumber,
+          optionalHash,
+          mode);
       return result;
     }
 
@@ -158,7 +166,9 @@ Future<InodePtr> InodeMap::lookupInode(fusell::InodeNumber number) {
         parentData->promises.back(),
         unloadedData->name,
         unloadedData->isUnlinked,
-        childInodeNumber);
+        childInodeNumber,
+        unloadedData->hash,
+        unloadedData->mode);
 
     if (alreadyLoading) {
       // This parent is already being loaded.
@@ -176,13 +186,18 @@ void InodeMap::setupParentLookupPromise(
     Promise<InodePtr>& promise,
     PathComponentPiece childName,
     bool isUnlinked,
-    fusell::InodeNumber childInodeNumber) {
+    fusell::InodeNumber childInodeNumber,
+    folly::Optional<Hash> hash,
+    mode_t mode) {
   promise.getFuture()
-      .then(
-          [name = PathComponent(childName), this, isUnlinked, childInodeNumber](
-              const InodePtr& inode) {
-            startChildLookup(inode, name, isUnlinked, childInodeNumber);
-          })
+      .then([name = PathComponent(childName),
+             this,
+             isUnlinked,
+             childInodeNumber,
+             hash,
+             mode](const InodePtr& inode) {
+        startChildLookup(inode, name, isUnlinked, childInodeNumber, hash, mode);
+      })
       .onError([this, childInodeNumber](const folly::exception_wrapper& ex) {
         // Fail all pending lookups on the child
         inodeLoadFailed(childInodeNumber, ex);
@@ -193,7 +208,9 @@ void InodeMap::startChildLookup(
     const InodePtr& parent,
     PathComponentPiece childName,
     bool isUnlinked,
-    fusell::InodeNumber childInodeNumber) {
+    fusell::InodeNumber childInodeNumber,
+    folly::Optional<Hash> hash,
+    mode_t mode) {
   auto treeInode = parent.asTreePtrOrNull();
   if (!treeInode) {
     auto bug = EDEN_BUG() << "parent inode " << parent->getNodeId() << " of ("
@@ -208,9 +225,8 @@ void InodeMap::startChildLookup(
   }
 
   if (isUnlinked) {
-    // FIXME: The UnloadedData object needs to have enough data to recreate
-    // this object, or we need to be able to load the info from the overlay.
-    XLOG(FATAL) << "reloading unlinked inodes not implemented yet";
+    treeInode->loadUnlinkedChildInode(childName, childInodeNumber, hash, mode);
+    return;
   }
 
   // Ask the TreeInode to load this child inode.
@@ -423,6 +439,8 @@ SerializedInodeMap InodeMap::save() {
     serializedEntry.name = entry.name.stringPiece().str();
     serializedEntry.isUnlinked = entry.isUnlinked;
     serializedEntry.numFuseReferences = entry.numFuseReferences;
+    serializedEntry.hash = thriftHash(entry.hash);
+    serializedEntry.mode = entry.mode;
 
     result.unloadedInodes.emplace_back(std::move(serializedEntry));
   }
@@ -450,6 +468,11 @@ void InodeMap::load(const SerializedInodeMap& takeover) {
       throw std::runtime_error(message);
     }
     unloadedEntry.isUnlinked = entry.isUnlinked;
+    if (!entry.hash.empty()) {
+      unloadedEntry.hash = hashFromThrift(entry.hash);
+    }
+    unloadedEntry.mode = entry.mode;
+
     auto result = data->unloadedInodes_.emplace(
         entry.inodeNumber, std::move(unloadedEntry));
     if (!result.second) {
@@ -645,6 +668,21 @@ void InodeMap::unloadInode(
         UnloadedInode(inode->getNodeId(), parent->getNodeId(), name);
     unloadedEntry.numFuseReferences = fuseCount;
     unloadedEntry.isUnlinked = isUnlinked;
+
+    auto* asTree = dynamic_cast<const TreeInode*>(inode);
+    auto* asFile = dynamic_cast<const FileInode*>(inode);
+    if (asTree) {
+      unloadedEntry.hash = asTree->getContents()->treeHash;
+      // There is no asTree->getMode() we can call,
+      // however, directories are always represented with
+      // this specific mode bit pattern in eden so we can
+      // force the value down here.
+      unloadedEntry.mode = S_IFDIR | 0755;
+    } else {
+      unloadedEntry.hash = asFile->getBlobHash();
+      unloadedEntry.mode = asFile->getMode();
+    }
+
     auto reverseInsertKey =
         std::make_pair(unloadedEntry.parent, unloadedEntry.name.piece());
     XLOG(DBG7) << "reverse unload emplace " << reverseInsertKey.first << ":"

eden/fs/inodes/InodeMap.h

@@ -18,6 +18,7 @@
 #include "eden/fs/fuse/FuseChannel.h"
 #include "eden/fs/fuse/gen-cpp2/handlemap_types.h"
 #include "eden/fs/inodes/InodePtr.h"
+#include "eden/fs/model/Hash.h"
 #include "eden/fs/utils/PathFuncs.h"
 
 namespace folly {
@@ -446,6 +447,18 @@ class InodeMap {
      */
     bool isUnlinked{false};
 
+    /** The complete st_mode value for this entry */
+    mode_t mode{0};
+
+    /**
+     * If the entry is not materialized, this contains the hash
+     * identifying the source control Tree (if this is a directory) or Blob
+     * (if this is a file) that contains the entry contents.
+     *
+     * If the entry is materialized, this field is not set.
+     */
+    folly::Optional<Hash> hash;
+
     /**
      * A list of promises waiting on this inode to be loaded.
      *
@@ -507,12 +520,16 @@ class InodeMap {
       folly::Promise<InodePtr>& promise,
       PathComponentPiece childName,
       bool isUnlinked,
-      fusell::InodeNumber childInodeNumber);
+      fusell::InodeNumber childInodeNumber,
+      folly::Optional<Hash> hash,
+      mode_t mode);
   void startChildLookup(
       const InodePtr& parent,
       PathComponentPiece childName,
       bool isUnlinked,
-      fusell::InodeNumber childInodeNumber);
+      fusell::InodeNumber childInodeNumber,
+      folly::Optional<Hash> hash,
+      mode_t mode);
 
   /**
    * Extract the list of promises waiting on the specified inode number to be

eden/fs/inodes/TreeInode.cpp

@@ -336,6 +336,65 @@ fusell::InodeNumber TreeInode::getChildInodeNumber(PathComponentPiece name) {
   return inodeNumber;
 }
 
+void TreeInode::loadUnlinkedChildInode(
+    PathComponentPiece name,
+    fusell::InodeNumber number,
+    folly::Optional<Hash> hash,
+    mode_t mode) {
+  try {
+    InodeMap::PromiseVector promises;
+    InodePtr inodePtr;
+
+    if (!S_ISDIR(mode)) {
+      auto file = std::make_unique<FileInode>(
+          number, inodePtrFromThis(), name, mode, hash);
+      promises = getInodeMap()->inodeLoadComplete(file.get());
+      inodePtr = InodePtr::newPtrLocked(file.release());
+    } else {
+      Dir dir;
+
+      if (hash) {
+        // Copy in the hash but we leave dir.entries empty
+        // because a directory can only be unlinked if it
+        // is empty.
+        dir.treeHash = hash;
+      } else {
+        // Note that the .value() call will throw if we couldn't
+        // load the dir data; we'll catch and propagate that in
+        // the containing try/catch block.
+        dir = getOverlay()->loadOverlayDir(number).value();
+
+        if (!dir.entries.empty()) {
+          // Should be impossible, but worth checking for
+          // defensive purposes!
+          throw new std::runtime_error(
+              "unlinked dir inode should have no children");
+        }
+      }
+
+      auto tree = std::make_unique<TreeInode>(
+          number, inodePtrFromThis(), name, std::move(dir));
+      promises = getInodeMap()->inodeLoadComplete(tree.get());
+      inodePtr = InodePtr::newPtrLocked(tree.release());
+    }
+
+    inodePtr->markUnlinkedAfterLoad();
+
+    // Alert any waiters that the load is complete
+    for (auto& promise : promises) {
+      promise.setValue(inodePtr);
+    }
+
+  } catch (const std::exception& exc) {
+    auto bug = EDEN_BUG() << "InodeMap requested to load inode " << number
+                          << "(" << name << " in " << getLogPath()
+                          << "), which has been unlinked, and we hit this "
+                          << "error while trying to load it from the overlay: "
+                          << exc.what();
+    getInodeMap()->inodeLoadFailed(number, bug.toException());
+  }
+}
+
 void TreeInode::loadChildInode(
     PathComponentPiece name,
     fusell::InodeNumber number) {

eden/fs/inodes/TreeInode.h

@@ -475,6 +475,21 @@ class TreeInode : public InodeBase {
    */
   void loadChildInode(PathComponentPiece name, fusell::InodeNumber number);
 
+  /**
+   * Internal API only for use by InodeMap.
+   *
+   * InodeMap will this API when a child inode that has been unlinked
+   * needs to be loaded.
+   *
+   * The TreeInode will call InodeMap::inodeLoadComplete() or
+   * InodeMap::inodeLoadFailed() when the load finishes.
+   */
+  void loadUnlinkedChildInode(
+      PathComponentPiece name,
+      fusell::InodeNumber number,
+      folly::Optional<Hash> hash,
+      mode_t mode);
+
   /**
    * Unload all unreferenced children under this tree (recursively).
    *

eden/integration/restart_test.py

@@ -21,6 +21,7 @@ class RestartTest:
         self.page1 = "1" * self.pagesize
         self.page2 = "2" * self.pagesize
         self.repo.write_file('hello', self.page1 + self.page2)
+        self.repo.write_file('deleted', self.page1 + self.page2)
         self.repo.commit('Initial commit.')
 
     def edenfs_logging_settings(self):
@@ -28,7 +29,36 @@ class RestartTest:
 
     def test_restart(self):
         hello = os.path.join(self.mount, 'hello')
-        with open(hello, 'r') as f:
+        deleted = os.path.join(self.mount, 'deleted')
+        deleted_local = os.path.join(self.mount, 'deleted-local')
+
+        # To test our handling of unlinked inodes, in addition
+        # to unlinking something that is in the manifest we
+        # need to check that we handle the case of a local
+        # file being deleted to make sure that we cover both
+        # code paths for FileInode.
+        with open(deleted_local, 'w') as dl:
+            dl.write(self.page1)
+            dl.write(self.page2)
+
+        # We'd like to make sure that we do something reasonable
+        # for directories that have been unlinked and that are
+        # still referenced via a file descriptor.  Ideally we'd call
+        # opendir() here and then readdir() it after we've performed
+        # the graceful restart, but we can't directly call those
+        # functions from python.  The approach used here is to
+        # open a file descriptor to the directory and then try
+        # to stat() it after the restart.  Since the directory
+        # has to be empty in order to be unlinked, a readdir
+        # from it wouldn't return any interesting results anyway.
+        deleted_dir = os.path.join(self.mount, 'deleted-dir')
+        os.mkdir(deleted_dir)
+        deleted_dir_fd = os.open(deleted_dir, 0)
+        os.rmdir(deleted_dir)
+
+        with open(hello, 'r') as f, \
+             open(deleted, 'r') as d, \
+             open(deleted_local, 'r') as dl:
             # Read the first page only (rather than the whole file)
             # before we restart the process.
             # This is so that we can check that the kernel really
@@ -37,6 +67,13 @@ class RestartTest:
             # it isn't just getting served from the kernel buffer cache
             self.assertEqual(self.page1, f.read(self.pagesize))
 
+            # Let's make sure that unlinked inodes continue to
+            # work appropriately too.  We've opened the file
+            # handles and are holding them alive in `d` and `dl`,
+            # so now let's unlink it from the filesystem
+            os.unlink(deleted)
+            os.unlink(deleted_local)
+
             print('=== beginning restart ===', file=sys.stderr)
             self.eden.graceful_restart()
             print('=== restart complete ===', file=sys.stderr)
@@ -47,7 +84,27 @@ class RestartTest:
             self.assertEqual(self.page1, f.read(self.pagesize))
             self.assertEqual(self.page2, f.read(self.pagesize))
 
-        # Let's also testing opening the same file up again,
+            # We should be able to read from the `d` file handle
+            # even though we deleted the file from the tree
+            self.assertEqual(self.page1, d.read(self.pagesize))
+            self.assertEqual(self.page2, d.read(self.pagesize))
+            # Likewise for the `dl` file handle
+            self.assertEqual(self.page1, dl.read(self.pagesize))
+            self.assertEqual(self.page2, dl.read(self.pagesize))
+
+        # Now check that the unlinked directory handle still seems
+        # connected.  This is difficult to do directly in python;
+        # the directory had to be empty in order to be removed
+        # so even if we could read its directory entries there
+        # wouldn't be anything to read.
+        # Note that os.stat() will throw if the fd is deemed
+        # bad either by the kernel or the eden instance,
+        # so we're just calling it and discarding the return
+        # value.
+        os.stat(deleted_dir_fd)
+        os.close(deleted_dir_fd)
+
+        # Let's also test opening the same file up again,
         # just to make sure that that is still working after
         # the graceful restart.
         with open(hello, 'r') as f: