Commit Graph

182 Commits

Author SHA1 Message Date
Katie Mancini
d15b6ccd81 get nfs_utils building on Windows
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.

NOTE: this one is more than removing ifdefs, please review carefully

Reviewed By: xavierd

Differential Revision: D44152041

fbshipit-source-id: 21affcc417dce492c2846902225686334eb2dc30
2023-03-29 12:52:44 -07:00
Katie Mancini
6ccae0c2f5 Get nfs_rpc building on Windows
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: D44151904

fbshipit-source-id: c14fa313b2817c3d55f616f4252a0611965ead78
2023-03-29 12:52:44 -07:00
Katie Mancini
bdeeebcd24 get rpc building on Widnows
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: D44151833

fbshipit-source-id: 5d39f9d3de527cd21506b8473555f9dbd2a587bb
2023-03-29 12:52:44 -07:00
Katie Mancini
444a19e5e9 get xdr building on Windows
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: D44151636

fbshipit-source-id: 0f13b997064d96995f48b658ccf0ecac6477dad8
2023-03-29 12:52:44 -07:00
Chad Austin
e419dd2799 remove the reference-counting overhead from DurationScope
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
2023-03-24 13:50:40 -07:00
Xavier Deguillard
317e55ca91 inodes: use EdenStats with shared_ptr, not raw pointers
Summary:
This makes lifetime easier to reason about, and allows easier use of
DurationScope (see next diff).

Reviewed By: chadaustin

Differential Revision: D44311712

fbshipit-source-id: 1f9f4cbcc59bafeb464e2b39a248958cedaf9ca6
2023-03-22 20:04:41 -07:00
Xavier Deguillard
553b4278a0 main: move folly::init default configuration to main
Reviewed By: fanzeyi

Differential Revision: D44263797

fbshipit-source-id: 61a219e98b96f61495eb549c0e01ca635ab29e9f
2023-03-21 14:17:32 -07:00
Mark Shroyer
58fa5c3541 Fix 100072 errors when copying file into EdenFS in Finder
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
2023-01-27 10:47:02 -08:00
Mark Shroyer
a9322aeae7 Restore metadata size mismatch logging
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
2022-12-14 03:18:23 -08:00
Mark Shroyer
83bb997ee9 Back out "Log metadata size mismatch on NFS READ"
Summary:
Original commit changeset: 968c2866d8fd

Original Phabricator Diff: D41393677 (6384a0f39a)

Reviewed By: xavierd

Differential Revision: D41624043

fbshipit-source-id: 051981d201d28905d97385012eecf58e75ce8634
2022-11-30 14:56:35 -08:00
Mark Shroyer
6384a0f39a Log metadata size mismatch on NFS READ
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
2022-11-28 14:42:03 -08:00
Xavier Deguillard
b7d87d807a utils: deny building an AbsolutePath from a literal
Summary:
We've had a couple of failures after landing D40818724 (f199e93924) due to some places still
building AbsolutePath from a literal. On Windows, this is almost always a bug
due to paths usually not being UNC.

To avoid this issue, we can simply forbid an AbsolutePath from being built in
the first place from a path, instead forcing the use of canonicalPath. This was
suggeted by mshroyer in the orignal diff.

Reviewed By: chadaustin

Differential Revision: D41229194

fbshipit-source-id: 3451bdcba276c87f98326b025e05f6a4eddbd1b7
2022-11-22 16:20:30 -08:00
Mark Shroyer
615748c257 Log if there's an error calling getattr during NFS read
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
2022-11-17 14:20:46 -08:00
Chad Austin
6f60c48ea8 decouple ObjectFetchContext lifetime from RequestContext
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
2022-11-15 13:35:45 -08:00
Di Xiao
5777b33e4d Make NFS Tracebus size configurable by EdenConfig
Summary: Adding an ConfigSetting "telemetry:nfs-tracebus-capacity" to allow dynamically readjusting NFS Tracebus buffer capacity from outside EdenFS.

Reviewed By: genevievehelsel

Differential Revision: D40917315

fbshipit-source-id: 159b3b2747702d812cf1891b09b1905c2c177508
2022-11-02 21:02:48 -07:00
Milen Dzhumerov
34884008f5 Fix spurious notifications on ARM64
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
2022-10-28 16:38:40 -07:00
Chad Austin
53b9dc5d3f Back out "decouple ObjectFetchContext lifetime from RequestContext"
Summary:
Original commit changeset: abc1785f194a

