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
Summary:
The FuseChannel destructor previously could be invoked while there are still
outstanding requests, which would cause problems when responses were generated
for these requests.
This makes the FuseChannel destructor private, requiring users to always
allocate FuseChannel objects on the heap and destroy them using a destroy()
method. The destroy() method still blocks waiting for the FUSE worker threads
to exit, but if there are still outstanding FUSE requests it defers the
deletion of the FuseChannel until they are complete.
The EdenMount class currently couldn't ever destroy FuseChannel objects with
outstanding requests, since it waits for the completion future to fire before
invoking the destructor. Fixing this behavior is mostly for correctness of the
FuseChannel API and for the benefit of our unit tests.
Reviewed By: chadaustin
Differential Revision: D7297829
fbshipit-source-id: 643d1332be84a1f25ee30ba2a2ea3515806f00ef
Summary:
Make ServerState also store the clock as a member variable. This moves more
server-wide state into the ServerState class, and allows us to eliminate
another argument to the EdenMount constructor.
Reviewed By: chadaustin
Differential Revision: D7297832
fbshipit-source-id: 2a063d67752f46686987390b3faefb304455568a
Summary:
Move the EdenCPUThreadPool from EdenServer to ServerState.
This ensures that all EdenMount objects always have a thread pool. Previously
most unit tests were creating an EdenMount without a thread pool set, but the
FileInode code and TreeInode code could end up wanting to use the thread pool
even if no FuseChannel is running.
Reviewed By: chadaustin
Differential Revision: D7297828
fbshipit-source-id: 9861b3439cd091afe31fa13f449aa14656983318
Summary:
Add a simple test program that starts a FUSE mount. At the moment the only
call it actually responds to is getattr() for the root inode. However this is
sufficient for exercising the FuseChannel startup and shutdown code.
Reviewed By: chadaustin
Differential Revision: D7297827
fbshipit-source-id: 1bae6015def37913a5bf57541a14c3c8c050eff5
Summary:
Remove the `FuseChannel::getSessionCompleteFuture()` method and instead have
`initialize()` and `initializeFromTakeover()` return the `Future` that can be
used to wait on session completion.
This makes it explicit from an API perspective that this session completion
future will only be invoked if initialization is successful. It also
eliminates the possibility of anyone calling getSessionCompleteFuture() more
than once.
I also changed the session completion future to use a folly::SemiFuture rather
than a folly::Future, to make it explicit that callers generally should not
execute their callback inline in the same thread where the SemiFuture is
fulfilled.
Reviewed By: chadaustin
Differential Revision: D7297835
fbshipit-source-id: 3a1157951f0738f1692833ed5e875c3e9c6d6d69
Summary:
The TakeoverServer::ConnHandler code uses a folly::EventBase to drive the
socket I/O, and therefore must perform all I/O from EventBase thread.
This changes ensures that we always call `sendTakeoverData()` from the correct
thread.
Up until now this has happened to work since the future returned by
`EdenServer::startTakeoverShutdown()` happened to always be completed in the
EdenServer main EventBase thread, and this is the same thread used for the
TakeoverServer socket.
This change makes it explicit that this particular code must be run from the
correct EventBase thread.
Reviewed By: chadaustin
Differential Revision: D7297834
fbshipit-source-id: cbace5f63ea13aa62dd53be0b89d59bea2779cf4
Summary: Testing convenience. The NO_CHECK constructor is no longer necessary now that simpkins made the original constructor constexpr.
Reviewed By: simpkins
Differential Revision: D7295539
fbshipit-source-id: 717f5a7a8d3d9790966fad441da7b3308495e2c3
Summary: Cleaning up the takeover initialization code for EdenMount and InodeMap.
Reviewed By: simpkins
Differential Revision: D7294419
fbshipit-source-id: 58506f04259bb1017e6cd2e80e40e5820de6200f
Summary:
Instead of having a rule that save() must be called after
InodeMap::shutdown, just have InodeMap::shutdown return the
SerializedInodeMap if it's desired.
Reviewed By: simpkins
Differential Revision: D7292773
fbshipit-source-id: 2ba35fc729e19af122fe5d6c5a3287ad6b8af946
Summary:
The FuseChannel unit tests would sometimes hang because a worker thread would
still be stuck in a blocking read() call.
The issue appears to be that the kernel would often automatically restart the
interrupted read(), so the worker would never wake up. This switches the code
from using SIGPIPE to wake up, and instead uses SIGUSR2, and installs a handler
for SIGUSR2 with the SA_RESTART flag unset.
It's possible that this was an issue only for the unit tests, where the FUSE
device is a socket. I'm not sure if the kernel would automatically restart
read operations on a real FUSE device.
There are still theoretically some race conditions here, since
`requestSessionExit()` could be called right after a worker thread checks
`(runState_` and before it enters the blocking `read()` call. Fixing that
would likely require switching to something like `folly::EventBase` or manually
using epoll. Fortunately I haven't seen that be an issue in practice so far.
Reviewed By: chadaustin, wez
Differential Revision: D7282002
fbshipit-source-id: 99d29de13a7858dd25f258fdc94d8f8c71ee84d9
Summary:
This changes FuseChannel to fulfill the session complete future immediately in
the thread where it finishes. This may occur inside a FUSE worker thread.
It is now up to the caller to use `via()` if they want their callback to run in
a separate thread.
This simplifies the FuseChannel code and also addresses a crash that occurs in
the unit tests sometimes: If the FuseChannel destructor is invoked without
explicitly invoking the FuseChannel, it would schedule the session complete
promise to be fulfilled in a separate EventBase thread. However, the promise
may then have already been destroyed by the time it can run in the EventBase
thread.
There are still additional synchronization issues to fix in the FuseChannel
tests, but this resolves one problem.
Reviewed By: chadaustin
Differential Revision: D7282001
fbshipit-source-id: be64d3ef6a0e664ed7a2cf93a549acc140184095
Summary:
If you run `eden daemon` on a machine where sudo needs input, sudo's
output would get redirected to edenfs.log and eden daemon would appear
to hang. Worse, if you pressed ctrl-C, sudo would remain in the
background and continue to swallow keypresses from the tty, appearing
to somewhat break your shell until it gave up. The fix is to stop
redirecting stdout and stderr from Python and instead have edenfs
redirect itself into the log if given.
Reviewed By: simpkins
Differential Revision: D7223399
fbshipit-source-id: bae17b150b6594a3dd87bb88741c8dcefd1c5213
Summary:
Update FuseChannel::startWorkerThreads() to return without doing anything if
runState_ indicates that the FuseChannel is currently stopping.
This addresses a race betwen startWorkerThreads() and the destructor. If the
destructor is invoked in parallel with startWorkerThreads() the destructor can
grab the state_ lock and clear out the workerThreads list first. This then
causes problems when startWorkerThreads() runs and adds new threads to the
workerThreads list, which then never get joined.
Reviewed By: wez
Differential Revision: D7253574
fbshipit-source-id: f2cac11f1e71e1a14e3f020368152e0f2948c625
Summary:
Update FuseChannel::readInitPacket() to honor `runState_` and stop if it is
changed to indicate that the channel is stopping. Previously the init worker
thread would always wait until it received an INIT packet or an error on the
FUSE channel.
This also refactors the `readInitPacket()` code slightly mainly to to reduce
the level of indentation.
Reviewed By: wez
Differential Revision: D7253575
fbshipit-source-id: 7f27698947b629daecd9cd4ddffb6e6a921ce41e
Summary:
Update the FuseChannel code to keep track of why it is stopping, and to return
this data in the session complete future.
This is mainly just available for diagnostic reasons, and at the moment nothing
is using this outside of the unit tests.
Reviewed By: wez
Differential Revision: D7253573
fbshipit-source-id: 0c7e5b8bd9c3c1de3788918c2257c06d4fe0a90c
Summary:
Empty .gitignore files should be processed normally, and treated as no new
ignore rules. The logic in D6659654 had accidentally introduced a bug where we
would completely skip processing any directories that contained empty
.gitignore files.
Reviewed By: wez
Differential Revision: D7261276
fbshipit-source-id: 033e199f15d7763bff5f9a080a226c50e481aa9d
Summary:
Catch exceptions thrown by FuseChannel::processSession() and terminate the
FuseChannel if any errors occur.
Also explicitly mark `fuseWorkerThread()` and `initWorkerThread()` as
`noexcept` so we will crash with useful exception backtraces if an unhandled
exception does ever attempt to propagate out of these functions. (Prior to
gcc 8.x, `std::thread()` ends up swallowing the exception backtrace information
if a thread function throws an exception.)
Reviewed By: chadaustin
Differential Revision: D7243909
fbshipit-source-id: fbe7173a2167532d7e42901f810c17bb173d8d63
Summary:
- Update FuseChannel::initializate() to not require an Executor. Rather than
performing initialization using the supplied Executor, it simply starts one
worker thread first and performs initialization there.
- Change the API semantics slightly so that the session complete future is
invoked only if initialization succeeds. Previously the session complete
future could be invoked if initialization failed as well.
- Replace the activeThreads counter with a stoppedThreads counter to determine
when the session complete future should be invoked. The previous
activeThreads behavior was somewhat racy, since the activeThreads counter was
incremented inside each worker thread, but most places that checked it did
not wait to ensure that all worker threads had successfully incremented it
yet.
Reviewed By: chadaustin
Differential Revision: D7243910
fbshipit-source-id: 93b3465509bd9bf6fa90ea097e70dac3193172f9
Summary: It's a little surprising to me the clang build passed without these explicit template instantiations!
Reviewed By: simpkins
Differential Revision: D7253536
fbshipit-source-id: 2f48d5571777f4e978b6947183eefb03158d5014
Summary:
Split up the FuseChannel and EdenMount APIs for starting a brand new FUSE
channel vs taking over an existing channel.
These operations are somewhat different as initializing a new FUSE channel
requires performing a negotiation with the kernel that will not complete
immediately. Therefore these APIs return a Future object. Taking over an
existing FUSE channel is always an immediate operation, and therefor does not
need to return a Future object.
Reviewed By: chadaustin
Differential Revision: D7241997
fbshipit-source-id: 22780f2df76b2d554ab2a4043b6425fa7a4e9c94
Summary:
Update FuseChannel to only join its worker threads in its destructor, rather
scheduling a function in the main EventBase thread to join each thread as soon
as it completes.
This makes FuseChannel a bit easier to use: the destructor can now be called at
any point in time (as long as it is not called inside one of the worker threads
themselves). If it is called while the FuseChannel is still running this will
automatically stop the worker threads.
Reviewed By: wez
Differential Revision: D7224369
fbshipit-source-id: 4de0e7c0c677e870fd629eaaaa5ab40d134ee870
Summary:
Ensure that we ignore SIGPIPE in unit tests. The
`FuseChannel::requestSessionExit()` function sends SIGPIPE to its worker
threads, and expects this to wake up the worker threads if they are stuck
inside a read() call.
In normal program builds folly/Subprocess.cpp causes SIGPIPE to be ignored on
startup. However, Subprocess.cpp is not necessarily linked into all of our
unit tests. I haven't tracked down exactly why, but SIGPIPE in the unit tests
seems to kill the entire process only in opt-ubsan builds.
In the future we probably should clean up how requestSessionExit() triggers its
worker threads to exit: the current code is still potentially racy, since the
worker threads could receive the SIGPIPE just after checking `sessionFinished_`
and before calling `read()`, in which case they may go back to sleep rather
than exiting. However for now this should be a simple fix to get the tests
passing in all build modes.
Reviewed By: wez
Differential Revision: D7224370
fbshipit-source-id: a56fe3331fc5aa6a49ccbe6b0678b479a44ffc07
Summary:
Only the FuseChannel code should invoke the handler methods. This makes them
all private.
This did require making handlerMap be a private static member variable of
FuseChannel so that it can access these methods during its initialization.
Reviewed By: wez
Differential Revision: D7126002
fbshipit-source-id: 28c421173cb6cf8b7a8e1ca085ad78ec9ea76b8f
Summary: This method is never removed anywhere, so delete it for now.
Reviewed By: wez
Differential Revision: D7126003
fbshipit-source-id: 974645c8063fc17c4e69e5e46c870e52fecfcd65
Summary:
To make it clearer to me why all the calls to newPtrLocked were safe, and to
eliminate some duplication, I captured the newPtrLocked call patterns into
member functions on LoadedInode and TreeInode::Entry.
Reviewed By: simpkins
Differential Revision: D7207542
fbshipit-source-id: 25de77e72c0898be43b3fbdddab835d64101755e
Summary:
To make it clearer why newPtrLocked is safe during the Inode class
construction process, add a takeOwnership function (better name
suggestions welcome) that converts from the newly-instantiated
unique_ptr to a managed InodePtr.
Reviewed By: simpkins
Differential Revision: D7204818
fbshipit-source-id: 8a40189588442490120623da86195c6fc99c9c51
Summary:
In some cases we could call `delete` with the wrong size in
`UnixSocket::SendQueueDestructor` when `__cpp_sized_deallocation` is available.
In particular, if the input message data contained some empty buffers in the
IOBuf chain we would allocate room for these elements when initially performing
the allocation in `createSendQueueEntry()`, but we would skip over them in the
`SendQueueEntry` constructor, so `iovCount` did not include them. The
`SendQueueDestructor` code used `iovCount` to calculate how much space had been
allocated, and so it would undercount the amount of allocated space in this
case.
This updates `createSendQueueEntry()` to also avoid allocating an iovec entry
for the empty buffers, so that `iovCount` should always accurately reflect how
many entries were originally allocated.
Reviewed By: chadaustin
Differential Revision: D7190579
fbshipit-source-id: 422cc737f146adeb1c133b9f3b500038e05bad10
Summary:
This diff is mostly preparation. It's submitted separately to
remove mechanical refactoring noise from the following diff, where
every entry is assigned an inode number upon construction.
Reviewed By: simpkins
Differential Revision: D7116832
fbshipit-source-id: 2943c45340a9a751eb52bf13e19d233d829494c0
Summary:
If the root TreeInode wants to allocate inode numbers, the inode
allocator must be initialized first. But complete InodeMap
initialization requires the root TreeInode. So split this into two
parts.
Also, I changed the inode allocator to a single atomic increment instead
of a lock acquisiton.
Finally, the extra assertions in this diff uncovered what looks like a
bug in the takeover logic where nextInodeNumber_ could end up being
smaller than the value in the takeover data, since the max inode
number from the overlay was assigned after loading from takeover data.
Reviewed By: simpkins
Differential Revision: D7107706
fbshipit-source-id: ec43cc81c11d709261598739c622609b372433a2
Summary:
We were curious why we needed `sudo gdb` to debug Eden even
though it drops privileges. The process dumpable bit is the trick.
Reviewed By: simpkins
Differential Revision: D7182742
fbshipit-source-id: 48e3dd778bf07ec1bfadace4edeb64668b936b9b
Summary:
This was an in-person code review item I forgot to enable
with the original diff.
Reviewed By: wez
Differential Revision: D7016624
fbshipit-source-id: 91d729772aa3c0b476f6bf8f6ee7e46cdac54626
Summary:
This integration test verifies that observed timestamps should persist
across runs. D6891479 makes it pass.
Reviewed By: wez
Differential Revision: D6930208
fbshipit-source-id: b8c95bce00933b9ae0de101a0bd8b6abfbfa1177
Summary:
Today, attaching gdb to edenfs requires sudo. Our hypothesis is that,
even though we drop uid and gid, the SELinux context remains elevated,
preventing ptrace from attaching.
Reviewed By: simpkins
Differential Revision: D7128692
fbshipit-source-id: 0f9b9a2ebef5a438bf0f4dcdc00a7b703a7aea02
Summary:
1. Moved the elapsed time calculation and logging for getScmStatus() request to the future completion.
2. Added a helper class "ThriftLogHelper" to attach the time logging to the future completion.
Reviewed By: simpkins
Differential Revision: D7118876
fbshipit-source-id: 5f8212296cd8725a3556c7f5c364ddfa66dbdc95
Summary:
I spent way too long trying to figure out why my refactorings were
causing invariant errors inside InodeMap. It turns out that we
initialize the root TreeInode before InodeMap::initialize is called,
which I suspect resulted in duplicate inode numbers being handed out.
Reviewed By: simpkins
Differential Revision: D7106302
fbshipit-source-id: b459734fb96bfbb6b4b27a1d23de8b6406d30ca4
Summary:
The getpath_unloaded_inode tests failed on my machine quite
regularly. The two possible races here I can imagine are racing the
system clock and FUSE not having released its refcount on an inode by
the time unload is called.
Reviewed By: wez
Differential Revision: D7118883
fbshipit-source-id: c3708f14a860f5ad04ddec988fc67a683b7dcfde
Summary:
`DCHECK_NE()` unfortunately is not constexpr, so it results in a build error
when used in a constexpr function. Fortunately `assert()` is allowed in
constexpr functions as of C++14, so use it instead.
Reviewed By: wez
Differential Revision: D7108887
fbshipit-source-id: 2be9181929a357c23f48c09173646683060aad28
Summary:
This adds a FakeFuse class to simulate a FUSE connection to the kernel, but
that is actually communicating with our userspace test harness code. This also
adds a FakePrivHelper class which implements the PrivHelper interface but
returns FakeFuse connections rather than real FUSE connections to the kernel.
This will enable us to write unit tests that exercise more of the FUSE and
EdenMount logic, and will also enable us to test error conditions and ordering
behaviors that are difficult to reliably reproduce with a real FUSE mount.
This also includes some very basic tests using this new code. The code in
fuse/test/FuseChannelTest.cpp creates a FuseChannel using a FakeFuse object,
and the code in inodes/test/FuseTest.cpp creates a full EdenMount object using
FakePrivHelper and FakeFuse. The tests are pretty similar for now, and only
exercise the FUSE initialization code. I will expand these tests in subsequent
diffs.
Reviewed By: wez
Differential Revision: D7050826
fbshipit-source-id: 4f82375d65664ca3a5a228a577caa4d1d47454fe
Summary:
I'm seeing test failures that I have not yet understood and I
thought they might be caused by an implicit conversion from
fusell::InodeNumber to bool. Well, they're not, but this is how I
discovered that. I'm not sure I want to land this change, but I'm
going to leave it around until I figure out what's happening with my
other diffs.
Reviewed By: simpkins
Differential Revision: D7077635
fbshipit-source-id: 50bf67026d2d0da0220c4709e3db24d841960f4b
Summary:
I am working on a stack of diffs that changes how we allocate inode
numbers to tree entries. I was hitting test failures I could not
understand, so in the process of trying to understand the flows
through InodeMap, I found newChildLoadStarted to be redundant with
shouldLoadChild.
Note: Today, allocateInodeNumber() acquires the InodeMap's lock, but
in a later diff, inode numbers will be assigned en masse during
TreeInode construction.
Reviewed By: simpkins
Differential Revision: D7059719
fbshipit-source-id: 624b861040d585d2cae41d7ec2aae7d528ff8336
Summary:
Previously we returned an ENOENT error in response to a FUSE lookup() call for
a name that does not exist. However, this does not allow FUSE to cache the
result, so we will continue to receive lookup() calls for this path in the
future.
This changes EdenDispatcher to return a successful response with an inode
number of 0 instead. This tells the kernel that the name does not exist, but
allows the kernel to cache this negative lookup result (for as long as
specified in the entry_valid field in the response).
Reviewed By: wez
Differential Revision: D7076811
fbshipit-source-id: a2b9977e58d6b6eecb584699b9d93b5ad29ad5ad
Summary:
Update the TreeInode code to always update the FUSE cache whenever we add a new
entry to the directory.
Up to now we have been fine without this since the kernel never cached negative
lookup responses for us. In order to turn on FUSE caching of negative lookup()
responses we need to invalidate the cache whenever we create a new entry.
- Have create(), symlink(), mknod() and mkdir() invalidate the FUSE entry cache
if they are not being invoked from a FUSE request. These are almost always
invoked from a FUSE request, but flushing the cache is the correct thing to
do if we add code in the future that can trigger these from a thrift call or
via some other non-FUSE code path.
- Change the checkout code to invalidate the FUSE cache whenever it creates new
children entries.
Reviewed By: wez
Differential Revision: D7076812
fbshipit-source-id: d6489995b415260e84b3701c49713b0ef514f85d
Summary:
Add a test that exercises `hg pull`. This confirms that eden can see new
commits created on the server after Eden and its hg_import_helper processes
have started. This test gets run in flatmanifest, hybrid treemanifest, and
treeonly mode.
This currently performs pulls using a local peer repository rather than over
SSH. This does exercise a different code path in mercurial than what typically
occurs in production. In the future we should perhaps also add a test that
uses a fake SSH helper program to exercise mercurial's sshpeer code paths as
well.
Reviewed By: chadaustin
Differential Revision: D6993788
fbshipit-source-id: 40628c0b3faac0dc8622b605a29b084979b8c089
Summary:
Whenever we tell Eden to change the working directory
parents, we need to make sure the appropriate objects are written to
disk. This fixes hg fold in Eden.
Reviewed By: simpkins
Differential Revision: D7045299
fbshipit-source-id: cbd51be59cf943a843b77c2abe66a84b745bce22
Summary:
Update the integration tests to avoid specifying an explicit path to the eden
extension. This way they use the version that we now package into hg.par
during the build.
This avoids issues with hg not being able to find and load native .so libraries
from the eden extension. Mercurial is able to find these libraries correctly
when they are packaged into hg.par (since the par start-up script sets
LD_LIBRARY_PATH to point to the par unpack directory). When using eden from an
external directory mercurial was not able to find these libraries.
Reviewed By: chadaustin
Differential Revision: D7047245
fbshipit-source-id: d56bffa953c178949c866efec507298a1f40da8b
Summary:
This removes the TARGETS files from the eden github repository. The
open source buck build has been failing for several months, since buck
removed support for the thrift_library() rule.
I will potentially take a stab at adding CMake build support for Eden
at some point in the future.
Reviewed By: chadaustin
Differential Revision: D6893233
fbshipit-source-id: e6023094a807cf481ac49998c6f21b213be6c288
Summary:
Most uses of `size_t` in `eden` are unqualified, but a few are qualified.
As discussed ad nauseum in
https://stackoverflow.com/questions/5813700/difference-between-size-t-and-stdsize-t
it is totally safe to use unqualified `size_t` with all compilers/platforms.
Since this saves 5 chars per use, and to improve uniformity, I ran:
```
$ find ~/fbsource/fbcode/eden -type f \
| egrep '\.(h|cpp)$' \
| xargs sed -i 's/std::size_t/size_t/g'
```
Reviewed By: chadaustin
Differential Revision: D7021980
fbshipit-source-id: da268e62a9a93d2a5168a40b6878795ae7516b7f
Summary: Per code review comments on D6983198, this simplifies the way we check if mode bits have changed in a meaningful-to-source-control way.
Reviewed By: simpkins
Differential Revision: D7015339
fbshipit-source-id: 548ead337fbea1c1dcb72b880921671e9b6188ac
Summary:
mode_t isn't really part of a TreeEntry and I also wanted to see all
the places where we convert an entry type from source control into
mode bits.
Reviewed By: simpkins
Differential Revision: D6983198
fbshipit-source-id: ce1d0976f5fc5130c34a8c93c07a4e26a7cdaf71
Summary:
This is the type of a tree entry, which may be another tree, so
FileType is not an accurate name.
Reviewed By: simpkins
Differential Revision: D6981168
fbshipit-source-id: 997eb8a27f599310ed678ce221c8083722db8bff
Summary:
Add a command line flag to control whether or not Eden should ever try falling
back to import tree data using flatmanifest if an error occurs trying to import
it directly via treemanifest, in repositories that support treemanifest.
This is particularly useful for tests, where we usually do not want to fall
back to flatmanifest import if an error occurs during treemanifest import. The
fallback can otherwise mask real issues that should trigger test failures.
This is probably also a good thing to have in general. Supporting
flatmanifest+treemanifest data in a single Eden repository has some unfortunate
problems today: we compute hashes differently for flatmanifest trees vs
treemanifest trees. As a result, we can end up with identical trees that have
different hashes. This can result in unfortunate performance consequences in
some cases where Eden assumes it must scan a directory for differences if the
hashes are different.
I have left flatmanifest import enabled by default for now, but we may want to
disable it by default in the future. I would be more inclined to disable it by
default if we did added a thrift method to explicitly re-enable it (or to
import a single commit using flatmanifest), so that users could work around
this setting if necessary without having to fully restart edenfs.
Reviewed By: wez
Differential Revision: D6993791
fbshipit-source-id: 6e091a426cf1e7c973df5a641d2f8a1101011346
Summary:
Update the integration tests to build `hg_import_helper` into a python archive
that includes the current mercurial sources from the local repository. This
way hg_import_helper will use the local mercurial code rather than whatever
mercurial modules are installed on the system. This will help ensure that we
detect any breakages caused by changes in the mercurial source when the
mercurial changes are made rather than when they are deployed.
Reviewed By: wez
Differential Revision: D6993790
fbshipit-source-id: f3ad404583cadcf07156bac1ce6bc869bd1160e1
Summary:
While we're changing how timestamps are stored, we might as well
implement this small-ish efficiency. Instead of storing timestamps as
a timespec (16 bytes), store them as 64-bit nanoseconds with a range
slightly larger than what ext4 supports. Assuming a million inodes,
this saves 24 MB.
This diff introduces the EdenTimestamp type. The next diff will start
using it.
Reviewed By: simpkins
Differential Revision: D6957659
fbshipit-source-id: 4551af6f5b8c1ff610ba88795f69e7d69d7f605d
Summary:
I want to rename FileType to TreeEntryType so I removed this one first
and replaced all of its uses with an isTree() method.
Reviewed By: simpkins
Differential Revision: D6980501
fbshipit-source-id: 105b8c599585e63efd44043e761db40e2824e77e
Summary:
Our Model TreeEntry code was a bit too general - in reality, both git
and hg only support a handful of specific tree entries: regular files,
executable files, symlinks, and trees. (git also supports
submodules.) This diff delays the expansion of a TreeEntry's type
into a full mode_t.
Reviewed By: simpkins
Differential Revision: D6980003
fbshipit-source-id: 73729208000668078a180b728d7e0bb9169c6f3c
Summary:
Update the `hg_test` decorator to accept additional parameters specifying the
list of test modes. e.g., `hg_test('Treemanifest')` asks to only run the test
in the Treemanifest configuration. With no arguments tests are still run with
both the Flatmanifest and Treemanifest configurations by default.
This also enables the TreeOnly configuration mode, which appears to work now.
(It was previously disabled since `hg init` would fail in treeonly
repositories.)
This new changes allows tests to explicitly opt-in to running in `TreeOnly`
mode.
Reviewed By: wez
Differential Revision: D6993789
fbshipit-source-id: 9ee51318d0f661038fe29f246b2b14eebbb1c3d9
