enable NFS detection earlier

Summary:
Now that Windows is going to support NFS. We need a reliable way to check at
runtime if a file should be read from disk (Prjfs) or it should be read from
the overlay (nfs).

We read inodes during fsck which is before the mount is fully initialized, so
we need to be able to detect the mount type earlier than how we do right now.

Here I am moving the NFS detection before eden mount initialization.

While moving this I realized that the InodeMap's view if NFS is incorrect
in the takeover case, so I am also fixing that here.

Reviewed By: chadaustin

Differential Revision: D45020849

fbshipit-source-id: b0a8fd431a37174c81b0d053d92b8bac026bd0f1

eden/fs/inodes/EdenMount.cpp

@@ -258,11 +258,7 @@ EdenMount::EdenMount(
           serverState_->getEdenConfig()->prjfsNumInvalidationThreads.getValue(),
           "prjfs-dir-inval")},
 #endif
-      shouldUseNFSMount_{shouldUseNFSMount()},
-      inodeMap_{new InodeMap(
-          this,
-          serverState_->getReloadableConfig(),
-          shouldUseNFSMount_)},
+      inodeMap_{new InodeMap(this, serverState_->getReloadableConfig())},
       objectStore_{std::move(objectStore)},
       blobCache_{std::move(blobCache)},
       blobAccess_{objectStore_, blobCache_},