Original Phabricator Diff: D40349610 (1eed4f0063)

Reviewed By: xavierd

Differential Revision: D40731896

fbshipit-source-id: 632c08fd3e44e8d2a126fdf688f41803354641a4
2022-10-26 15:07:38 -07:00
Chad Austin
1eed4f0063 decouple ObjectFetchContext lifetime from RequestContext
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
2022-10-25 20:16:34 -07:00
Chad Austin
7ab1054530 simplify Nfsd3ServerProcessor request handling
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
2022-10-25 20:16:34 -07:00
Chad Austin
4f6b77af28 remove RequestContext::makeSharedRequestContext
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
2022-10-25 20:16:34 -07:00
Chad Austin
e120426cea minor ObjectFetchContext refactoring
Summary:
In advance of some trickier changes, make a few small improvements to
ObjectFetchContext.

Reviewed By: genevievehelsel

Differential Revision: D39901208

fbshipit-source-id: ed50141558201dd2666d55a21bf63db5bf2ffd43
2022-10-12 11:43:39 -07:00
Chad Austin
08086fafc0 migrate utils/Throw.h to fmt
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
2022-10-04 21:42:44 -07:00
Chad Austin
6857ef5676 split FsChannelStats into FuseStats, NfsStats, PrjfsStats
Summary:
This allows us to remove ifdefs and begin enforcing consistent
prefixes in StatsGroup.

Reviewed By: genevievehelsel

Differential Revision: D39782327

fbshipit-source-id: c453e0ada5d5d9d3b221c707f67dce7d1e0a970d
2022-09-30 16:48:23 -07:00
Chad Austin
50b9eafa6b remove the thread-local implementation detail from the public EdenStats API
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
2022-09-30 11:35:05 -07:00
Mark Shroyer
4ca1838e1c Fix incorrect EOF value in TreeInode::readdirImpl
Summary:
If we run out of NFS buffer space while trying to add the final directory entry
in TreeInode::readdirImpl we would incorrectly return true, indicating that
we've returned the entire listing.

This means that on NFS we would incorrectly send an EOF to the client, which
would then not send a subsequent, offset readdir request.  Because of this, for
certain NFS-mounted Eden directory contents, calling `ls` would result in a
single file being missing from the listing.

Reviewed By: chadaustin

Differential Revision: D39911610

fbshipit-source-id: 96a8fc7958a8e800ff7f2489c968f2dde23a86da
2022-09-30 11:32:31 -07:00
Chad Austin
142166abaf light EdenStats refactoring
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
2022-09-23 17:45:28 -07:00
Katie Mancini
b44082e107 remove folly::Format InodeNumber
Summary:
From the folly format docs
> Use fmt::format instead of folly::format for better performance, build times and compatibility with std::format

eden build times have gotten a bit high, cutting out folly format will help reduce build time, so let's start
by banishing it from eden

buck2 build times have been un-reliable, I have been trying `--local-only` and `buck clean` but the build times seem too goo for clean builds and are all over the place. Will try more throughly clearing caches. I think that folly/format.h is still being linked  due to recursive dependencies, so I am looking into making sure it at least isn't being recursively included in any of our headers.

Reviewed By: xavierd

Differential Revision: D38250995

fbshipit-source-id: b02d8c458fe92a068426031cffd20ef01ae0edf7
2022-08-02 11:44:19 -07:00
Chad Austin
265128f4f4 remove some conditional includes
Summary:
To simplify the build and fall in line with Folly's conventions,
remove several conditional includes. Instead, headers conditionally
define symbols.

Reviewed By: xavierd

Differential Revision: D37897850

fbshipit-source-id: 8b52d5310f5cd8cb17fdc013117c271ae09dd5d6
2022-07-19 12:37:57 -07:00
Mark Shroyer
b02c805973 Fix incorrect lock use in range-based for loops
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
2022-07-18 10:22:18 -07:00
Chad Austin
b12c397618 compilation fixes for gcc
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
2022-06-23 15:45:36 -07:00
Chad Austin
9fa292b9ed standardize namespaces on C++17 syntax
Reviewed By: genevievehelsel

Differential Revision: D36429182

fbshipit-source-id: 7d355917abf463493c37139856810de13e1090ff
2022-05-17 10:12:56 -07:00
Katie Mancini
d1953ac1e3 fix IOBuf manipulations
Summary:
We trim off the fragment header from the NFS requests we get. Sometimes during
the test_takeover_with_io tests this was crashing in this code with:

