Summary:
Update all of the C++ unit tests that create temporary files and directories
to use the new `facebook::eden::makeTempFile()` and
`facebook::eden::makeTempDir()` functions.
Note that this likely changes the behavior of some code paths in meaningful
ways: `/dev/shm` normally does not support `getxattr()`, and Eden's overlay
code attempts to store the SHA-1 for materialized files as using extended
attributes. This means that the tests will now typically hit the fallback
code path and will not store SHA-1 data in the overlay.
Reviewed By: chadaustin, strager
Differential Revision: D12971162
fbshipit-source-id: 6cc5eba2e04be7e9a13a30e90883feadfb78f9ce
Summary:
Thanks to some bpf tracing by strager, we traced the ESTALE response to
`d_splice_alias` and noted this comment above the implementation in the kernel:
> If a non-IS_ROOT directory is found, the filesystem is corrupt, and
> we should error out: directories can't have multiple aliases.
Well, our magic `.eden` directory is a directory with aliases and we were
seeing the error trigger on that dir. So, this diff replaces hardlinking
directories into each tree with a hardlink to a symlink in each tree!
At mount time we create `.eden/this-dir` as a symlink to `/abs/path/to/mount/.eden`
so that `readlink("/abs/path/to/mount/sub/dir/.eden/socket")` still
resolves as it did prior to this diff.
Reviewed By: strager
Differential Revision: D12954819
fbshipit-source-id: 7f3b1b53f2bd5b9c51e64055fc34110657a19110
Summary:
Sandcastle has several cases where we chown the entire
repository which performs terribly on Eden. As a workaround we have a
command to do this in eden without loading all the files.
Reviewed By: chadaustin
Differential Revision: D12857956
fbshipit-source-id: 36cebcc710fbcf4e1eb265df901513cf50a227b9
Summary:
Checkout.modifyFile is the long pole when running the Eden unit
tests. Splitting it into three runs it across multiple threads. It
could be split further if needed.
Reviewed By: strager
Differential Revision: D12813684
fbshipit-source-id: 49e847d46bc54f16fcb2b273ec81e92eaf756dbb
Summary:
Using boost::filesystem to create the bind mount directories causes
edenfs to communicate with itself via FUSE, which causes edenfs to die
at startup (sometimes?). Instead, create the directories by traversing
the TreeInodes.
Reviewed By: simpkins
Differential Revision: D10154921
fbshipit-source-id: 4d8be71911e8a988b6fc5796904856f58a9a5153
Summary:
Instead of calling getBlobMetadata in multiple places and only using
the .sha1 field, add a getSha1 function directly to ObjectStore. This
gives ObjectStore the latitude to fetch it and store it in different ways.
Reviewed By: wez
Differential Revision: D10227935
fbshipit-source-id: 180830534db3c42c07f04216599e496406af5ced
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: We've diverged in a few places from clang-format, so run it across the entirety of Eden.
Reviewed By: wez
Differential Revision: D10137785
fbshipit-source-id: 9603c2eeddc7472c33041ae60e3e280065095eb7
Summary:
My eden crashed with an invariant violation in InodeMap. It looks
related to background unloading, so I attempted to reproduce the
sequence in a unit test.
These tests pass but they're worth checking in I think.
Reviewed By: strager
Differential Revision: D9998970
fbshipit-source-id: d182fc8d5b185c286082320adda3ea4c862f2f18
Summary:
If the overlay file for a directory is corrupted (e.g. empty), Overlay::scanForNextInodeNumber throws. This causes Eden to crash on start [1]. Fix the crash by ignoring corrupted directories.
[1] `test_mount_possible_after_corrupt_directory_and_cached_next_inode_number` reproduces this crash.
Reviewed By: chadaustin
Differential Revision: D9806105
fbshipit-source-id: 1b95083b6a6aa253a2296d6f754edbf4b9f64734
Summary:
Part of the larger project to modify Future<T>::then to be r-value qualified and use Future<T>::thenTry or Future<T>::thenValue.
The goal is to disambiguate folly::Future and to improve type and lifetime safety of Future and its methods.
Codemod:
future<T>.then(callable with operator()(not-a-try)) to future<T>.thenValue(callable with operator()(not-a-try)).
future<T>.then(callable with operator()()) to future<T>.thenValue(callable with operator()(auto&&)).
future<T>.then(callable with operator()(auto)) to future<T>.thenValue(callable with operator()(auto)).
future<T>.then(callable with operator()(folly::Try<T>)) to future<T>.thenTry(callable)
Reviewed By: Orvid
Differential Revision: D9819578
fbshipit-source-id: f9e31f47354c041ecbf0a90953cbe50ebfda6adc
Summary: When a RawOverlayTest test fails, it's sometimes difficult to diagnose the failure because assertions use inode numbers instead of file paths. When a test fails, include inode data (with file paths) in the failure message.
Reviewed By: chadaustin
Differential Revision: D9806106
fbshipit-source-id: 6160632bf8c64ceeb84e9d4709347e9268747ca4
Summary:
Reduce some code duplication in RawOverlayTest:
* Factor testDir_-to-AbsolutePath conversion into a getLocalDir function.
* Factor Overlay construction into a loadOverlay function.
* Use getLocalDir to construct the path to next-inode-number.
This refactor will let me use getLocalDir for more things in future diffs.
This diff should not change behavior.
Reviewed By: chadaustin
Differential Revision: D9813443
fbshipit-source-id: ab0dc88d36e91fc04e0aeb48468060b93b48f0ec
Summary: Add yet another unloading code path... This one is used for fast checkouts.
Reviewed By: strager
Differential Revision: D9781956
fbshipit-source-id: ae89377aea823f94e2ec1bcc2fa209c8f9bc821c
Summary:
unloadChildrenNow would only unload files or trees at the leaf. Apply
the fixes from D8302998 to unloadChildrenNow, which is only called
during takeover.
Reviewed By: strager
Differential Revision: D9774970
fbshipit-source-id: c2f4d1e6b838cc3b9e99eb8786114e643128a519
Summary:
Overall plan to modify Future<T>::then to be r-value qualified and use Future<T>::thenTry or Future<T>::thenValue.
The goal is to disambiguate folly::Future and to improve type and lifetime safety of Future and its methods.
Codemod:
* future<T>.then(callable with operator()(not-a-try)) to future<T>.thenValue(callable with operator()(not-a-try)).
* future<T>.then(callable with operator()()) to future<T>.thenValue(callable with operator()(auto&&)).
* future<T>.then(callable with operator()(auto)) to future<T>.thenValue(callable with operator()(auto)).
Reviewed By: chadaustin
Differential Revision: D9443286
fbshipit-source-id: be712b58b92dc7422f128713deaf6f46b29b36ce
Summary:
As a prerequisite to running inode unloading code in a background
queue, use a deterministic executor in TestMount to make sequencing in
unit tests reliable.
Reviewed By: simpkins
Differential Revision: D9323878
fbshipit-source-id: 0b85632c1637a8cf83d6f238675e5b6bbb6923c7
Summary:
Watchman's Eden integration has a bug where the combination of
Watchman querying Eden for overlapping delta ranges ("give me changes
between X and Y, now changes between X+1 and Y+1") and Eden eliding
redundant change events ("add-modify-remove" -> []) results in
Watchman sometimes reporting that a file exists in its final
subscription update when it no longer does.
The fix is to never elide events, even for files that were added and
removed in the same sequence. To continue to support Watchman's `new`
flag, track whether a file existed at the beginning and end of a
journal delta.
Reviewed By: wez
Differential Revision: D9304964
fbshipit-source-id: f34c12b25f2b24e3a0d46fc94aa428528f4c5098
Summary:
Overall plan to modify Future<T>::then to be r-value qualified and use Future<T>::thenTry or Future<T>::thenValue.
The goal is to disambiguate folly::Future and to improve type and lifetime safety of Future and its methods.
6/n: Codemod rvalue-future<T>.then(...) to rvalue-future<T>.then(...).
Reviewed By: yfeldblum
Differential Revision: D9152002
fbshipit-source-id: 166475c1dcafb29a11154cbfbdf7e2e1feaf745b
Summary:
To improve the determinism of our C++ tests, I am planning to switch
TestMount to a ManualExecutor. This adds a ManualExecutor constructor
to UnboundedQueueExecutor.
In Rust, I'd use a trait, but a simple class with two constructors works fine.
Reviewed By: strager
Differential Revision: D8846553
fbshipit-source-id: c52752105255503d26f1e65494c32b3f62882e44
Summary:
InodePath is difficult to work with because it's just an alias to std::array. Make InodePath look more like std::string so using InodePath instances is more intuitive.
This diff should not change behaviour.
Reviewed By: chadaustin
Differential Revision: D8923073
fbshipit-source-id: 85a2548f0cfa61e50b6590048084076b9bece3da
Summary:
Make use of the TopLevelIgnores class to hold the system and user
GitIgnoreStack details. This is a cleaner implementation making ownership
semantics more intuitive. In later commits we will provide acess to the
TopLevelIgnores as part of the ServerState. It will be dynamically loaded.
Reviewed By: simpkins
Differential Revision: D8906226
fbshipit-source-id: d955436582498861ac4b4113a47f357432c8f32e
Summary:
Some of the heavier watchman->eden queries in www today
trigger calls to `getFileInformation` with very large numbers
of paths.
For a set of paths with a lot of common prefixes there is a lot
of repeated work to load same inodes over and over again.
eg: `a/b/c/d/A` and `a/b/c/d/B` both have to load `a -> b -> c -> d` before
they can load the leaf node.
This diff pre-processes the list of paths and builds a tree-shaped plan
so that we won't need to load any inode referenced by any of the paths
more than once.
The `getSHA1` method could benefit from a similar change; I'll do that in
a follow-up diff.
Reviewed By: strager
Differential Revision: D8578261
fbshipit-source-id: e899640a2ef6dcdb6b903d2a2735e9240496d74b
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:
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: yfeldblum
Differential Revision: D8711368
fbshipit-source-id: fbfcb731097cdf9d8d98583956bc7fe614157a6b
Summary:
The OverlayGoldMasterTest.can_load_overlay_v2 unit test is failing stress test
runs because it keeps overlay data checked into the source repository and runs
the tests directly against this directory. This causes the tests to fail if
multiple test are run in parallel since they are all trying to use this same
directory simultaneously.
We need to make a copy of this directory rather than having the tests run
directly against the source tree to avoid locking issues. The overlay code
can potentially also try to upgrade the overlay format when it opens the
directory. This seems like another reason why it should not run directly
against the directory in the source repository.
This changes the test to make a copy of the overlay directory in a temporary
location, and run the tests against that directory. As part of this change I
also bundled the original input into a tar file.
Reviewed By: chadaustin
Differential Revision: D8555716
fbshipit-source-id: bf24bd96a0a31c097d9cf8e0fbe8b0bfaf009943
Summary:
There was a conflict between the DirContents refactoring and some of
the Overlay diffs.
(Note: this ignores all push blocking failures!)
Reviewed By: strager
Differential Revision: D8427680
fbshipit-source-id: a01a2a8456f7a35249d8b65f2b5fcd55825438ee
Summary:
As code review follow-up from D8321501 and D8330070, have getFilePath
return InodePath rather than the index to the null terminator.
Also move getFileTmpPath and ensureTmpDirectoryCreated into their own
functions
Reviewed By: strager
Differential Revision: D8363811
fbshipit-source-id: 42718ed4af07fc7a6600564cc7d934f85fe333ed
Summary:
To get a read on whether there's anything egregious about the
overlay on various filesystem types and whether we can make any quick
improvements, I wrote up this quick benchmark.
Reviewed By: strager
Differential Revision: D8310783
fbshipit-source-id: 15fa1ea4bcbb7e4896e52b4acf0f8802133dbf3c
Summary:
Per a conversation with simpkins when code reviewing D7882648, this
diff removes the inheritance relationship between TreeInodeState and
DirContents. It doesn't change the binary layout of anything, but
defines DirContents as a typedef of PathMap<DirEntry>.
Reviewed By: strager
Differential Revision: D8232052
fbshipit-source-id: a2166f3ca2ab90fabbded0e48307b8a92a2b0250
Summary:
The two FileInode::isSameAs() methods called FileInode::getSha1(), which
returns a folly::Future object, and immediately called `value()` on the
resulting future without waiting for it to complete first.
This bug dates back to D5430128. However, I suspect it was D7888344 that
actually exposed this issue recently. D7888344 updates the
`RocksDbLocalStore` code to perform work in an I/O thread pool. Before this
change the SHA1 futures were likely always immediately ready, so this bug was
not causing problems.
Reviewed By: wez
Differential Revision: D8393671
fbshipit-source-id: ec2116751ddda31a119bfe85eab5612b622f83cf
Summary:
This fixes a bug simpkins pointed out in D6891479 - we weren't
updating mtime and ctime on renames.
Reviewed By: simpkins
Differential Revision: D7937303
fbshipit-source-id: 08fd8f4fe5d99d33e9f7629965d6146330c8f35b
Summary:
1. Enabled a number of additional C++ compiler warnings in Eden.
2. Fixed warnings-turned-errors that resulted from this change.
Reviewed By: simpkins
Differential Revision: D8132543
fbshipit-source-id: 2290ffaaab55024d582e29201a1bcaa1152e6b3e
Summary:
Like D7867399, split TreeInode's synchronized state into a top-level
class. This is a step towards using the type system to perform
lock-safe metadata updates.
Reviewed By: simpkins
Differential Revision: D7882648
fbshipit-source-id: 27262df8ed9137c8478c68ebf4c4f13878655754
Summary:
Remember the maximum inode number in a file when the Overlay is shut
down cleanly, avoiding a full Overlay scan.
Reviewed By: simpkins
Differential Revision: D7912647
fbshipit-source-id: 8adbafed15259af668a221baa829e1b1f44090d7
Summary:
The Overlay is the natural home for nextInodeNumber_ now that every
directory allocates inode numbers for its children right away. This
also simplifies serializing nextInodeNumber_ to disk in the following
diff.
Reviewed By: simpkins
Differential Revision: D8192442
fbshipit-source-id: 9b776a73c8d7653002b55985d592b1746e52f878
Summary:
Before making changes to the way we record the maximum inode number,
cover it with tests.
Reviewed By: simpkins
Differential Revision: D7912059
fbshipit-source-id: 29fdc88bd74c6c07e870017d743d9fd0797baaf3
Summary:
Most filesystems limit path components to 255. To remain consistent,
let's have Eden do the same, at least for write operations.
Reviewed By: simpkins
Differential Revision: D8151020
fbshipit-source-id: 251da94a076f5765111c8e3d9d8a25c37682e2e3
Summary:
Relax the restriction on changing uid/gid on inodes. We'll see what
cans of worms this opens I guess. (Landing this is low priority, but
might be important for making some of the existing tooling in fbsource
and www work.)
Reviewed By: simpkins
Differential Revision: D7768655
fbshipit-source-id: 95fe02fe7ddc001335dbdb34e16a989a85820240
Summary:
Unify how inode metadata is modified across inode types. This allows
changing the permission bits on a directory.
Reviewed By: simpkins
Differential Revision: D7767254
fbshipit-source-id: 35e9cf652c84c7d8680cc22dec7942e94e9f5af1
Summary: Store tree and file timestamps in the InodeTable so they persist across runs.
Reviewed By: simpkins
Differential Revision: D6891479
fbshipit-source-id: 1c9e6266375aceeaf293a81e73cf7f5334dbc32d
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:
This allows multiple inodes to update their metadata simultaneously
without contending on InodeTable's locks.
Reviewed By: simpkins
Differential Revision: D7885245
fbshipit-source-id: cc8ab6cd90b7424beec314a115852e08b64026ae