@@ -388,7 +384,12 @@ class TreeLookupProcessor {
 
 FOLLY_NODISCARD folly::Future<folly::Unit> EdenMount::initialize(
     OverlayChecker::ProgressCallback&& progressCallback,
-    const std::optional<SerializedInodeMap>& takeover) {
+    const std::optional<SerializedInodeMap>& takeover,
+    const std::optional<MountProtocol>& takeoverMountProtocol) {
+  // it is an invariant that shouldUseNfs_ is set before we transition to
+  // INITIALIZING
+  calculateIsNfsMount(takeoverMountProtocol);
+
   transitionState(State::UNINITIALIZED, State::INITIALIZING);
 
   auto parentCommit = checkoutConfig_->getParentCommit();
@@ -1251,6 +1252,53 @@ std::optional<MountProtocol> EdenMount::getMountProtocol() const {
   return std::nullopt;
 }
 
+void EdenMount::calculateIsNfsMount(
+    const std::optional<MountProtocol>& takeover) {
+  if (takeover) {
+    shouldUseNFSMount_ = takeover.value() == MountProtocol::NFS;
+  } else {
+    shouldUseNFSMount_ = getEdenConfig()->enableNfsServer.getValue() &&
+        getCheckoutConfig()->getMountProtocol() == MountProtocol::NFS;
+  }
+  // So this whole method should run before the mount is initialized.
+  XCHECK_LT(
+      folly::to_underlying(state_.load(std::memory_order_acquire)),
+      folly::to_underlying(State::INITIALIZING))
+      << "The invariant that shouldUseNFSMount_ should not be modified after "
+         "the mount has started initializing has been violated. This could "
+         "make calls to shouldBeOrIsNfsChannel unsafe.";
+}
+
+bool EdenMount::shouldBeOrIsNfsChannel() const {
+  XCHECK_GE(
+      folly::to_underlying(state_.load(std::memory_order_acquire)),
+      folly::to_underlying(State::INITIALIZING))
+      << "Though we guarantee that we won't modify shouldUseNFSMount_ after "
+         "after initialization begins. shouldUseNFSMount_ might be set any time "
+         "before initialization starts and we provide no explicit synchronization "
+         "on it, so it is not safe to access right now.";
+  XCHECK(shouldUseNFSMount_.has_value())
+      << "shouldUseNFSMount_ should have been set by this point. It is intended"
+         " that this is set before the mount begins initializing, and we only "
+         "access it after the mount has started initializing. ";
+  return shouldUseNFSMount_.value();
+}
+
+bool EdenMount::throwEstaleIfInodeIsMissing() const {
+  return shouldBeOrIsNfsChannel();
+}
+
+EdenMount::ReadLocation EdenMount::getReadLocationForMaterializedFiles() const {
+#ifdef _WIN32
+  if (!shouldBeOrIsNfsChannel()) {
+    // if we are on Windows and  the mount is not an NFS channel then it must be
+    // a prjfs one.
+    return ReadLocation::InRepo;
+  }
+#endif
+  return ReadLocation::Overlay;
+}
+
 ProcessAccessLog& EdenMount::getProcessAccessLog() const {
 #ifdef _WIN32
   return getPrjfsChannel()->getProcessAccessLog();
@@ -2083,7 +2131,7 @@ folly::Future<folly::Unit> EdenMount::fsChannelMount(bool readOnly) {
         AbsolutePath mountPath = getPath();
         auto edenConfig = getEdenConfig();
 
-        if (shouldUseNFSMount_) {
+        if (shouldBeOrIsNfsChannel()) {
           auto iosize = edenConfig->nfsIoSize.getValue();
           auto useReaddirplus = edenConfig->useReaddirplus.getValue();
 
@@ -2399,7 +2447,7 @@ void EdenMount::fsChannelInitSuccessful(
 void EdenMount::takeoverFuse(FuseChannelData takeoverData) {
 #ifndef _WIN32
   transitionState(State::INITIALIZED, State::STARTING);
-  shouldUseNFSMount_ = false;
+
   try {
     beginMount().setValue();
 
@@ -2426,7 +2474,6 @@ void EdenMount::takeoverFuse(FuseChannelData takeoverData) {
 folly::Future<folly::Unit> EdenMount::takeoverNfs(NfsChannelData takeoverData) {
 #ifndef _WIN32
   transitionState(State::INITIALIZED, State::STARTING);
-  shouldUseNFSMount_ = true;
   try {
     beginMount().setValue();
 

eden/fs/inodes/EdenMount.h

@@ -301,7 +301,8 @@ class EdenMount : public std::enable_shared_from_this<EdenMount> {
    */
   FOLLY_NODISCARD folly::Future<folly::Unit> initialize(
       OverlayChecker::ProgressCallback&& progressCallback = [](auto) {},
-      const std::optional<SerializedInodeMap>& takeover = std::nullopt);
+      const std::optional<SerializedInodeMap>& takeover = std::nullopt,
+      const std::optional<MountProtocol>& takeoverMountProtocol = std::nullopt);
 
   /**
    * Destroy the EdenMount.
@@ -435,6 +436,26 @@ class EdenMount : public std::enable_shared_from_this<EdenMount> {
   bool fsChannelIsInitialized() const;
   std::optional<MountProtocol> getMountProtocol() const;
 
+  /**
+   * This allows earlier detection if this is an NFS mount than isNfsdChannel.
+   *
+   * This should only be called after the mount is "Initializing". (It is the
+   * caller's responsibility to perform proper synchronization here with the
+   * mount start operation.  This method provides no internal synchronization of
+   * its own.)
+   */
+  bool shouldBeOrIsNfsChannel() const;
+
+  bool throwEstaleIfInodeIsMissing() const;
+
+  enum class ReadLocation { InRepo, Overlay };
+
+  /**
+   * Depending on the channel type materialized files (and directories in some
+   * cases) should be read directly out of the repo (ProjFS).
+   */
+  ReadLocation getReadLocationForMaterializedFiles() const;
+
   /**
    * Wait for all inflight notifications to complete.
    *
@@ -1039,18 +1060,17 @@ class EdenMount : public std::enable_shared_from_this<EdenMount> {
       const ObjectFetchContextPtr& context);
 
   /**
-   * Should only be called by the mount contructor. We decide wether this
-   * mount should use nfs at construction time and do not change the decision.
-   * This is so that we can consitently determine if we are determining if we
-   * are using an nfs mount without checking if the channel is an NFS mount.
-   * Needed because the InodeMap which is a dependency of ourselves needs to be
-   * NFS aware. We don't want a dependency inversion where the inode map relies
-   * on the mount to determine if its an NFS inode map.
+   * Should only be called before the EdenMount transitions to the INITIALIZING
+   * state.
+   * We decide wether this mount should use nfs before initialization time and
+   * do not change the decision. This is so that we can consitently determine if
+   * we are determining if we are using an nfs mount without checking if the
+   * channel is an NFS mount. Needed because we need to know the type of mount
+   * earlier than channel object creation. This is for responding to stale
+   * inodes specially for NFS and reading inodes specially off nfs on windows.
    */
-  bool shouldUseNFSMount() {
-    return getEdenConfig()->enableNfsServer.getValue() &&
-        getCheckoutConfig()->getMountProtocol() == MountProtocol::NFS;
-  }
+  void calculateIsNfsMount(
+      const std::optional<MountProtocol>& takeoverMountProtocol);
   /**
    * Clear the fs reference count for all stale inodes. Stale inodes are those
    * that have been unlinked and not recently referenced.
@@ -1243,7 +1263,7 @@ class EdenMount : public std::enable_shared_from_this<EdenMount> {
    * Windows). We calculate this when the mount is created based on the
    * underlying dynamic configuration.
    */
-  bool shouldUseNFSMount_;
+  std::optional<bool> shouldUseNFSMount_{std::nullopt};
 
   std::unique_ptr<InodeMap> inodeMap_;
 

eden/fs/inodes/InodeMap.cpp

@@ -99,13 +99,8 @@ InodeType InodeMap::UnloadedInode::getInodeType() const {
   return S_ISDIR(mode) ? InodeType::TREE : InodeType::FILE;
 }
 
-InodeMap::InodeMap(
-    EdenMount* mount,
-    std::shared_ptr<ReloadableConfig> config,
-    bool throwEstaleIfInodeIsMissing)
-    : mount_{mount},
-      config_{std::move(config)},
-      throwEstaleIfInodeIsMissing_{throwEstaleIfInodeIsMissing} {}
+InodeMap::InodeMap(EdenMount* mount, std::shared_ptr<ReloadableConfig> config)
+    : mount_{mount}, config_{std::move(config)} {}
 
 InodeMap::~InodeMap() {
   // TODO: We need to clean up the EdenMount / InodeMap destruction process a
@@ -281,7 +276,7 @@ ImmediateFuture<InodePtr> InodeMap::lookupInode(InodeNumber number) {
   // Look up the data in the unloadedInodes_ map.
   auto unloadedIter = data->unloadedInodes_.find(number);
   if (UNLIKELY(unloadedIter == data->unloadedInodes_.end())) {
-    if (throwEstaleIfInodeIsMissing_) {
+    if (mount_->throwEstaleIfInodeIsMissing()) {
       XLOG(DBG3) << "NFS inode " << number << " stale";
       // windows does not have ESTALE. We need some other error to turn into the
       // nfs stale error. For now let's just let it throw.

eden/fs/inodes/InodeMap.h

@@ -98,10 +98,7 @@ class InodeMap {
  public:
   using PromiseVector = std::vector<folly::Promise<InodePtr>>;
 
-  explicit InodeMap(
-      EdenMount* mount,
-      std::shared_ptr<ReloadableConfig> config,
-      bool throwEstaleIfInodeIsMissing);
+  explicit InodeMap(EdenMount* mount, std::shared_ptr<ReloadableConfig> config);
   virtual ~InodeMap();
 
   InodeMap(InodeMap&&) = delete;
@@ -747,15 +744,6 @@ class InodeMap {
    */
   folly::Synchronized<Members> data_;
 
-  /**
-   * This boolean controls EdenFS's response to receiving a request for an
-   * unknown inode. When this is true ESTALE is thrown. When this is false
-   * this will abruptly terminate the EdenFS process. This should be set on
-   * platforms that need to be forgiving of old inode numbers being used (ex.
-   * NFSv3).
-   */
-  bool throwEstaleIfInodeIsMissing_;
-
   /**
    * The number of inodes that we have unloaded with our periodic
    * unlinked inode unloading. Periodic unlinked inode unloading is run after

eden/fs/service/EdenServer.cpp

@@ -39,6 +39,7 @@
 
 #include "eden/common/utils/ProcessNameCache.h"
 #include "eden/fs/config/CheckoutConfig.h"
+#include "eden/fs/config/MountProtocol.h"
 #include "eden/fs/config/TomlConfig.h"
 #include "eden/fs/fuse/privhelper/PrivHelper.h"
 #include "eden/fs/inodes/EdenMount.h"
@@ -67,6 +68,7 @@
 #include "eden/fs/store/TreeCache.h"
 #include "eden/fs/store/hg/HgBackingStore.h"
 #include "eden/fs/store/hg/HgQueuedBackingStore.h"
+#include "eden/fs/takeover/TakeoverData.h"
 #include "eden/fs/telemetry/EdenStats.h"
 #include "eden/fs/telemetry/IHiveLogger.h"
 #include "eden/fs/telemetry/RequestMetricsScope.h"
@@ -1529,6 +1531,8 @@ folly::Future<std::shared_ptr<EdenMount>> EdenServer::mount(
   auto initializeFuture = edenMount->initialize(
       std::move(progressCallback),
       doTakeover ? std::make_optional(optionalTakeover->inodeMap)
+                 : std::nullopt,
+      doTakeover ? std::make_optional(optionalTakeover->getMountProtocol())
                  : std::nullopt);
   return std::move(initializeFuture)
       .thenValue([this,

eden/fs/takeover/TakeoverData.cpp

@@ -5,8 +5,6 @@
  * GNU General Public License version 2.
  */
 
-#ifndef _WIN32
-
 #include "eden/fs/takeover/TakeoverData.h"
 
 #include <memory>
@@ -29,13 +27,13 @@ using folly::IOBuf;
 using std::string;
 
 namespace facebook::eden {
-
+#ifndef _WIN32
 namespace {
 
 /**
  * Determines the mount protocol for the mount point encoded in the mountInfo.
  */
-TakeoverMountProtocol getMountProtocol(
+TakeoverMountProtocol getTakeoverMountProtocol(
     const TakeoverData::MountInfo& mountInfo) {
   if (std::holds_alternative<FuseChannelData>(mountInfo.channelInfo)) {
     return TakeoverMountProtocol::FUSE;
@@ -49,6 +47,24 @@ TakeoverMountProtocol getMountProtocol(
 }
 
 } // namespace
+#endif
+
+MountProtocol TakeoverData::MountInfo::getMountProtocol() const {
+  if (std::holds_alternative<FuseChannelData>(channelInfo)) {
+    return MountProtocol::FUSE;
+  } else if (std::holds_alternative<NfsChannelData>(channelInfo)) {
+    return MountProtocol::NFS;
+  } else if (std::holds_alternative<ProjFsChannelData>(channelInfo)) {
+    return MountProtocol::PRJFS;
+  }
+
+  throwf<std::runtime_error>(
+      "unrecognized mount protocol {} for mount: {}",
+      channelInfo.index(),
+      mountPath);
+}
+
+#ifndef _WIN32
 
 const std::set<int32_t> kSupportedTakeoverVersions{
     TakeoverData::kTakeoverProtocolVersionThree,
@@ -517,7 +533,7 @@ IOBuf TakeoverData::serializeThrift(uint64_t protocolCapabilities) {
 
   std::vector<SerializedMountInfo> serializedMounts;
   for (const auto& mount : mountPoints) {
-    auto mountProtocol = getMountProtocol(mount);
+    auto mountProtocol = getTakeoverMountProtocol(mount);
 
     checkCanSerDeMountType(
         protocolCapabilities, mountProtocol, mount.mountPath.view());
@@ -702,7 +718,6 @@ TakeoverData TakeoverData::deserializeThriftMounts(
   }
   return data;
 }
+#endif
 
 } // namespace facebook::eden
-
-#endif

eden/fs/takeover/TakeoverData.h

@@ -15,6 +15,7 @@
 #include <folly/futures/Promise.h>
 #include <folly/io/Cursor.h>
 
+#include "eden/fs/config/MountProtocol.h"
 #include "eden/fs/takeover/gen-cpp2/takeover_types.h"
 #include "eden/fs/utils/FsChannelTypes.h"
 #include "eden/fs/utils/FutureUnixSocket.h"
@@ -256,6 +257,8 @@ class TakeoverData {
           channelInfo{std::move(projfsChannelData)},
           inodeMap{std::move(inodeMap)} {}
 
+    MountProtocol getMountProtocol() const;
+
     AbsolutePath mountPath;
     AbsolutePath stateDirectory;
     std::vector<AbsolutePath> bindMounts;

eden/fs/testharness/TestMount.cpp

@@ -508,7 +508,11 @@ void TestMount::remountGracefully() {
       serverState_,
       std::move(journal),
       stats_.copy());
-  edenMount_->initialize([](auto) {}, takeoverData)
+  edenMount_
+      ->initialize(
+          [](auto) {},
+          takeoverData,
+          std::optional<MountProtocol>(MountProtocol::FUSE))
       .getVia(serverExecutor_.get());
   rootInode_ = edenMount_->getRootInodeUnchecked();
 }