```
F0418 20:55:24.216660 25733 IOBuf.h:765] Check failed: amount <= length_ (4 vs. 1)
*** Check failure stack trace: ***
*** Aborted at 1650340524 (Unix time, try 'date -d 1650340524') ***
*** Signal 6 (SIGABRT) (0x50aa) received by PID 20650 (pthread TID 0x7f5f83aa9700) (linux TID 25733) (maybe from PID 20650, UID 0) (code: -6), stack trace: ***
    @ 00000000000192fd folly::symbolizer::(anonymous namespace)::innerSignalHandler(int, siginfo_t*, void*)
                       ./fbcode/folly/experimental/symbolizer/SignalHandler.cpp:449
    @ 00000000000102ab folly::symbolizer::(anonymous namespace)::signalHandler(int, siginfo_t*, void*)
                       ./fbcode/folly/experimental/symbolizer/SignalHandler.cpp:470
    @ 0000000000000000 (unknown)
    @ 000000000003e530 __GI_raise
    @ 000000000002551c __GI_abort
    @ 000000000000dbac google::LogMessage::Fail()
                       /home/engshare/third-party2/glog/0.3.2_fb/src/glog-0.3.2/src/logging.cc:1519
    @ 00000000000109f2 google::LogMessage::SendToLog()
                       /home/engshare/third-party2/glog/0.3.2_fb/src/glog-0.3.2/src/logging.cc:1473
    @ 000000000000d7f2 google::LogMessage::Flush()
                       /home/engshare/third-party2/glog/0.3.2_fb/src/glog-0.3.2/src/logging.cc:1346
    @ 000000000000eaa8 google::LogMessageFatal::~LogMessageFatal()
                       /home/engshare/third-party2/glog/0.3.2_fb/src/glog-0.3.2/src/logging.cc:2013
    @ 000000000009dead folly::IOBuf::trimStart(unsigned long)
                       ./buck-out/v2/gen/fbcode/8107dfcc75db0fad/folly/io/__iobuf__/headers/folly/io/IOBuf.h:765
                       -> ./fbcode/eden/fs/nfs/rpc/Server.cpp
    @ 000000000009ce3f facebook::eden::RpcTcpHandler::tryConsumeReadBuffer()::$_0::operator()()
                       ./fbcode/eden/fs/nfs/rpc/Server.cpp:243
    @ 000000000009bedc void folly::detail::function::FunctionTraits<void ()>::callSmall<facebook::eden::RpcTcpHandler::tryConsumeReadBuffer()::$_0>(folly::detail::function::Data&)
                       ./buck-out/v2/gen/fbcode/8107dfcc75db0fad/folly/__function__/headers/folly/Function.h:364
                       -> ./fbcode/eden/fs/nfs
...
...
...
y/__named_thread_factory__/headers/folly/executors/thread_factory/NamedThreadFactory.h:40
                       -> ./fbcode/configerator/distribution/api/ConfigeratorStaticData.cpp
    @ 000000000033b32c void std::__invoke_impl<void, folly::NamedThreadFactory::newThread(folly::Function<void ()>&&)::{lambda()#1}>(std::__invoke_other, folly::NamedThreadFactory::newThread(folly::Function<void ()>&&)::{lambda()#1}&&)
                       ./fbcode/third-party-buck/platform009/build/libgcc/include/c++/trunk/bits/invoke.h:60
                       -> ./fbcode/configerator/distribution/api/ConfigeratorStaticData.cpp
    @ 000000000033b2bc std::__invoke_result<folly::NamedThreadFactory::newThread(folly::Function<void ()>&&)::{lambda()#1}>::type std::__invoke<folly::NamedThreadFactory::newThread(folly::Function<void ()>&&)::{lambda()#1}>(std::__invoke_result&&, (folly::NamedThreadFactory::newThread(folly::Function<void ()>&&)::{lambda()#1}&&)...)
                       ./fbcode/third-party-buck/platform009/build/libgcc/include/c++/trunk/bits/invoke.h:95
                       -> ./fbcode/configerator/distribution/api/ConfigeratorStaticData.cpp
    @ 000000000033b294 void std:🧵:_Invoker<std::tuple<folly::NamedThreadFactory::newThread(folly::Function<void ()>&&)::{lambda()#1}> >::_M_invoke<0ul>(std::_Index_tuple<0ul>)
                       ./fbcode/third-party-buck/platform009/build/libgcc/include/c++/trunk/thread:244
                       -> ./fbcode/configerator/distribution/api/ConfigeratorStaticData.cpp
    @ 000000000033b264 std:🧵:_Invoker<std::tuple<folly::NamedThreadFactory::newThread(folly::Function<void ()>&&)::{lambda()#1}> >::operator()()
                       ./fbcode/third-party-buck/platform009/build/libgcc/include/c++/trunk/thread:251
                       -> ./fbcode/configerator/distribution/api/ConfigeratorStaticData.cpp
    @ 000000000033b0ed std:🧵:_State_impl<std:🧵:_Invoker<std::tuple<folly::NamedThreadFactory::newThread(folly::Function<void ()>&&)::{lambda()#1}> > >::_M_run()
                       ./fbcode/third-party-buck/platform009/build/libgcc/include/c++/trunk/thread:195
                       -> ./fbcode/configerator/distribution/api/ConfigeratorStaticData.cpp
    @ 00000000000d9660 execute_native_thread_routine
    @ 000000000000920b start_thread
    @ 000000000011816e __GI___clone
```

