diff --git a/eden/fs/inodes/ServerState.cpp b/eden/fs/inodes/ServerState.cpp index 46117b023390a..890340baeb035 100644 --- a/eden/fs/inodes/ServerState.cpp +++ b/eden/fs/inodes/ServerState.cpp @@ -7,6 +7,8 @@ #include "eden/fs/inodes/ServerState.h" +#include +#include #include #include @@ -64,6 +66,11 @@ ServerState::ServerState( privHelper_{std::move(privHelper)}, threadPool_{std::move(threadPool)}, fsChannelThreadPool_{std::move(fsChannelThreadPool)}, + validationThreadPool_{std::make_shared( + initialConfig.numVerifierThreads.getValue(), + std::make_unique< + folly::LifoSemMPMCQueue>( + 10 * initialConfig.maxNumberOfInvlidationsToVerify.getValue()))}, clock_{std::move(clock)}, processInfoCache_{std::move(processInfoCache)}, structuredLogger_{std::move(structuredLogger)}, diff --git a/eden/fs/inodes/ServerState.h b/eden/fs/inodes/ServerState.h index 2b5782faf3c9f..387f80af9caf4 100644 --- a/eden/fs/inodes/ServerState.h +++ b/eden/fs/inodes/ServerState.h @@ -153,6 +153,23 @@ class ServerState { return fsChannelThreadPool_; } + /** + * Gets a thread pool for running validation. Validation will read file + * contents through the filesystem. Reads through the filesystem can call + * back into EdenFS, so we need to ensure that validation does not block + * any of the threads that EdenFS uses to serve filesystem operations. + * + * It's pretty similar to the invalidation threadpool that the channels use. + * However, this thread pool also errors when reaches capacity rather than + * blocking. We want this threadpoool to be bounded because we don't want + * blocking here to increase memory usage until we OOM. Additionally, we don't + * want to block because this could block checkout. Validation is an + * asynchronous action that should not effect EdenFS behavior. + */ + const std::shared_ptr& getValidationThreadPool() const { + return validationThreadPool_; + } + /** * Get the Clock. */ @@ -216,6 +233,7 @@ class ServerState { std::shared_ptr privHelper_; std::shared_ptr threadPool_; std::shared_ptr fsChannelThreadPool_; + std::shared_ptr validationThreadPool_; std::shared_ptr clock_; std::shared_ptr processInfoCache_; std::shared_ptr structuredLogger_; diff --git a/eden/fs/service/EdenServer.cpp b/eden/fs/service/EdenServer.cpp index a3e8465f3e1b3..f90eb76f56154 100644 --- a/eden/fs/service/EdenServer.cpp +++ b/eden/fs/service/EdenServer.cpp @@ -64,6 +64,7 @@ #include "eden/fs/config/MountProtocol.h" #include "eden/fs/config/TomlConfig.h" #include "eden/fs/inodes/EdenMount.h" +#include "eden/fs/inodes/FileInode.h" #include "eden/fs/inodes/InodeAccessLogger.h" #include "eden/fs/inodes/InodeBase.h" #include "eden/fs/inodes/InodeMap.h" @@ -2119,6 +2120,128 @@ ImmediateFuture EdenServer::checkOutRevision( } }); } + // Verify if cache file invalidation works as expected after + // checkout + // TODO T213849128: This is to collect data for S439820. + // We can remove this once SEV close. + if (isNfs && + serverState_->getReloadableConfig() + ->getEdenConfig() + ->verifyFilesAfterCheckout.getValue()) { + std::vector inodesToValidate; + inodesToValidate.swap(result.sampleInodesToValidate); + folly::via( + this->getServerState()->getValidationThreadPool().get(), + [this, + inodesToValidate = std::move(inodesToValidate), + mountPath = mountPath.copy(), + maxSizeOfFileToVerifyInvalidation = + this->serverState_->getReloadableConfig() + ->getEdenConfig() + ->maxSizeOfFileToVerifyInvalidation.getValue(), + structuredLogger = + this->serverState_->getStructuredLogger()]() { + // for each inode + std::vector>>> + expectedContents; + { + // note we don't want to hold the mount handle in the + // capture because that would block shutdown until this + // future completes. This check can block waiting for the + // eden fuse threads to complete an fs request. We don't + // want to make a cycle, so to be safe we just attempt to + // lookup the mount and gracefully handle errors. + auto mountHandle = this->getMount(mountPath); + auto inodeMap = mountHandle.getEdenMount().getInodeMap(); + for (auto& inodeNum : inodesToValidate) { + auto inode = inodeMap->lookupLoadedInode(inodeNum); + if (!inode) { + // Skip if inode doesn't exist because it is already + // invalidated + continue; + } + auto path = inode->getPath(); + if (!path.has_value()) { + // skip if we can't get the path because we + // can't validate it anyway + continue; + } + if (auto fileInode = inode.asFile()) { + expectedContents.emplace_back( + fileInode + ->getSha1( + ObjectFetchContext::getNullContext()) + .thenValue( + [path = inode->getPath(), + inodeNum](auto&& sha1) mutable + -> std::tuple< + RelativePath, + InodeNumber, + std::optional> { + return { + std::move(path).value(), + inodeNum, + std::move(sha1)}; + })); + } else { + expectedContents.emplace_back(std::tuple< + RelativePath, + InodeNumber, + std::optional>( + std::move(path).value(), inodeNum, std::nullopt)); + } + } + } + + return collectAll(std::move(expectedContents)) + .thenValue([mountPath = mountPath.copy(), + maxSizeOfFileToVerifyInvalidation, + structuredLogger](auto&& expectedResults) { + for (auto& result : expectedResults) { + if (!result.hasValue()) { + continue; + } + auto [path, inodeNum, sha1Result] = result.value(); + auto fileStat = + getFileStat((mountPath + path).value().c_str()); + if (!fileStat.hasValue()) { + // Skip if file stat is not available + continue; + } else if ( + fileStat.value().size > + maxSizeOfFileToVerifyInvalidation) { + // Skip verification for large files + continue; + } + // sha1 the path + std::string contents; + folly::readFile( + (mountPath + path).value().c_str(), contents); + + auto actualSha1 = Hash20::sha1(contents); + + if (actualSha1 != sha1Result) { + // log to scuba. + structuredLogger->logEvent(StaleContents{ + path.value(), inodeNum.getRawValue()}); + std::string sha1ResultStr = + sha1Result ? sha1Result->toString() : "none"; + XLOG( + WARN, + "Stale file contents after checkout for path {} (ino {}) - actual sha1: {} , expected sha1: {}", + path.value(), + inodeNum.getRawValue(), + actualSha1.toString(), + sha1ResultStr); + } + } + }); + }); + } + return std::move(result); }); diff --git a/eden/fs/telemetry/LogEvent.h b/eden/fs/telemetry/LogEvent.h index 0c76399fc1bf5..f4fa5ccc675d8 100644 --- a/eden/fs/telemetry/LogEvent.h +++ b/eden/fs/telemetry/LogEvent.h @@ -262,6 +262,23 @@ struct FinishedCheckout : public EdenFSEvent { } }; +struct StaleContents : public EdenFSEvent { + std::string path; + uint64_t ino; + + explicit StaleContents(std::string path, uint64_t ino) + : path(std::move(path)), ino(ino) {} + + void populate(DynamicEvent& event) const override { + event.addString("path", path); + event.addInt("ino", ino); + } + + const char* getType() const override { + return "stale_contents"; + } +}; + struct FinishedMount : public EdenFSEvent { std::string backing_store_type; std::string repo_type;