Summary:
DurhamG discovered that setting max_background in fuse_init_out allows
a larger number of concurrent FUSE requests. The documentation
indicates it's intended to affect background requests like readahead,
but empirically you can observe increased live FUSE requests with `rg
-j200` and `eden top`. Default to 1000 because EdenFS can handle a
large amount of concurrency and we want to avoid blob and tree fetches
to block FUSE IO when not necessary.
Reviewed By: xavierd
Differential Revision: D27526032
fbshipit-source-id: 0d3fa383772f719524a9be84b73fa2eb599580d7
Summary:
gtest includes some windows headers that will have conflicts with the
folly portability versions. This caused some issues in my in-memory tree
cache diffs (D27050310 (8a1a529fcc)).
We should probably generally be using the folly portable gtests so we can
avoid such issues in the future.
see here for more details: bd600cd4e8/folly/portability/GTest.h (L19)
I ran this with codemod yes to all
- convert all the includes with quotes:
`codemod -d eden/fs --extensions cpp,h '\#include\ "gtest/gtest\.h"' '#include <folly/portability/GTest.h>'`
- convert all the includes with brackets
`codemod -d eden/fs --extensions cpp,h '\#include\ <gtest/gtest\.h>' '#include <folly/portability/GTest.h>'`
- convert the test template
`codemod -d eden/facebook --extensions template '\#include\ <gtest/gtest\.h>' '#include <folly/portability/GTest.h>'`
then used `arc lint` to clean up all the targets files
Reviewed By: genevievehelsel, xavierd
Differential Revision: D28035146
fbshipit-source-id: c3b88df5d4e7cdf4d1e51d9689987ce039f47fde
Summary: `CaseSensitivity::Sensitive` is better than a mere `true` out of nowhere.
Reviewed By: kmancini
Differential Revision: D27867180
fbshipit-source-id: 39d21d3cc3b70c78c6984c3ddbd65b53520770be
Summary:
The world has moved on utf-8 as the default encoding for files and data, but
EdenFS still accepts non utf-8 filenames to be written to it. In fact, most of
the time when a non utf-8 file is written to the working copy, and even though
EdenFS handles it properly, Mercurial ends up freaking out and crash. In all of
these cases, non-utf8 files were not intentional, and thus refusing to create
them wouldn't be a loss of functionality.
Note that this diff makes the asumption that Mercurial's manifest only accept
utf8 path, and thus we only have to protect against files being created in the
working copy that aren't utf8.
The unfortunate part of this diff is that it makes importing trees a bit more
expensive as testing that a path is utf8 valid is not free.
Reviewed By: chadaustin
Differential Revision: D25442975
fbshipit-source-id: 89341a004272736a61639751da43c2e9c673d5b3
Summary:
By passing the argument to the channel, we can make it so that the NFS code
correctly replies to whether it is case sensitive or not.
Reviewed By: kmancini
Differential Revision: D26500112
fbshipit-source-id: 2988eae403ff3648b50a1a8f0c978be2828ba568
Summary:
Instead of having one "Dispatcher" type that the various backend overload,
let's simply have a per-mount type dispatcher type. The previous model worked
fine when EdenFS supported only one way of mounting a repository, but with NFS
coming, unix platform will support both FUSE and NFS, making the Dispatcher
overload nonsensical.
As a behavioral change, the dispatcher lifetime and ownership is changed a bit.
It used to live for the duration of the EdenMount object, but is now tied to
the channel lifetime, as it is now owned by it.
Reviewed By: kmancini
Differential Revision: D26329477
fbshipit-source-id: 3959b90a4909e3ab0898caa308f54686f59a943c
Summary:
Instead of logging in the Dispatcher, move strace logging to
FuseChannel where it can be standardized for all FUSE request types.
Reviewed By: wez
Differential Revision: D24035838
fbshipit-source-id: c84d8c27b62f9944e2d26a35a7ed7bbbeeb5bf0e
Summary:
None of these were used, let's remove them.
ps: I thought we had a system to detect unused headers and lint about them?
Reviewed By: chadaustin
Differential Revision: D23465783
fbshipit-source-id: c21a34c9838db29f4fd0057d3be4e0fcb527cd6d
Summary:
Avoid some overhead and complexity by storing BufVec as a
unique_ptr<IOBuf>. The complexity can be reintroduced if we ever find
FUSE splice support to be a performance win for us.
Reviewed By: kmancini
Differential Revision: D22710795
fbshipit-source-id: e58eedc0fb5cea9e9743ccd20d3e4e2b7cc5d198
Summary: Tracing was not an accurate name for what this directory had become. So rename it to telemetry.
Reviewed By: wez
Differential Revision: D17923303
fbshipit-source-id: fca07e8447d9b9b3ea5d860809a2d377e3c4f9f2
Summary:
Update the copyright & license headers in C++ files to reflect the
relicensing to GPLv2
Reviewed By: wez
Differential Revision: D15487078
fbshipit-source-id: 19f24c933a64ecad0d3a692d0f8d2a38b4194b1d
Summary:
I'm confused by the naming of EdenThreadStats and ThreadLocalEdenStats. For example, when I see "ThreadLocalEdenStats", I think that such objects can only be accessed by one thread. That's definitely not what "ThreadLocalEdenStats" means!
I think the following naming scheme makes more sense:
* **EdenThreadStats**: Statistics which are updated by one thread. Currently called EdenThreadStats.
* **EdenStats**: Statistics for all threads. Provides access to EdenThreadStats. Currently called ThreadLocalEdenStats.
Implement my preferred scheme: rename ThreadLocalEdenStats to EdenStats.
This diff should not change behavior.
Note: Prior to D14822274, EdenThreadStats was called EdenStats.
Reviewed By: simpkins
Differential Revision: D14822271
fbshipit-source-id: bd20179b1010588e3fc16dc9ed0657d458606f16
Summary:
I want to use EdenStats in eden/fs/store/. EdenStats currently lives in eden/fs/fuse/, and making eden/fs/store/ depend upon eden/fs/fuse/ is confusing. (It's also confusing that some code in eden/fs/fuse/ is used on Windows.)
Reorganize the code: move EdenStats into eden/fs/tracing/.
This diff should not change behavior.
Reviewed By: chadaustin
Differential Revision: D14677337
fbshipit-source-id: af26d214bcc3a9919920fbd4e59e6098fe4e3834
Summary:
FuseChannel::initialize throws runtime_error when the FUSE connection is interrupted. I want EdenMount::startFuse to handle unexpected-unmount errors from FuseChannel::initialize, but catching runtime_error would catch unrelated errors too.
When the FUSE connection is interrupted during initialization, make FuseChannel throw FuseDeviceUnmountedDuringInitialization instead of runtime_error.
Reviewed By: chadaustin
Differential Revision: D14077848
fbshipit-source-id: ed7b7d370a83ed1a9c36a443d8bb06ba940dc032
Summary:
Now that the deadlock in ProcessNameCache has been fixed, bring it
back.
Reviewed By: strager
Differential Revision: D13742965
fbshipit-source-id: f407105e06b9954766bdb48ef1303e2003c07284
Summary:
To mitigate a deadlock, I want to make ProcessAccessLog not access /proc/. Allow this by making ProcessNameCache optional.
This diff should not change behavior.
Reviewed By: simpkins
Differential Revision: D13540948
fbshipit-source-id: 4c5d68c972c04122de1d2414084debfec078dd4c
Summary:
Eden no longer tracks any state in file handles, and has no plans to in the future.
Therefore, remove all related code.
Reviewed By: strager
Differential Revision: D13354307
fbshipit-source-id: 341d081f64c6c8fb2b4b1b5a5ff42f2cc7d38039
Summary:
Send readdir requests to TreeInode. This may not sound like a good
idea: the FUSE documentation suggests that stateful directory handles
are required to implement correct readdir semantics under concurrent
deletes and renames. However, the 63-bit offset value is treated as a
cookie that is passed from one readdir call into the next, and 63 bits
should be sufficient to implement readdir concurrent with
rename/unlink. So move readdir's implementation into TreeInode in
preparation for the complete removal of TreeInodeDirHandle.
Reviewed By: strager
Differential Revision: D13287664
fbshipit-source-id: c0d615675edd9b83353534468a69b89068bba923
Summary:
Send fsyncdir straight through the inode rather than going through
DirHandle. This is the better design anyway, since the DirHandle does
not receive directory-mutating requests like mkdir.
Reviewed By: strager
Differential Revision: D13287610
fbshipit-source-id: 154fa32a3877c89a204a2d10b4e2b637410d9486
Summary: Always send write requests straight to the inode rather than going through FileHandle.
Reviewed By: wez
Differential Revision: D10220619
fbshipit-source-id: 9ce328583cf0fa9d7d8850d92d9e15ddc382d6a3
Summary:
Always send read requests straight to the inode rather than going
through the FileHandle.
Reviewed By: wez
Differential Revision: D10220604
fbshipit-source-id: 6aa5d20f3ce09696a29bd5c1cb95d0b987ab213c
Summary:
Always send setattr and getattr straight to the inode rather than
going through the FileHandle.
Reviewed By: wez
Differential Revision: D10187876
fbshipit-source-id: 4c3aaa977cd568d5f9cc4b28583e164119c07c1b
Summary: Eliminate the need to look up an InodeNumber from a FileHandle. Instead, simply preserve the mapping when it's created.
Reviewed By: wez
Differential Revision: D10187120
fbshipit-source-id: dc47f7776294871ff2398f33c31bd85d240ead50
Summary: Begin tracking pids passed into FUSE in the ProcessAccessLog.
Reviewed By: strager
Differential Revision: D9595795
fbshipit-source-id: 02e5fefebcd0de860274409ba6258f14fa050b55
Summary:
Fixed a data race in Eden's `FuseChannel` implementation that could cause a crash if a `FUSE_INTERRUPT` request was concurrently on a different thread while a first thread was still launching the original request.
Modified `FakeFuse` to use `SOCK_SEQPACKET` instead of `SOCK_STREAM` so that tests can submit several requests in a pipelined fashion without having to first wait until the previous request was received. This requires the `recvResponse()` function to first read the header with `MSG_PEEK` to determine the response size and then subsequently read the entire message atomically to avoid reading a truncated message.
Added a new unit-test that exercises the `FUSE_INTERRUPT` race condition by sending a series of alternating `FUSE_LOOKUP`/`FUSE_INTERRUPT` requests.
Reviewed By: simpkins
Differential Revision: D9023654
fbshipit-source-id: 0eb44669ea8c4f58cf4313adf6ceb11098115a70
Summary:
Many of the tests in FuseChannelTest.cpp perform blocking waits on Futures.
Previously they used a 100ms timeout. This could sometimes result in flaky
test failures if the system was under heavy CPU load. Bumping up the timeout
to 1 second appears to avoid this issue.
Reviewed By: lewissbaker
Differential Revision: D9024730
fbshipit-source-id: 7af78c61008c6b4c1e5e130b3d37b2f3ac787a01
Summary:
This is part of "the great r-valuification of folly::Future":
* This is something we should do for safety in general.
* Context: `Future::get(...)` means both `Future::get()` and `Future::get(Duration)`
* Using lvalue-qualified `Future::get(...)` has caused some failures around D7840699 since lvalue-qualification hides that operation's move-out semantics - leads to some use of future operations that are really not correct, but are not obviously incorrect.
* Problems with `Future::get(...) &`: it moves-out the result but doesn't invalidate the Future - the Future remains (technically) valid even though it actually is partially moved-out. Callers can subsequently access that moved-out result via things like `future.get(...)`, `future.result()`, `future.value()`, etc. - these access an already-moved-out result which is/can be surprising.
* Reasons `Future::get(...) &&` is better: its semantics are more obvious and user-testable. It moves-out the Future, leaving it with `future.valid() == false`.
Reviewed By: LeeHowes
Differential Revision: D8756764
fbshipit-source-id: 5c75c79cebcec77658a3a4605087716e969a376d
Summary:
[Folly] In Futures Core, destroy context when destroying callback since they basically go together.
Also removes expectations in an Eden FS test case which affirm the old behavior, along with a TODO to remove the expectations once the affirmed behavior is fixed.
Reviewed By: marshallcline
Differential Revision: D8347040
fbshipit-source-id: ed9fec932ad1e0aa1e40675cf70081d19bbe4325
Summary:
This is not at all clear from cppreference.com, but per
https://www.youtube.com/watch?v=dTeKf5Oek2c, it sounds to me like
recommended practice is to either:
`using namespace std::chrono_literals` (or string_literals or
whatever) to pull in a focused set of literals.
or
`using namespace std::literals` to pull in all standard literals.
or
`using namespace std` to pull in everything.
`using namespace std::literals::chrono_literals` is unnecessarily
verbose.
Adopt those standards in Eden.
Reviewed By: simpkins
Differential Revision: D8060944
fbshipit-source-id: 4d9dd4329698b7ff5e5c81b5b28780ca4d81a2a1
Summary:
Update the folly::Init code to define a `--logging` command line flag, and call
`folly::initLoggingOrDie()` with the value of this command line during
initialization.
This is similar to the existing code that initializes the glog library.
(Programs can use both glog and folly logging together in the same program, and
I expect that many programs will do so as parts get converted to folly::logging
and parts remain using glog.)
Reviewed By: yfeldblum
Differential Revision: D7827344
fbshipit-source-id: 8aa239fbad43bc0b551cbe40cad7b92fa97fcdde
Summary:
Promote the folly logging code out of the experimental subdirectory.
We have been using this for several months in a few projects and are pretty
happy with it so far.
After moving it out of the experimental/ subdirectory I plan to update
folly::Init() to automatically support configuring it via a `--logging` command
line flag (similar to the initialization it already does today for glog).
Reviewed By: yfeldblum, chadaustin
Differential Revision: D7755455
fbshipit-source-id: 052db34c97f7516728f7cbb1a5ad959def2f6efb
Summary:
The changes in this diff comments out unused parameters. All changes are automated using clang-tidy.
This will allow us to enable `-Wunused-parameter` as error.
Reviewed By: simpkins
Differential Revision: D7371610
fbshipit-source-id: 0134e2f0b916313d690c073a46d747c52399a226
Summary:
Added a thrift call to return the outstanding FUSE requests.
Cli will call the thrift and print the output.
Added a unit test to test getOutstandingRequests().
Reviewed By: simpkins
Differential Revision: D7314584
fbshipit-source-id: 420790405babdb734f598e19719b487096ec53ca
Summary:
This test exercises code under eden/fs/fuse, so put it with that directory's
tests.
Reviewed By: chadaustin
Differential Revision: D7314457
fbshipit-source-id: b664aaa3086b2e65f7ae8b76ae71878f39fd9d17
Summary:
Previously the FuseChannel code unconditionally set `connInfo_.major` and
`connInfo_.minor` to the FUSE version that it supports, rather than the FUSE
version supported by the kernel it is talking to.
This does not appear to be the intended behavior, given that in several other
location it checks the value of `connInfo_->minor` to see if particular
features are supported.
Reviewed By: wez
Differential Revision: D7300845
fbshipit-source-id: 1f5946134037b2cbe03a8b86241acaa401c21ffa
Summary:
Remove the `FuseChannel::stealFuseDevice()` method, and instead change the
session completion future to return the FUSE device FD if it is still valid.
This ensures we only extract the FUSE device when it is still valid: if an
unmount event is being processed by a FUSE worker thread simultaneously with a
call to `FuseChannel::takeoverStop()` the session completion future will return
an empty `folly::File` since the FUSE device has been shut down.
This also helps clarify the synchronization semantics around modification to
the `fuseDevice_` member variable.
Reviewed By: wez
Differential Revision: D7300847
fbshipit-source-id: 02ced8fc9d24e7cd526d911782949d0bfbf0e5a7
Summary:
The FuseChannel destructor previously could be invoked while there are still
outstanding requests, which would cause problems when responses were generated
for these requests.
This makes the FuseChannel destructor private, requiring users to always
allocate FuseChannel objects on the heap and destroy them using a destroy()
method. The destroy() method still blocks waiting for the FUSE worker threads
to exit, but if there are still outstanding FUSE requests it defers the
deletion of the FuseChannel until they are complete.
The EdenMount class currently couldn't ever destroy FuseChannel objects with
outstanding requests, since it waits for the completion future to fire before
invoking the destructor. Fixing this behavior is mostly for correctness of the
FuseChannel API and for the benefit of our unit tests.
Reviewed By: chadaustin
Differential Revision: D7297829
fbshipit-source-id: 643d1332be84a1f25ee30ba2a2ea3515806f00ef
Summary:
Remove the `FuseChannel::getSessionCompleteFuture()` method and instead have
`initialize()` and `initializeFromTakeover()` return the `Future` that can be
used to wait on session completion.
This makes it explicit from an API perspective that this session completion
future will only be invoked if initialization is successful. It also
eliminates the possibility of anyone calling getSessionCompleteFuture() more
than once.
I also changed the session completion future to use a folly::SemiFuture rather
than a folly::Future, to make it explicit that callers generally should not
execute their callback inline in the same thread where the SemiFuture is
fulfilled.
Reviewed By: chadaustin
Differential Revision: D7297835
fbshipit-source-id: 3a1157951f0738f1692833ed5e875c3e9c6d6d69
Summary:
The FuseChannel unit tests would sometimes hang because a worker thread would
still be stuck in a blocking read() call.
The issue appears to be that the kernel would often automatically restart the
interrupted read(), so the worker would never wake up. This switches the code
from using SIGPIPE to wake up, and instead uses SIGUSR2, and installs a handler
for SIGUSR2 with the SA_RESTART flag unset.
It's possible that this was an issue only for the unit tests, where the FUSE
device is a socket. I'm not sure if the kernel would automatically restart
read operations on a real FUSE device.
There are still theoretically some race conditions here, since
`requestSessionExit()` could be called right after a worker thread checks
`(runState_` and before it enters the blocking `read()` call. Fixing that
would likely require switching to something like `folly::EventBase` or manually
using epoll. Fortunately I haven't seen that be an issue in practice so far.
Reviewed By: chadaustin, wez
Differential Revision: D7282002
fbshipit-source-id: 99d29de13a7858dd25f258fdc94d8f8c71ee84d9
Summary:
This changes FuseChannel to fulfill the session complete future immediately in
the thread where it finishes. This may occur inside a FUSE worker thread.
It is now up to the caller to use `via()` if they want their callback to run in
a separate thread.
This simplifies the FuseChannel code and also addresses a crash that occurs in
the unit tests sometimes: If the FuseChannel destructor is invoked without
explicitly invoking the FuseChannel, it would schedule the session complete
promise to be fulfilled in a separate EventBase thread. However, the promise
may then have already been destroyed by the time it can run in the EventBase
thread.
There are still additional synchronization issues to fix in the FuseChannel
tests, but this resolves one problem.
Reviewed By: chadaustin
Differential Revision: D7282001
fbshipit-source-id: be64d3ef6a0e664ed7a2cf93a549acc140184095
Summary:
Update FuseChannel::startWorkerThreads() to return without doing anything if
runState_ indicates that the FuseChannel is currently stopping.
This addresses a race betwen startWorkerThreads() and the destructor. If the
destructor is invoked in parallel with startWorkerThreads() the destructor can
grab the state_ lock and clear out the workerThreads list first. This then
causes problems when startWorkerThreads() runs and adds new threads to the
workerThreads list, which then never get joined.
Reviewed By: wez
Differential Revision: D7253574
fbshipit-source-id: f2cac11f1e71e1a14e3f020368152e0f2948c625
Summary:
Update FuseChannel::readInitPacket() to honor `runState_` and stop if it is
changed to indicate that the channel is stopping. Previously the init worker
thread would always wait until it received an INIT packet or an error on the
FUSE channel.
This also refactors the `readInitPacket()` code slightly mainly to to reduce
the level of indentation.
Reviewed By: wez
Differential Revision: D7253575
fbshipit-source-id: 7f27698947b629daecd9cd4ddffb6e6a921ce41e