The reason we are crashing is that the first IOBuf in the chain does not
necessarily contain the full fragment header. So we might attempt to trim off
more data than is in the IOBuf. We can fix this my trimming from the IOBuf chain
instead.

TBH idk why only this test triggers the problem, it seems like eden should
crash more often than that. But it should be fixed now anyways.

Reviewed By: xavierd

Differential Revision: D35863737

fbshipit-source-id: 21640b252a703fe4fa52f66111e6ae50a94bc347
2022-05-06 22:31:51 -07:00
Michael Cuevas
b2ae3f9cc3 eden/{integration, fs, facebook}: switch to platform010
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
2022-04-26 13:22:33 -07:00
Michael Cuevas
91b6afcfd1 Back out "eden/{integration, fs, facebook}: switch to platform010"
Reviewed By: chadaustin

Differential Revision: D35850816

fbshipit-source-id: 282c06fbf7cc249030ae0173779722175c662133
2022-04-22 14:52:01 -07:00
Michael Cuevas
8fb9ea454a eden/{integration, fs, facebook}: switch to platform010
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
2022-04-21 15:37:37 -07:00
Katie Mancini
9463488e8b fix C++ 20 compile error
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
2022-04-20 17:37:54 -07:00
Katie Mancini
8f3f873874 fix invalidation on NFS
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
2022-04-19 20:32:54 -07:00
Katie Mancini
ad04eb9bc1 add unit test for access
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
2022-04-18 21:55:15 -07:00
Katie Mancini
3893aa9cf6 log extra connections to NFS server
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
2022-04-07 15:34:08 -07:00
Katie Mancini
63ad96b091 Support access calls
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
2022-03-29 10:21:05 -07:00
Katie Mancini
7ba3360a97 really weird std::optional + delayed destructor bug
Summary:
my eden-dev on my M1 is always crashing on `eden stop` with this trace:

