Summary:
Update the Eden CLI to preserve the `ASAN_OPTIONS` environment variable when
starting edenfs.
Reviewed By: wez
Differential Revision: D7440148
fbshipit-source-id: c9932c5c14b5a2e040309268f95229c04715bb28
Summary:
Fix FuseChannel::processSession() to always process all FUSE requests that it
reads. Previously it checked to see if it should stop immediately after
reading FUSE request. It was possible for the old process to successfully read
a FUSE request, see that it was supposed to stop, and then exit this worker
thread without ever processing this FUSE request. This would cause the client
that sent the request to hang indefinitely, since no response would ever be
sent.
Reviewed By: wez
Differential Revision: D7436867
fbshipit-source-id: c58c2f6c49102fa6b66ac83fc1639595a5277ce0
Summary:
Perform cleanup with an `addCleanup()` function rather than in `tearDown()`.
This is slightly simpler, but also avoids throwing an exception in `tearDown()`
if `setUp()` failed before it created the thrift client.
Reviewed By: wez
Differential Revision: D7436636
fbshipit-source-id: fe525c1fac86bb866c560cca0ad940ea87d96f07
Summary:
Update the `eden clone` command to accept extra command line flags telling it
where to find the edenfs executable, as well as additional arguments to pass
to edenfs.
This enables the `test_clone_should_start_daemon()` integration test to pass in
flags to tell it how to start edenfs correctly.
Reviewed By: chadaustin
Differential Revision: D7433367
fbshipit-source-id: 0eddd93d1332e113fb85fa4b8fc87ba51d7eab2c
Summary:
Update most of the Eden CLI code to include python type annotations.
I believe the stats.py and stats_print.py are the only CLI files that do not
have complete typing information now.
Reviewed By: chadaustin
Differential Revision: D7433368
fbshipit-source-id: dfd6a064cacffeeed9147739da7064f3303de789
Summary:
Update test_version() so that it can succeed even if the RPM is not installed
on the local system.
Reviewed By: wez
Differential Revision: D7433366
fbshipit-source-id: 5502cca02f9b481cc136beb665048525bae72807
Summary:
This cleans up the two createOverlayFile() implementations as well as
saveOverlayDir() to all use the same logic for writing out overlay files. This
new logic is more efficient than some of the old code, as we only open the file
once.
Additionally, this also changes the Overlay code to use openat() and related
APIs when accessing files in the overlay, so that we can specify paths relative
the top-level overlay directory. This means that all pathnames are guaranteed
to fit in a small path length known at compile time, and we can avoid ever
allocating path names on the heap.
One potential downside of using openat() is that this functionality may not be
available on Mac OS X, so we will likely still need to provide alternate
implementations that do use heap-allocated absolute paths for Mac support.
Reviewed By: chadaustin
Differential Revision: D7411499
fbshipit-source-id: dd76395130dda4c2b9403cce04f4201f6def0d10
Summary: Add a test case to confirm the behavior of Overlay::getFilePath().
Reviewed By: chadaustin
Differential Revision: D7411498
fbshipit-source-id: f859f29e252c1c296eb236113ad2a8b0a892db37
Summary:
Most of the logic in TreeInode::create(), symlink() and mknod() was very
similar. This adds a TreeInode::createImpl() helper method that these three
functions can share.
This also updates TreeInode::create() to throw EEXIST if the file in question
does exist, rather than using EDEN_BUG(). I believe it is possible (but
unlikely) for this to occur in practice since inode invalidation operations
triggered by a checkout are now processed asynchronously.
Reviewed By: chadaustin
Differential Revision: D7411500
fbshipit-source-id: 85d97995139eee6bff96381561fc28e76d7a2b7c
Summary:
Update all Overlay APIs to accept InodeNumbers and compute the path to the
overlay file internally, rather than requiring the caller to pass in the path
to the file inside the overlay.
Reviewed By: chadaustin
Differential Revision: D7411497
fbshipit-source-id: d057babb0b1b8d6c3b9adf1a21b84eed363dffa1
Summary: After the changes in D7052470 and D7346263, this bit is no longer necessary.
Reviewed By: simpkins
Differential Revision: D7401540
fbshipit-source-id: 04f3ae0376625d4e9584f31932f46774eb5caba4
Summary:
This adds a new LockedState helper class in FileInode to also keep track of
whether or not we are currently holding an refcount on the open file/blob. The
LockedState object automatically decrements this refcount on destruction.
Alternatively, this refcount can be transferred to a new EdenFileHandle object
when unlocking the state.
Previously most of the internal FileInode code simply used EdenFileHandle
objects to manage this refcount. However, this is prone to deadlock, since you
have to ensure that EdenFileHandle objects are never destroyed while the state
is already locked. This new LockedState API makes it harder to have code paths
that may accidentally destroy an EdenFileHandle object while holding the state
lock.
Reviewed By: chadaustin
Differential Revision: D7407423
fbshipit-source-id: 610fcda3220a9f49b734910b7a13e8d68a81a779
Summary:
Update InodeMap::onInodeUnreferenced() to take a pointer to a non-const
InodeBase. This allows the methods it calls, including
InodeBase::updateOverlayHeader() to be non-const.
Using non-const InodeBase objects seems to make sense here since the inode is
in the process of being destroyed.
In a future diff I plan to update FileInode::updateOverlayHeader() to share the
same code as normal file methods to ensure that the overlay file is open. This
modifies the FileInode state's open refcount, so it is useful to have this
method be non-const for that purpose.
Reviewed By: chadaustin
Differential Revision: D7407424
fbshipit-source-id: 541656c7b9b283c5e5650445de5bbdbaae3fc57f
Summary:
Update FuseChannel to send all invalidation requests in a separate thread.
This eliminates a deadlock that could previously occur during checkout
operations. The invalidation requests would block until they could acquire the
kernel's inode lock on the inode in question. However, the inode lock may
already be held by another thread attempting to perform an unlink() or rename()
call. These FUSE unlink or rename operations would be blocked waiting on
Eden's mount point rename lock, which was acquired by the checkout operation.
Checkout operations now let the invalidations complete asynchronously, but we
wait for all invalidation operations to complete before indicating to our
caller that the checkout has succeeded.
Reviewed By: chadaustin, wez
Differential Revision: D7404971
fbshipit-source-id: 6fa20c00d054e210eb0258d247d083010557f210
Summary:
The status_deadlock_test code has a small helper function used to create
directory trees in the test. This moves that function into a helper module so
we can re-use it in other tests in the future.
Reviewed By: chadaustin
Differential Revision: D7407492
fbshipit-source-id: 257e5a2ce7543bb6cd218b412d165f0fac852970
Summary:
Request data was coming back as null because of a race between ctx->getContextData() in FuseChannel::getOutstandingRequests() and the it initialization.
Also, added a check to verify rdata is valid before dereferencing it.
Plus, fixed debug.py where output gave an error.
Reviewed By: chadaustin
Differential Revision: D7377993
fbshipit-source-id: 8343119983c74185fd5d8cc05c2f5af63dcff99e
Summary:
Decouple inode number assignment from materialization status.
The idea is that we will always assign entries an inode number and
track whether an entry is materialized otherwise. This is necessary
to give consistent inode values across remounts.
Reviewed By: simpkins
Differential Revision: D7052470
fbshipit-source-id: 80d3f2a2938463198a3132182537e6223c79d509
Summary:
Verify Eden handles looking up an inode by number after graceful
restart and checkout.
Reviewed By: simpkins
Differential Revision: D7346263
fbshipit-source-id: 876b4837708da9ac31f72c06e7defc797fe126f3
Summary: Small refactoring prior to a fix in a later diff.
Reviewed By: simpkins
Differential Revision: D7358218
fbshipit-source-id: 198160df1aa0dc65b917c5a96c2998c3675b03fe
Summary: Now we can write unit tests for takeover. :)
Reviewed By: simpkins
Differential Revision: D7345419
fbshipit-source-id: 05741dab65016c59109bf190c83be2f676a6141d
Summary:
This replaces the `ensureDataLoaded()`, `materializeForWrite()`, and
`materializeAndTruncate()` methods in `FileInode` with new safer alternatives:
`runWhileDataLoaded()`, `runWhileMaterialized()`, and `truncateAndRun()`
These new methods take a function to run, and run it while holding the state
lock with the FileInode guaranteed to be in the desired state. They also
require the caller move in the state lock as an argument, to help ensure that
the caller gets the locking behavior correct.
The old methods required that the caller not already hold the state lock while
calling them. This pre-condition was violated in `FileInode::write()` and
possibly other places. They also required releasing the lock between they
confirmed the file state and when the caller's function ran, requiring
additional lock and unlock operations, and also making the code harder to
reason about since other threads could change the state during the time when
the lock was released.
Reviewed By: chadaustin
Differential Revision: D7363180
fbshipit-source-id: d8e667d0bc7006c519252a8d0682af97517997eb
Summary:
The changes in this diff comments out unused parameters. All changes are automated using clang-tidy.
This will allow us to enable `-Wunused-parameter` as error.
Reviewed By: simpkins
Differential Revision: D7371610
fbshipit-source-id: 0134e2f0b916313d690c073a46d747c52399a226
Summary:
FileInode::read() did not handle being called in the BLOB_LOADING or NOT_LOADED
states. This method is only called from EdenFileHandle::read(), which does no
checking that it is fully loaded first.
This fixes read() to always call ensureDataLoaded() first.
Reviewed By: chadaustin
Differential Revision: D7359878
fbshipit-source-id: f5a1c8a28db3267da3180b67f970430e3ea291da
Summary:
Update edenfs to enable the `async` setting for the default log handler.
This will avoid ever blocking due to logging, and will cause edenfs drop
messages if they are being logged faster than they can be written to stderr.
Reviewed By: chadaustin
Differential Revision: D7348517
fbshipit-source-id: 7cdf5772742e4e92bb15dd4de6315103cd42301e
Summary:
Set the default logging settings by overriding folly::getBaseLoggingConfig()
rather than by setting a default value for the `--logging` command line
argument.
This has two advantages:
- Custom logging settings supplied in the `--logging` argument will now
combined with the base logging settings rather than completely replacing
them. Previously users had to always add "eden=DBG2" to this argument if
they did not want to drop this setting.
- This will make it possible to move the definion of the `--logging` flag to
folly::Init and not lose our custom settings for Eden.
Reviewed By: yfeldblum
Differential Revision: D7348519
fbshipit-source-id: bd9012f387b0460da8bcc9c7c31aef46b1abec5c
Summary:
While debugging a takeover crash (task #25590050) we wanted to help confirm
that inode objects weren't ever getting copied somehow. That turned out not to
be the issue, but it still seems worth explicitly deleting the copy and move
constructors and assignment operators for InodeBase.
Reviewed By: wez
Differential Revision: D7342050
fbshipit-source-id: ae07214072bfb8353584a07066aae2dc1adf2797
Summary:
Add a new integration test that performs a graceful restart after invoking the
getScmStatusBetweenRevisions() thrift call.
Prior to D7341609 this would cause edenfs to crash on shutdown with the error
"!!BUG!! After InodeMap::shutdown() finished, 2 inodes still loaded; they must
all (except the root) have been unloaded for this to succeed!"
Before D7341609 the `EdenMount::diffRevisions()` created a new temporary inode
tree solely to perform a diff. This resulted in multiple root inodes that all
pointed to the same EdenMount, but the EdenMount didn't know about any of these
alternate root inodes. These temporary inode trees never got destroyed,
causing this error on shutdown.
Reviewed By: chadaustin, wez
Differential Revision: D7333005
fbshipit-source-id: 8406d2e2ceb00264050b0aceec583baae2da69ec
Summary:
Update the getScmStatusBetweenRevisions() thrift call to use the new
diffCommits() function that diffs source control Tree objects without creating
TreeInode objects to perform the diff.
This addresses two bugs:
- Each call to EdenMount::diffRevisions() constructed a new root inode
associated with the mount, and this would never get destroyed. It was not
destroyed at the end of the diffRevisions() call since inodes are normally
not destroyed immediately when they are unreferenced. It was not destroyed
during EdenMount::shutdown() since EdenMount didn't have any references to
these additional root inode structures and their children.
- EdenMount::diffRevisions() incorrectly swapped ADDED and REMOVED statuses in
the result.
Reviewed By: wez
Differential Revision: D7341609
fbshipit-source-id: 16e755a0ff685f51c977c3b27d6af96908f33494
Summary:
Add a function for diffing two source control commits without needing to
instantiate TreeInode objects.
Reviewed By: wez
Differential Revision: D7341604
fbshipit-source-id: 557eef87faa2785ab96d51b09569a46f892a71f6
Summary:
Update Eden's thrift service handler code to accept BinaryHash arguments either
as 20-byte binary values or as 40-byte hexadecimal values.
This will make it easier to transition APIs like getScmStatusBetweenRevisions()
to use 20-byte binary hash arguments without breaking existing clients.
Reviewed By: wez
Differential Revision: D7341607
fbshipit-source-id: 3e952211900d3ec4b9c2073cf3afd55ae7e253ea
Summary:
Add an integration test for the getScmStatusBetweenRevisions() thrift call.
This call apparently gets the ADDED and REMOVED states backwards. For now the
test checks for the current (incorrect) behavior.
This also fixes the thrift definition for this function to stop using the
BinaryHash typedef. Unlike most of our other thrift functions this method
appears to require the arguments as 40-byte hexadecimal strings.
Reviewed By: wez
Differential Revision: D7341606
fbshipit-source-id: 73cbd0ecf4445da6b1f0ef9cf6d9dce47e6fb593
Summary:
This makes gitrepo.commit() return the new commit ID as a hexadecimal string,
just like hgrepo.commit() currently does.
Reviewed By: wez
Differential Revision: D7341605
fbshipit-source-id: 83ebddb8c23d5e4650432bea6f8dcb8d18c0ff38
Summary:
Update the hgrepo and gitrepo helper classes with a new `remove_file()` method,
and also improve the git code to include the process stderr in the exception
message if the command fails.
Reviewed By: wez
Differential Revision: D7341610
fbshipit-source-id: 28cca89520923d92bba4833a4dcfab6d21357cfb
Summary:
Update repobase.py, hgrepo.py, and gitrepo.py to include python type hints for
all functions and to pass most of mypy's strict type checking.
There are still a couple minor errors around the fact that hgrepo.hg() and
gitrepo.git() return an `Optional[str]` rather than a plain `str`. Many callers
know that they cannot return None unless the stdout argument is None, but mypy
can't figure this out. In the long run maybe we should split these into two
separate methods, one that always returns `str` and one that always returns
`None`.
Reviewed By: wez
Differential Revision: D7341608
fbshipit-source-id: c62da578fb32edb9272363fadabbdc11b1d5d2c2
Summary:
Fix the code to generate exceptions based on an errno error using
std::generic_category rather than std::system_category.
Reviewed By: yfeldblum
Differential Revision: D7329997
fbshipit-source-id: 3fe257bbbc7a631c801f31120592c8bdbc25c8bf
Summary:
Add a new utils/SystemError.h header with helper functions to check if a
`std::system_error` contains an errno value.
Most of the code in Eden previously only checked for `std::system_category`
when looking for errno values. `std::generic_category` is the correct category
to use for errno exceptions, but folly/Exception.h incorrectly throws them as
`std::system_category` today. This change makes Eden treat either error type
as errno values for now.
Reviewed By: yfeldblum
Differential Revision: D7329999
fbshipit-source-id: 67a3c3ea10371c53a2e34236b7575deac4cbd53a
Summary:
Added a thrift call to return the outstanding FUSE requests.
Cli will call the thrift and print the output.
Added a unit test to test getOutstandingRequests().
Reviewed By: simpkins
Differential Revision: D7314584
fbshipit-source-id: 420790405babdb734f598e19719b487096ec53ca
Summary: Fix a simple TODO in EdenServiceHandler.cpp
Reviewed By: chadaustin
Differential Revision: D7329635
fbshipit-source-id: b42b2cc13be3ad5b18a629ed15f6c51cea52fbda
Summary:
This helps distinguish `facebook::eden::FileHandle` from
`facebook::eden::fusell::FileHandle`, and will make it possible to eliminate
the separate `facebook::eden::fusell` namespace in the future.
Reviewed By: chadaustin
Differential Revision: D7314456
fbshipit-source-id: 0869427766fe666d119a59c7df1c97825a8ad36e
Summary:
This test exercises code under eden/fs/fuse, so put it with that directory's
tests.
Reviewed By: chadaustin
Differential Revision: D7314457
fbshipit-source-id: b664aaa3086b2e65f7ae8b76ae71878f39fd9d17
Summary:
There's some code that uses Entry::hasInodeNumber to decide whether an
inode is in the process of being loaded. This change is a
prerequisite for D7052470.
Reviewed By: simpkins
Differential Revision: D7311182
fbshipit-source-id: e04a89c814950e6ce33e73b963b3ffb0f0e353b8
Summary:
As of D7300847 the FuseChannel code will now return the FUSE FD as a closed
File object if the mount was unmounted, even if a takeover stop was requested.
This updates the EdenServer code to deal with the fact that it might receive a
closed FD even during a takeover. The correct behavior is to simply skip this
mount when building the takeover state to pass to the new process.
This is a somewhat minimal change to fix up this bit of the code. It would
probably be nice to continue simplifying this logic in the future.
Reviewed By: wez
Differential Revision: D7300877
fbshipit-source-id: c2f617449e155e4b42784ac42ec3aaa378e3288b
Summary:
Previously the FuseChannel code unconditionally set `connInfo_.major` and
`connInfo_.minor` to the FUSE version that it supports, rather than the FUSE
version supported by the kernel it is talking to.
This does not appear to be the intended behavior, given that in several other
location it checks the value of `connInfo_->minor` to see if particular
features are supported.
Reviewed By: wez
Differential Revision: D7300845
fbshipit-source-id: 1f5946134037b2cbe03a8b86241acaa401c21ffa
Summary:
Remove the `FuseChannel::stealFuseDevice()` method, and instead change the
session completion future to return the FUSE device FD if it is still valid.
This ensures we only extract the FUSE device when it is still valid: if an
unmount event is being processed by a FUSE worker thread simultaneously with a
call to `FuseChannel::takeoverStop()` the session completion future will return
an empty `folly::File` since the FUSE device has been shut down.
This also helps clarify the synchronization semantics around modification to
the `fuseDevice_` member variable.
Reviewed By: wez
Differential Revision: D7300847
fbshipit-source-id: 02ced8fc9d24e7cd526d911782949d0bfbf0e5a7
Summary:
I noticed from the logs that this was funky:
```
C0131 01:55:28.870926 24027 Bug.cpp:55] EDEN_BUG at eden/fs/inodes/InodeMap.cpp425: !!BUG!! InodeMap::save() called with 104957 inodes still loaded; they must all (except the root) have been unloaded for this to succeed!
```
Reviewed By: chadaustin
Differential Revision: D7310995
fbshipit-source-id: 8c7840bbd60e3e5e815c8cef0d50bf4021e89baf
Summary:
Eliminate a few unnecessary includes from Dispatcher.h and one from
RequestData.h
Reviewed By: wez
Differential Revision: D7300846
fbshipit-source-id: 0cde1f70f605fcb5c95ed13950f4fa8d353fd72f
Summary:
FuseChannel::processSession() was missing a return statement after we received
EOF from the FUSE device. This would cause us to fall through and try
processing the empty buffer as a request. This would fail, and we would try to
send a reply, which would in turn fail since the FUSE device was closed.
This was only an issue with the unit tests: on real FUSE devices we get an
ENODEV error when reading from the device. In unit tests we get an EOF
instead.
Reviewed By: wez
Differential Revision: D7299336
fbshipit-source-id: c275e03edbf594c0d120257b1cfc89d7c75f60f6
