Summary:
To support better telemetry and logging in watchman we want to use Eden's components. Lets migrate and detangle the needed pieces.
This change moves Throw.h and it's related tests from eden to edencommon.
Reviewed By: genevievehelsel
Differential Revision: D54046153
fbshipit-source-id: 669d702c13e70536d9c0b58ff8ff17f826237851
Summary:
`-Wextra-semi` or `-Wextra-semi-stmt` found an extra semi
If the code compiles, this is safe to land.
Reviewed By: MichaelCuevas
Differential Revision: D53776196
fbshipit-source-id: f83dda6df32622f1ced24d051eadeaa8ebfda4e8
Summary: In preparation for merging ProcessSimpleName and ProcessName functionality into ProcessNameCache renaming now The new cache will cache ProcessInfo object which will include both names, and the parent pid.
Reviewed By: genevievehelsel
Differential Revision: D48855222
fbshipit-source-id: 4cb10df7b6cc32efc0d655771d01d58e7ba57dd5
Summary:
Server.h is such a generic name to call up in myles, and now the class
name matches the header name.
While I was touching these files, I broke a couple #include dependencies.
Reviewed By: xavierd
Differential Revision: D47681923
fbshipit-source-id: 46c1b8ce130ba63c7bd6e44db31d0719f57e81bf
Summary:
Aiming towards moving unmount() into FsChannel, move the knowledge of
how to unmount an NFS mount into Nfsd3.
To support unmounting on Windows, we can eventually add an invocation
of unmount.exe to the Windows PrivHelper implementation.
Reviewed By: kmancini
Differential Revision: D45296963
fbshipit-source-id: 55fa7fe0f6190d3708caa21a0cd4b3868f464f8b
Summary:
Provided implementation for EdenMount::chown on NFS (MacOS). Utilized exisiting code to get all referencered Inodes and then loaded and invaliated them.
Ran into some issues compiling Ranges - something to learn more about - where certain iterator traits needed to be defined. In subsquent diff should be able to address (if desired), but for this diff just used a vector instead.
Reviewed By: MichaelCuevas
Differential Revision: D46946414
fbshipit-source-id: b405f16712d6d6315d43d6973e7615aced7e5c7d
Summary:
After a takeover, the new socket was initialized with a "remote
EventBase", causing stopAccepting to be asynchronous, tripping an
assertion.
There was a latent bug here: it might have been possible for a new
connection to be accepted by the old process after takeover begun.
Reviewed By: xavierd
Differential Revision: D45584154
fbshipit-source-id: cdd8118d82100bc1e19f6f19676cc2fa412b8775
Summary:
FuseChannel already had an asynchronous initialize(). Lift that to
FsChannel and unify initialization of Prjfs and NFS.
Reviewed By: kmancini
Differential Revision: D45106889
fbshipit-source-id: 632ddfa275c732fc30efc1984bda43f61c37fd5e
Summary:
EdenMount shouldn't know about the vagaries of FsChannel teardown. In
fact, FuseChannel already has its own requirements, hidden behind a
unique_ptr deleter. Use the same mechanism for NFS, allowing us to
remove a .via() special-case.
Reviewed By: kmancini
Differential Revision: D44810223
fbshipit-source-id: db2f88bc3843b3842de60c5a993dfd9890dc3c15
Summary:
After my previous refactoring, the memory safety bug in Nfsd3 no
longer manifests, and we can remove the unnecessary defer call.
Reviewed By: kmancini
Differential Revision: D44988749
fbshipit-source-id: 2ebd5fbf96d05ce6850fc248a642409f827ffeec
Summary:
I'm carefully reading the takeover code and noticed a few things to
improve.
Reviewed By: kmancini
Differential Revision: D44946740
fbshipit-source-id: 558441a47425b15c19ca2a6816ae2e8bf3d68019
Summary:
I'm trying to track down a subtle lifetime rule violation in
EdenMount. While reading RpcServer, I noticed a few possible
simplifications.
Reviewed By: xavierd
Differential Revision: D44849773
fbshipit-source-id: 4c27c47a7e2c211dcd040acce955e9b2f617b55b
Summary:
By moving NFS's StopData behind FsStopData, the std::variant and
runtime checks during takeover can be unified into one path. The next
diff will do the same for Windows.
Reviewed By: kmancini
Differential Revision: D44737160
fbshipit-source-id: 19ca623f90b367d25385f177cafa44d7bf7ebb62
Summary:
Now that we have the plumbing we can start registering our NFS RPC servers (
mountd and nfsd) with the portmapper/rpcbind.
I am taing a short cut for prototyping. I am skipping general purpose
rpc registration (i.e. making these rpc calls to register a service). This is
because (1) I am trying to prototype quickly, so I don't want to implement the
portmapper endpoints that allow registering/ unregistering a server and (2)
there are maybe security considerations, and I don't want to think about those
rn.
Reviewed By: chadaustin
Differential Revision: D44987884
fbshipit-source-id: 4b849d1dd060d2e033ab48df883e228015906a7d
Summary:
A common FsChannel base class allows removing some ifdefs from
EdenServiceHandler. With some effort, we should be able to find the
correct abstractions that allow us to entirely decouple particular
FsChannel implementations from the rest of the inode layer.
Reviewed By: kmancini
Differential Revision: D44657395
fbshipit-source-id: 9702186f1d0c9ce76a11e766010f5899e2b517af
Summary:
I'm getting nfs to build on windows to prototype it and see how feasible it
might be as an option on Windows. PrjFS has a very different model than EdenFS,
and that has made EdenFS correctness on Windows very difficult. NFS may be
easier to get correct, though the performance is suspect. Just exploring
options here.
Reviewed By: xavierd
Differential Revision: D44153042
fbshipit-source-id: 012adea3c89e9c4b37c08cff685e3937d2c6678b
Summary:
DurationScope has a pair of atomic reference count operations at the
beginning and end of every recorded duration. To avoid that overhead,
reference EdenStats with RefPtr and, in production EdenFS, store a
global, unowned EdenStats object.
This gives us the benefits of explicit dependencies and no global
state in tests, while avoiding atomic RMWs in the release build.
Reviewed By: xavierd
Differential Revision: D44323723
fbshipit-source-id: 1b3384d2e6a0a2959fd79774a8ba46afc4c176ca
Summary:
# Problem
In macOS Ventura, copying a file into an EdenFS NFS mount in Finder only creates an empty file, producing the error message:
> The operation can’t be completed because an unexpected error occurred (error code 100072)
This error code indicates a bad RPC struct, and it happens because we're sending a malformed response to some SETATTR requests.
# Analysis
In D40798840 (34884008f5) we added code to ignore two types of setattr requests:
- On NFS, requests that are no-op aside from updating an `atime`.
- Everywhere, requests that are entirely no-op.
Contrary to the description of D40798840 (34884008f5), it's the former type of request that can be created by `mmap`-ing a file over NFS. The latter appears to be generated at various points by macOS, including when copying a file into an NFS share with Finder.
We currently return an empty response body to that second type of request, without even a response status. As of macOS Ventura this produces an error message.
# Fix
Here we modify the `is_nop` check at the NFS level so that SETATTRs that only update `atime` become fully no-op, falling through to the FileInode check (which doesn't ignore `atime`) which then considers the request no-op. This returns up through the NFS stack to produce a well-formed NFS response.
This leaves in place the `atime`-sensitive behavior at the FileInode level.
This does not affect the invalidation behavior introduced in D35435764 (8f3f873874), which sets `mode`.
Reviewed By: xavierd
Differential Revision: D42785781
fbshipit-source-id: af174abd61b4927040f481d0a177443eb64ae8da
Summary:
This re=implements the backed-out D41393677 (6384a0f39a) with some changes:
- Only log if the size metadata is _less than_ the size of the file contents
we're returning. This way we avoid erroneously logging partial reads, as
happened in T139036919.
- Only log once per Nfsd3ServerProcessor instance.
Reviewed By: chadaustin
Differential Revision: D41751040
fbshipit-source-id: 2503ecb488b0600bdeabbb11e532e4590edbd280
Summary:
Log to text logs and Scuba if there's an NFS READ where the size of the file
contents didn't match the size in metadata. This way we can see whether size
mismatches are still happening much in practice, in order to know how to
prioritize further investigation.
Based on kmancini's D41239395, but adds Scuba logging too.
Reviewed By: chadaustin, kmancini
Differential Revision: D41393677
fbshipit-source-id: 968c2866d8fd353e89ad871080a57308cacdc102
Summary:
During NFS reads we call getattr and lookupInode to get post-operation
attributes, but we currently silently return empty attributes if lookupInode
fails but the read otherwise succeeds. This adds logging so we can check if
this is happening when we reproduce NFS returning truncated file contensts.
Reviewed By: xavierd
Differential Revision: D41364371
fbshipit-source-id: 8f85ae6ed767ad880ec5cbc863b2d573fdc341f2
Summary:
Requiring that callers ensure that ObjectFetchContext outlives the
asynchronous result is a burden and a memory-safety risk. Without
Rust's borrow checker, we can't feel confident this is safe under all
error and timeout conditions. It also requires we use
folly::collectAll and collectAllSafe, which means that an error during
fanout has to wait until the fanout completes before we can report the
error to our caller.
Moreover, tying the ObjectFetchContext's lifetime to the
FUSE/NFS/Thrift/etc. request's lifetime gives us incorrect statistics
about how long the request takes. I consider this an API design
mistake (on my part).
Correct the situation by decoupling ObjectFetchContext from
RequestContext, and begin reference-counting ObjectFetchContext.
This does increase the number of atomic RMW operations, but we can
alleviate those where they matter by manually checking
*Future::isReady() before calling .then*.
Reviewed By: xavierd
Differential Revision: D40744706
fbshipit-source-id: 5baf654f6d65790ed0cafb45c0bc7f2aaa8ecec2
Summary:
# Problem
On ARM64, macOS will send nop/empty `setattr` requests. If we don't filter them, we end up incorrectly changing the `ctime` of the file and thus generating spurious/invalid change notifications.
This ends up slowing down buck2 builds as as part of each build, we end generating lots of spurious file change events and it invalidates buck2 daemon state incorrectly (as files haven't changed)
# Investigation
I set a breakpoint in `Nfsd3ServerProcessor` to figure out what was happening. As suspected, it was a completely empty/nop request P542900494
```
(facebook::eden::DesiredMetadata) desired = {
size = Has Value=false {}
mode = Has Value=false {}
uid = Has Value=false {}
gid = Has Value=false {}
atime = Has Value=false {}
mtime = Has Value=false {}
}
```
# Solution
Filter out empty `setattr` requests. As per discussion with chadaustin, we do not need to care about `atime` changes, so those are dropped.
Reviewed By: chadaustin
Differential Revision: D40798840
fbshipit-source-id: ef463d581bae89f905aa8ada3f6452e6eeb744d2
Summary:
Separately allocate ObjectFetchContext in RequestContext so that its
lifetime can extend longer than the request itself.
Reviewed By: kmancini
Differential Revision: D40349610
fbshipit-source-id: abc1785f194abb06383a2e72cb2ca71c266ae42b
Summary:
Remove an unnecessary context capture in NFS request handling. This is
a small simplification in preparation for changes to how
ObjectFetchContext is managed.
Reviewed By: genevievehelsel
Differential Revision: D40349525
fbshipit-source-id: 0b013d5485302b46959f6535692e116cb0aeea72
Summary:
Now that finishRequest is moved into the destructor, we no longer need
a custom shared_ptr constructor function.
Reviewed By: genevievehelsel
Differential Revision: D40349291
fbshipit-source-id: 1ac8c745cab4a09c7c5cf899803e5683d2aeedf7
Summary:
In advance of some trickier changes, make a few small improvements to
ObjectFetchContext.
Reviewed By: genevievehelsel
Differential Revision: D39901208
fbshipit-source-id: ed50141558201dd2666d55a21bf63db5bf2ffd43
Summary:
The main non-mechanical conversion from folly/Conv to fmt is replacing
folly::to<std:string>'s variadic concatenation operation with
fmt::join. Now that that's solved, we can slowly migrate the rest.
Reviewed By: genevievehelsel
Differential Revision: D39992102
fbshipit-source-id: 81df8e805c78ea5ca80ad5977f20b19e5a7467a3
Summary:
This allows us to remove ifdefs and begin enforcing consistent
prefixes in StatsGroup.
Reviewed By: genevievehelsel
Differential Revision: D39782327
fbshipit-source-id: c453e0ada5d5d9d3b221c707f67dce7d1e0a970d
Summary:
The public EdenStats API was both too chatty and exposed too many
implementation details. Don't mention thread-locals in the public API,
and replace uses of `addValue` with explicit `addDuration` on Duration
and `increment` on Counter.
Reviewed By: genevievehelsel
Differential Revision: D39661497
fbshipit-source-id: 8f6f2a9d72a4c385cde32e72fba0cebf8f5b4274
Summary:
Rename "channel" to "fs channel", rename StatPtr to DurationStatPtr,
and generalize DurationStatPtr to every stat category.
Reviewed By: xavierd
Differential Revision: D39631597
fbshipit-source-id: 147f9a9d9fe97a39c0d38ccecf57e73c93af0d8c
Summary:
LockedPtrs used as temporaries within range expressions may go out of scope
before the end of the loop.
https://en.cppreference.com/w/cpp/language/range-for:
> If range-expression returns a temporary, its lifetime is extended until the
> end of the loop, as indicated by binding to the forwarding reference __range,
> but beware that the lifetime of any temporary within range-expression is not
> extended.
Reviewed By: xavierd
Differential Revision: D37904242
fbshipit-source-id: 915812e7bb4d19c333dc4c309261610884f8e61d
Summary:
GCC provides some useful warnings and errors when compiling EdenFS,
and I noticed our open source build was failing. Fix some of the
issues.
Reviewed By: xavierd
Differential Revision: D37157074
fbshipit-source-id: de5705ec2eac18cd269143739289031ca5d478fc
Summary:
We should all be migrating to platform010, for the improved performance of its
generated code and for its improved diagnostics/portability.
Reviewed By: kmancini
Differential Revision: D35851539
fbshipit-source-id: d42b12c77ddeacb4777f2e360fdebd1cbbc1bd6d
Summary:
We should all be migrating to platform010, for the improved performance of its
generated code and for its improved diagnostics/portability.
Reviewed By: xavierd
Differential Revision: D35573251
fbshipit-source-id: 5339212de0e617df13277ffcb5a67a535263847d
Summary:
When we try to switch to platform 10 we get this compile error:
```
eden/fs/nfs/Nfsd3.cpp:1378:26:
error: call to consteval function
'fmt::basic_format_string<char, const unsigned int &>::basic_format_string<const char *, 0>'
is not a constant expression
return fmt::format(fmtString, std::get<typename T::TrueVariant>(val.v));
```
The problem is that fmtString either needs to be a `basic_format_string` or
needs to be a constexpr so that we can cast it to `basic_format_string` through
various means.
Not totally sure why this worked pre c++ 20, but C++20 introduced some stricter
type checking on the fmt library or constexpr stuff in general i guess.
https://github.com/fmtlib/fmt/issues/2438 outlines the possible fixes
`fmt::runtime` looks like the best way.
Reviewed By: vitaut
Differential Revision: D35756658
fbshipit-source-id: 7b65ccf0719a964ab1f2fc1aed72744e72374d20
Summary:
TL;DR: File invalidation after checkout is broken on NFS macOS. This proposes a
fix.
Previously, to invalidate things on NFS we opened and closed all the parent
directories of any files/directories that changed during a checkout operation.
This worked on Linux because all open calls result in some request into the
EdenFS daemon (usually a getattr I think). The returned response from this
would show that the directory had update timestamps. So the open would see the
parent directories in their updated state. This would trigger the NFS client to
clear it's caches for that directory and their recursive children to preserve
CTO. CTO or close-to-open consistency guarantees that if on process observed a
file in state A and closed the file, then another process opened that same file
, it will see the file in state A or a later state.
macOS does not seem to do CTO.
It seems that most of the "read" filesystem calls can basically be served from
cache on NFS. Only writes seem to be guaranteed to make it into eden.
So instead of using a "read" filesystem call to trigger invalidation, we need to
use a write one.
I tried opening things in write mode, but the problem is that we need to
invalidate directories (to ensure the entry information is updated) and
directories can not be opened in write mode.
I tried writing 0 bytes to the files themselves that have changed, but this
empty write is short circuited somewhere in the kernel.
The only filesystem call that can be a noop, and seems to trigger a call into
eden is chmod. We are not really working off any guarantees any more, but
it seems to work on both Linux and macOS in my testing so far and is better
than our current broken state of invalidation.
So now to invalidate we chmod parent directories that have changed with their
current mode. Note that this could get extra tricky if we were mixing updating
directory permissions with invalidating them. We would need to ensure we chmod
with the most up to date mode bits. However, because we do not update
permissions anywhere that we also invalidate (checkout does not know how to
update directory permissions) things are a little simpler.
It's important that the chmod can not complete until it seems an updated view of
the parent directory. So we have to be careful about locking here. (Although
that hasn't really changed since we switched from open to chmod.)
One thing that does change is that since chmod is technically a write, it could
cause extra materialization that would be bad for checkout and status
performance. To prevent these problems I have updated setattr to not materialize
a directory when the attribute change is a noop which it should be in our
invalidation technique unless another client modified the directory in the
meantime in which case the directory should be modified anyways. This would
mean that we could end up wiping out clients changes to permissions in the
working copy during checkout. but this matches non eden checkout behavior. So I
think this is ok.
Reviewed By: xavierd
Differential Revision: D35435764
fbshipit-source-id: 196c4995b130b595f9582204237e726e5212993f
Summary: This code has enough risk of a copy paste error that it deserves a unit test.
Reviewed By: chadaustin
Differential Revision: D35161787
fbshipit-source-id: 5691d13a74a0f059dfd6a93ea2852dca8399a165
Summary:
We have seen that sometimes the a client sends us nfsv4 requests with the
nfsv3 version on it.
We fail to parse those.
It's likely the kernel that is sending us that event, but technically any client
could connect to us, so we don't know that for sure.
Let's make sure we log if we see more than one client connected to us. so that
we can confirm if this is the kernel or not.
We should probably also not let non kernel clients connect to us. But I have not
figured out how to do that. tbd
Reviewed By: fanzeyi
Differential Revision: D35448531
fbshipit-source-id: e0810c8961c18b305b80bb874ae4f6aee9583d07
Summary:
In the NFS protocol, the client does not completely resolve permissions based
on the access bits in the mode field of stat.
Clients make a special call into NFS, ACCESS, to check if a certain user has
permissions to perform some operation on a file or directory.
Access was perviously unimplemented for NFS and returned open permissions.
This fully implements the access procedure.
Reviewed By: xavierd
Differential Revision: D34632815
fbshipit-source-id: eda39e02c12c189cd1fc3a32dbe864f00d2c3458
Summary:
If an NFS client sends us an improperly formatted request, EdenFS crashes.
This is dangerous because any process on the machine could send us an NFS
request and this could crash eden.
Eden should be resilient to badly formatted requests/requests it fails to
parse.
This will make debugging failed requests harder as errors will be less
obvious to the user, but it will also allow users to keep using their mount
after a failed request.
Reviewed By: xavierd
Differential Revision: D34981438
fbshipit-source-id: 107de2324b1dc145bd426398614ee76b72c5c446
Summary:
Refactor the existing notification class so it uses the new Notifier interface. The notifier interface will be used to set up notifications/notification trays on different platforms.
The notifier interface is not fully completed yet. More methods will be added in a future diff.
As of now, we only have 1 Notifier, the command notifier. I hope to remove this notifier once we implement notifiers for all platforms that we support.
Reviewed By: chadaustin
Differential Revision: D34291809
fbshipit-source-id: a2a67af2683f1f88781428e8d88191f49e100e96
Summary:
When ran in the destructor, the class inheriting from RequestContext would have
been destroyed, and thus the vtable for the RequestContext would no longer from
the child class. In practice that meant that the PID of all requests was always
a nullopt, making `eden minitop` and others completely useless.
To solve this, we can simply create a shared_ptr with a custom deleter that
calls finishRequest first before calling the destructor.
Reviewed By: genevievehelsel
Differential Revision: D34160296
fbshipit-source-id: 7f6d9bfa36bc3db256cfe0f61478c22a44028001
Summary:
In the case where the shared_ptr holding a RequestContext is copied, some stats
may still be updated after the request completes. Thus it's best to call
finishRequest in the destructor to make sure the stats are properly updated.
Reviewed By: chadaustin
Differential Revision: D33724775
fbshipit-source-id: 5e4b247d44bdbeb322f25316dc98798e551cc43b
Summary:
With Facebook having been renamed Meta Platforms, we need to change the license
headers.
Reviewed By: fanzeyi
Differential Revision: D33407812
fbshipit-source-id: b11bfbbf13a48873f0cea75f212cc7b07a68fb2e