```
Assertion failed: (dd_->guardCount_ > 0), function ~DestructorGuard, file DelayedDestructionBase.h, line 96.
*** Aborted at 1648171587 (Unix time, try 'date -d 1648171587') ***
*** Signal 6 (SIGABRT) (0x1bb2fd9b8) received by PID 31226 (pthread TID 0x115a58580) (maybe from PID 31226, UID 501) (code: 0), stack trace: ***
I0324 18:26:27.215606 6572327 EdenServer.cpp:1574] mount point "/Users/kmancini/t-fbsource" stopped
V0324 18:26:27.215625 6572327 EdenMount.cpp:800] beginning shutdown for EdenMount /Users/kmancini/t-fbsource
0   edenfs#macosx-arm64                 0x000000010626a968 _ZN5folly10symbolizer17getStackTraceSafeEPmm + 36
1   edenfs#macosx-arm64                 0x000000010611739c _ZN5folly10symbolizer21SafeStackTracePrinter15printStackTraceEb + 108
2   edenfs#macosx-arm64                 0x000000010603e854 _ZN5folly10symbolizer12_GLOBAL__N_118innerSignalHandlerEiP9__siginfoPv + 232
3   edenfs#macosx-arm64                 0x000000010603c9d4 _ZN5folly10symbolizer12_GLOBAL__N_113signalHandlerEiP9__siginfoPv + 108
4   libsystem_platform.dylib            0x00000001bb3484e4 _sigtramp + 56
5   libsystem_pthread.dylib             0x00000001bb330eb0 pthread_kill + 288
6   libsystem_c.dylib                   0x00000001bb26e314 abort + 164
7   libsystem_c.dylib                   0x00000001bb26d72c err + 0
8   edenfs#macosx-arm64                 0x0000000105148ab0 _ZN5folly22DelayedDestructionBase15DestructorGuardD2 (45261a919d)Ev + 180
9   edenfs#macosx-arm64                 0x0000000105143438 _ZN5folly22DelayedDestructionBase15DestructorGuardD1 (19e5072be0)Ev + 28
10  edenfs#macosx-arm64                 0x0000000104df1eb0 _ZNSt3__124__optional_destruct_baseIN5folly22DelayedDestructionBase15DestructorGuardELb0EED2 (45261a919d)Ev + 44
11  edenfs#macosx-arm64                 0x0000000104df1e74 _ZNSt3__123__optional_storage_baseIN5folly22DelayedDestructionBase15DestructorGuardELb0EED2 (45261a919d)Ev + 28
12  edenfs#macosx-arm64                 0x0000000104df1e48 _ZNSt3__120__optional_copy_baseIN5folly22DelayedDestructionBase15DestructorGuardELb0EED2 (45261a919d)Ev + 28
13  edenfs#macosx-arm64                 0x0000000104df1e1c _ZNSt3__120__optional_move_baseIN5folly22DelayedDestructionBase15DestructorGuardELb0EED2 (45261a919d)Ev + 28
14  edenfs#macosx-arm64                 0x0000000104df1df0 _ZNSt3__127__optional_copy_assign_baseIN5folly22DelayedDestructionBase15DestructorGuardELb0EED2 (45261a919d)Ev + 28
15  edenfs#macosx-arm64                 0x0000000104df1dc4 _ZNSt3__127__optional_move_assign_baseIN5folly22DelayedDestructionBase15DestructorGuardELb0EED2 (45261a919d)Ev + 28
16  edenfs#macosx-arm64                 0x0000000104df1d98 _ZNSt3__18optionalIN5folly22DelayedDestructionBase15DestructorGuardEED2 (45261a919d)Ev + 28
17  edenfs#macosx-arm64                 0x0000000104df1bb8 _ZNSt3__18optionalIN5folly22DelayedDestructionBase15DestructorGuardEED1 (19e5072be0)Ev + 28
18  edenfs#macosx-arm64                 0x0000000104df3364 _ZN8facebook4eden9RpcServer17RpcAcceptCallbackD2 (45261a919d)Ev + 60
19  edenfs#macosx-arm64                 0x0000000104df0bd0 _ZN8facebook4eden9RpcServer17RpcAcceptCallbackD1 (19e5072be0)Ev + 28
20  edenfs#macosx-arm64                 0x0000000104df0bfc _ZN8facebook4eden9RpcServer17RpcAcceptCallbackD0Ev + 28
21  edenfs#macosx-arm64                 0x0000000102dfc330 _ZN5folly18DelayedDestruction16onDelayedDestroyEb + 88
22  edenfs#macosx-arm64                 0x0000000105148aec _ZN5folly22DelayedDestructionBase15DestructorGuardD2 (45261a919d)Ev + 240
23  edenfs#macosx-arm64                 0x0000000105143438 _ZN5folly22DelayedDestructionBase15DestructorGuardD1 (19e5072be0)Ev + 28
24  edenfs#macosx-arm64                 0x0000000104dedf3c _ZNSt3__124__optional_destruct_baseIN5folly22DelayedDestructionBase15DestructorGuardELb0EE5resetEv + 40
25  edenfs#macosx-arm64                 0x0000000104dede20 _ZN8facebook4eden9RpcServer17RpcAcceptCallback13acceptStoppedEv + 416
26  edenfs#macosx-arm64                 0x000000010514c260 _ZZN5folly17AsyncServerSocket14RemoteAcceptor4stopEPNS_9EventBaseEPNS0_14AcceptCallbackEENK3$_1clEv + 44
27  edenfs#macosx-arm64                 0x000000010514c228 _ZN5folly6detail8function14FunctionTraitsIFvvEE9callSmallIZNS_17AsyncServerSocket14RemoteAcceptor4stopEPNS_9EventBaseEPNS6_14AcceptCallbackEE3$_1EEvRNS1_4DataE + 32
28  edenfs#macosx-arm64                 0x0000000101fc287c _ZN5folly6detail8function14FunctionTraitsIFvvEEclEv + 40
29  edenfs#macosx-arm64                 0x0000000105d3b40c _ZN5folly9EventBase20FunctionLoopCallback15runLoopCallbackEv + 32
30  edenfs#macosx-arm64                 0x0000000105d3476c _ZN5folly9EventBase16runLoopCallbacksERN5boost9intrusive4listINS0_12LoopCallbackENS2_18constant_time_sizeILb0EEEvvvEE + 120
31  edenfs#macosx-arm64                 0x0000000105d31ef0 _ZN5folly9EventBase16runLoopCallbacksEv + 88
32  edenfs#macosx-arm64                 0x0000000105d336a8 _ZN5folly9EventBase8loopBodyEib + 1396
33  edenfs#macosx-arm64                 0x0000000105d31c10 _ZN5folly9EventBase8loopOnceEi + 44
34  edenfs#macosx-arm64                 0x000000010201a700 _ZN8facebook4eden10EdenServer14performCleanupEv + 2004
35  edenfs#macosx-arm64                 0x0000000101fb8d80 _ZN8facebook4eden11runEdenMainEONS0_8EdenMainEiPPc + 3240
36  edenfs#macosx-arm64                 0x0000000101f5e56c main + 56
37  dyld                                0x00000001159e50f4 start + 520
(safe mode, symbolizer not available)
```

