don't pass next inode number through thrift

Summary:
The desired end state for how next inode number gets passed across
takeover. This should not land as is - the prior diffs need to land
and wait for a while until everyone transitions.

Reviewed By: simpkins

Differential Revision: D8195550

fbshipit-source-id: 2fc40f881cc1a331df95ef99f7e9e4f2e69c8643

Author: chadaustin

eden/fs/inodes/EdenMount.cpp

@@ -196,10 +196,15 @@ folly::Future<folly::Unit> EdenMount::initialize(
                << "; max existing inode number is " << maxInodeNumber;
   } else {
     XLOG(DBG2) << "Initializing eden mount " << getPath() << " from takeover";
-    overlay_->setNextInodeNumber(
-        InodeNumber::fromThrift(takeover->nextInodeNumber));
+    if (!overlay_->hasInitializedNextInodeNumber()) {
+      XLOG(WARN) << "A clean shutdown before takeover did not leave an "
+                    "initialized inode number! Rescanning...";
+      overlay_->scanForNextInodeNumber();
+    }
   }
 
+  CHECK(overlay_->hasInitializedNextInodeNumber());
+
   return createRootInode(*parents).then(
       [this, parents, takeover](TreeInodePtr initTreeNode) {
         if (takeover) {
@@ -367,10 +372,8 @@ EdenMount::shutdownImpl(bool doTakeover) {
         // Close the Overlay object to make sure we have released its lock.
         // This is important during graceful restart to ensure that we have
         // released the lock before the new edenfs process begins to take over
-        // the mount piont.
-        inodeMap.nextInodeNumber = overlay_->close();
-        CHECK(inodeMap.nextInodeNumber)
-            << "nextInodeNumber should always be nonzero";
+        // the mount point.
+        overlay_->close();
         state_.store(State::SHUT_DOWN);
         return std::make_tuple(fileHandleMap, inodeMap);
       });

eden/fs/inodes/InodeMap.cpp

@@ -614,7 +614,6 @@ Future<SerializedInodeMap> InodeMap::shutdown(bool doTakeover) {
     }
 
     SerializedInodeMap result;
-    XLOG(DBG5) << "InodeMap::save nextInodeNumber: " << result.nextInodeNumber;
     result.unloadedInodes.reserve(data->unloadedInodes_.size());
     for (const auto& it : data->unloadedInodes_) {
       const auto& entry = it.second;

eden/fs/inodes/Overlay.cpp

@@ -95,11 +95,11 @@ Overlay::~Overlay() {
   close();
 }
 
-uint64_t Overlay::close() {
+void Overlay::close() {
   CHECK_NE(std::this_thread::get_id(), gcThread_.get_id());
 
   if (!infoFile_) {
-    return nextInodeNumber_.load(std::memory_order_relaxed);
+    return;
   }
 
   // Make sure everything is shut down in reverse of construction order.
@@ -113,8 +113,12 @@ uint64_t Overlay::close() {
   inodeMetadataTable_.reset();
   dirFile_.close();
   infoFile_.close();
+}
 
-  return nextInodeNumber_.load(std::memory_order_relaxed);
+bool Overlay::hasInitializedNextInodeNumber() const {
+  // nextInodeNumber_ is either 0 (uninitialized) or nonzero (initialized).
+  // It's only initialized on one thread, so relaxed loads are okay.
+  return 0 != nextInodeNumber_.load(std::memory_order_relaxed);
 }
 
 void Overlay::initOverlay() {
@@ -207,6 +211,8 @@ void Overlay::tryLoadNextInodeNumber() {
 }
 
 void Overlay::saveNextInodeNumber() {
+  // nextInodeNumber_ is either 0 (uninitialized) or nonzero (initialized).
+  // It's only initialized on one thread, so relaxed loads are okay.
   auto nextInodeNumber = nextInodeNumber_.load(std::memory_order_relaxed);
   if (!nextInodeNumber) {
     return;
@@ -292,7 +298,7 @@ void Overlay::initNewOverlay() {
       infoPath.stringPiece(), ByteRange(infoHeader.data(), infoHeader.size()));
 
   // kRootNodeId is reserved - start at the next one. No scan is necessary.
-  setNextInodeNumber(InodeNumber{kRootNodeId.get() + 1});
+  nextInodeNumber_.store(kRootNodeId.get() + 1, std::memory_order_relaxed);
 }
 
 void Overlay::ensureTmpDirectoryIsCreated() {
@@ -314,22 +320,6 @@ void Overlay::ensureTmpDirectoryIsCreated() {
   }
 }
 
-void Overlay::setNextInodeNumber(InodeNumber nextInodeNumber) {
-  if (auto ino = nextInodeNumber_.load(std::memory_order_relaxed)) {
-    // It's okay to allow setNextInodeNumber as long as the values are
-    // consistent. This code path will disappear when takeover transitions to
-    // relying on the Overlay efficiently remembering the next inode number
-    // itself.
-    DCHECK_EQ(ino, nextInodeNumber.get())
-        << "Overlay nextInodeNumber already initialized with " << ino
-        << " so it should not be initialized with " << nextInodeNumber;
-    return;
-  }
-
-  DCHECK_GT(nextInodeNumber, kRootNodeId);
-  nextInodeNumber_.store(nextInodeNumber.get(), std::memory_order_relaxed);
-}
-
 InodeNumber Overlay::allocateInodeNumber() {
   // InodeNumber should generally be 64-bits wide, in which case it isn't even
   // worth bothering to handle the case where nextInodeNumber_ wraps.  We don't
@@ -558,7 +548,7 @@ InodeNumber Overlay::scanForNextInodeNumber() {
     }
   }
 
-  setNextInodeNumber(InodeNumber{maxInode.get() + 1});
+  nextInodeNumber_.store(maxInode.get() + 1, std::memory_order_relaxed);
 
   return maxInode;
 }

eden/fs/inodes/Overlay.h

@@ -76,14 +76,14 @@ class Overlay {
    * Returns the next available InodeNumber to be passed to any process taking
    * over an Eden mount.
    */
-  uint64_t close();
+  void close();
 
   /**
-   * For now, this method exists to populate the next inode number from takeover
-   * data. Eventually, it will be unnecessary - the Overlay, upon a clean
-   * shutdown, will remember its next inode number in a file on disk.
+   * Returns true if the next inode number was initialized, either upon
+   * construction by loading the file left by a cleanly-closed Overlay, or by
+   * calling scanForNextInodeNumber().
    */
-  void setNextInodeNumber(InodeNumber nextInodeNumber);
+  bool hasInitializedNextInodeNumber() const;
 
   /**
    * Scans the Overlay for all inode numbers currently in use and sets the next

eden/fs/takeover/takeover.thrift

@@ -18,7 +18,6 @@ struct SerializedInodeMapEntry {
 }
 
 struct SerializedInodeMap {
-  1: i64 nextInodeNumber
   2: list<SerializedInodeMapEntry> unloadedInodes,
 }