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
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:
While running the secfs filesystem validation tests against Eden, I
discovered a test that caused the eden process to abort. I bisected
and found that D7451330 regressed renaming a directory onto an empty
one. This fixes that case.
Reviewed By: simpkins
Differential Revision: D7945727
fbshipit-source-id: 592ede1b391528c02cd12b2b6ebbf3733fe8f503
Summary:
I got tired of typing PathComponentPiece{"..."} in tests so here are
some operator literals.
Reviewed By: simpkins
Differential Revision: D7956732
fbshipit-source-id: 85d9f3fd725853a54da9e70fc659bd7eb9e0862c
Summary:
A persistent (but notably non-durable) mapping from inode
number to a fixed-size record stored in a memory-mapped file. The two
primary goals here are:
1. efficiently (and lazily) reify timestamps for inodes that aren't in the overlay
2. allow the kernel's page cache to drop pages under memory pressure
Reviewed By: simpkins
Differential Revision: D6877361
fbshipit-source-id: a4366b12e21e2bf483c83069cd93ef150829b2ac
Summary:
Historically, we have seen a number of messages like the following in the Eden
logs:
```
Journal for .hg/blackbox.log holds invalid Created, Created sequence
```
Apparently we were getting these invalid sequences because we were not always
recording a "rename" correctly. The "rename" constructor for a `JournalDelta`
assumed that the source path should be included in the list of "removed" files
while the destination path should be included in the list of "created" files.
However, that is not accurate if the destination path already existed before
the user ran `mv`.
Fortunately, we already check whether the destination file exists in
`TreeInode::doRename()`, so it is straightforward to determine whether the
action is a "rename" (destination does not exist) or an "replace" (destination
already exists) and then classify the destination path accordingly.
As demonstrated by the new test introduced in this commit
(`JournalUpdateTest::moveFileReplace`), in the old implementation,
a file that was removed after it was overwritten would not show up as
removed in the merged `JournalDelta`. Because Watchman relies on
`JournalDelta::merge()` via the Thrift method `getFilesChangedSince()`,
this would cause Watchman to report such a file as still existing even
though it was removed.
This definitely caused bugs in Nuclide. It is likely that other tools that rely
on Watchman in Eden (such as Buck) may have also done incorrect things
because of this bug, so this could explain past reported issues.
Reviewed By: simpkins
Differential Revision: D7888249
fbshipit-source-id: 3e57963f27c5421a6175d1a759db8d9597ed76f3
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:
To avoid bugs similar to the ones fixed in D7781691, mark a bunch of
folly::Future<folly::Unit> functions with FOLLY_NODISCARD.
Reviewed By: simpkins
Differential Revision: D7782224
fbshipit-source-id: 23ba42aa63011cc33e5a6e18d5bc6d00403a78d3
Summary:
Disallow any kind of mutation operation inside of the .eden directory. We had some
code in place to prevent some of this already, but errors (including EPERM) weren't
passed out from unlink and rename out to FUSE.
Reviewed By: simpkins
Differential Revision: D7781691
fbshipit-source-id: aaecf13779eca75d6ee8765fc8bb3727ce9341de
Summary: Also print the inode path if the assertions in EXPECT_FILE_INODE fail.
Reviewed By: simpkins
Differential Revision: D7035517
fbshipit-source-id: 6c50acb588d1c985c7e7a1586c06f04a657698f9
Summary:
Now that Eden writes tree data into the overlay even when not
materialized, checkouts can be slow as Eden actively tries to forget
allocated inode data for potentially large trees when
kPreciseInodeMemory is false. This pushes the bulk of that garbage
collection work onto a background thread, making checkouts fast again.
Reviewed By: simpkins
Differential Revision: D7683120
fbshipit-source-id: 2a22e4612a4438b3ed2527670343c49dfcd902bc
Summary:
This updates the code to store ServerState using a shared_ptr rather than
having it be an inlined member variable of EdenServer.
Previously EdenMount objects contained a raw pointer to the ServerState, with
the reasoning that the EdenServer object should outlive the EdenMount objects.
It turns out that this is not quite true in practice--EdenMount::destroy() will
normally be called before the EdenServer is destroyed, but this may not
actually destroy the EdenMount object immediately.
This fixes a race condition in the FuseTest.initMount() test that could cause
this test to occasionally fail when run on a heavily loaded system.
Reviewed By: chadaustin
Differential Revision: D7720509
fbshipit-source-id: 056ff5985587c8d8c32c11d17ba637ebd7598677
Summary:
I was worried we might leak data in the overlay in this particular
sequence of operations, so here's a test that shows we don't.
Reviewed By: simpkins
Differential Revision: D7663419
fbshipit-source-id: b4b0d08f863fe7b23501e9e4943d18ba0b746e42