for some reason the destructor is being called twice even though the constructor is only called once.

this makes the delated destructor count double decrement and hit its internal assert.

I think this is because std::optional is directly calling the destructor on the value maybe and so it is being double destructed.

keeping a direct member variable gets rid of the crash.

This probably deserves a follow up to understand why std::optional is doing this.

Reviewed By: xavierd

Differential Revision: D35134797

fbshipit-source-id: 6b9184c878962b7f773f6be55ac70a83dc00fa42
2022-03-25 13:43:16 -07:00
Katie Mancini
84057e7171 make parsing errors non fatal and log them
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
2022-03-24 15:20:43 -07:00
Katie Mancini
ee29930b55 log parsing errors more clearly
Summary:
A EdenFS messenger dogfooder on macOS is hitting our check here that file
handles be a certain size.

This likely indicates that we have a big somewhere in parsing that is causing
us to parse something that is not a file handle as a file handle. Its also
possible this request could be malformed.

Eventually we should not trust clients enough to allow them to crash eden with
a malformed request, but the first step to fixing this bug is understanding
wether the request that crashes us is malformed or not.

So first I am adding some extra logging info to this xcheck so that we can
determine the cause. I will follow up with a solution to prevent clients
from crashing EdenFS.

Reviewed By: xavierd

Differential Revision: D34969740

fbshipit-source-id: 783ca1147f6c9d9c86996836d14263a0a79d6b7d
2022-03-21 11:54:16 -07:00
Michael Cuevas
aa4732e433 refactor notifications
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
2022-03-01 17:52:58 -08:00
Xavier Deguillard
5995517153 inodes: do not call RequestContext::finishRequest in the destructor
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
2022-02-11 15:57:52 -08:00
Xavier Deguillard
6722ba460d inodes: move RequestContext::finishRequest to the destructor
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
2022-01-26 10:24:35 -08:00
Xavier Deguillard
a29d465ee8 fs: fix license header
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
2022-01-04 15:00:07 -08:00
Michael Cuevas
e79d41cc77 change entry3 and entryplus3 constructors
Summary: Instead of taking a uint64_t, we'll take in an InodeNumber. This allows us to avoid the conversion from InodeNumber -> uint64_t -> InodeNumber.

Reviewed By: chadaustin, xavierd

Differential Revision: D33135685

fbshipit-source-id: c9e4317b1f5f97ad924dce5da322860c91c64115
2021-12-15 21:13:36 -08:00
Michael Cuevas
d068b2e05a move include inside of windows ifndef
Summary: don't include this header if we're on Windows

Reviewed By: xavierd

Differential Revision: D33111628

fbshipit-source-id: 8bf6c7db4f023f7c348f1e1c439c15856acbf476
2021-12-15 16:08:54 -08:00
Michael Cuevas
670b274424 move statToPostOpAttr() and other functions into a utility file
Summary: In the next diff, we need to use statToPostOpAttr() in another file. We will move these utility functions so we can access them from any file.

Reviewed By: xavierd

Differential Revision: D32771540

fbshipit-source-id: f4d6e2819da3ef66248134337f98eb0a938b6edb
2021-12-15 16:08:54 -08:00