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
Summary:
Update the integration tests so that they do not delete the temporary test
directories if the environment variable `EDEN_TEST_NO_CLEANUP` is set.
This makes it easier to manually examine the repository state after a test
fails.
Reviewed By: chadaustin
Differential Revision: D6986217
fbshipit-source-id: 727321c2c3da4d19d9edf8ed20b2aca3449779de
Summary:
Update the logic for how the Eden integration tests find the hg binary:
- Use the contents of the EDEN_HG_BINARY environment variable if set. When
running tests via `buck test` buck will pass the hg.par output location in
this variable.
- If EDEN_HG_BINARY is not set, use libfb.py.pathutils to find the location of
the //scm/hg:hg rule output. This makes sure the integration tests still
prefer this par path even when run manually without EDEN_HG_BINARY set. This
is convenient when running individual tests not through buck.
If for some reason the hg python_binary() output cannot be found then we still
search through $PATH for hg.real or hg as usual. For internal fbsource builds
we generally shouldn't hit this fallback case, though.
Reviewed By: wez, quark-zju
Differential Revision: D6986221
fbshipit-source-id: 982cb99112405a674dbc45df4ada73a990536489
Summary: I should have caught this before...
Reviewed By: wez
Differential Revision: D6995395
fbshipit-source-id: 08efd0aacb051f2b1754930b311e3a42afc3c538
Summary:
Remove the declaration of HgRepo::initHg(), which is never used or defined
anywhere. (There is a public HgRepo::hgInit() method.)
Reviewed By: chadaustin
Differential Revision: D6996812
fbshipit-source-id: 1ad15f624b3839ccb8a2005f3bcf0897563f82f5
Summary:
Several parts of the mercurial remotefilelog and treemanifest code can
incorrectly throw KeyErrors when accessing legitimate objects that do exist.
This appears to occur if the data was created since the mercurial code on the
server side started.
These KeyErrors are often generated on the server-side of a pull or
prefetchtrees operation. We can't workaround these by invalidating caches on
our local repository object. The only way that I've been able to work around
these bogus KeyErrors is by completely recreating the repo object before
retrying.
Our code now generally attempts to load data 3 times: if the initial attempt
fails we call `repo.invalidate()` and then retry. If that retry fails we then
try destroying and recreating the entire repo object and then try again.
Ideally it would be nicer to fix mercurial itself not to throw KeyErrors for
data that does exist. However that seems somewhat more complicated.
this_is_fine
Reviewed By: wez
Differential Revision: D6986220
fbshipit-source-id: 57905dd25e11c4858822020b44185a6f83ecd363
Summary:
Explicitly mark the UnionDatapackStore as needing a refresh after we have
fetched data from the server. Otherwise we may not be able to find the tree
data that we just fetched.
Reviewed By: wez
Differential Revision: D6986219
fbshipit-source-id: c50b92ee4242665c7f5770f87a5dbab17698b8b9
Summary:
Recent changes to the treemanifest extension removed the
treemanifest.prefetchtrees() function, and replaced moved this functionality to
a repo.prefetchtrees() function instead.
Update the Eden extension to call `repo.prefetchtrees()` if it is available.
Reviewed By: wez
Differential Revision: D6986218
fbshipit-source-id: 02e07ab5194f311e49d8ecb4755d79d55acb080c
Summary:
Another small refactoring that I don't want mixed into my
bigger diffs.
Reviewed By: wez
Differential Revision: D6927482
fbshipit-source-id: 28cc60dfdffb50921a5ef9cca4e2814b90d3b701
Summary:
Introduces a persistent non-durable storage mechanism backed
by a memory-mapped file with fixed-length records.
Reviewed By: simpkins
Differential Revision: D6877217
fbshipit-source-id: 0ddacb4137cfe43e67c822dce4064356cdf515b5
Summary:
There were places we were acquiring a lock unnecessarily. In
addition, I'm looking at reducing the number of places where we store
the full mode_t to see if we can get away with dirtype_t or something
similar.
Reviewed By: wez
Differential Revision: D6972140
fbshipit-source-id: bb29a4473f3056e39596600d22e67374ca484735
Summary:
We would immediately satisfy the health check and
tell the user that the system is healthy and show the pid of
the prior incarnation rather than the one for the instance
that we just launched.
This diff refactors the health checking code so that we can
share the implementation between the cli and the integration
tests; the integration tests already had code to do the right
thing for this.
Reviewed By: simpkins
Differential Revision: D6944989
fbshipit-source-id: 7c0f02c875b1b81f8f1b7521add67928200b27ed
Summary:
chadaustin is going to think about how to test this
in unit tests intestead :-p
Reviewed By: chadaustin
Differential Revision: D6951788
fbshipit-source-id: 137f285f3a1f080ce43392a621c73640ce3a9bf7
Summary:
This makes the default `memory` for speed and minimal
flakiness, but allows a test to select a different engine where
appropriate (eg: restart and remount tests).
Reviewed By: chadaustin
Differential Revision: D6944207
fbshipit-source-id: 1fb11387beda02d059a796dad5a42d56ddcf6e88
Summary:
refactor this test so that we can apply it to any LocalStore
implementation, and have it run against the Memory, Rocks and SQLite
implementations.
Reviewed By: chadaustin, simpkins
Differential Revision: D6919455
fbshipit-source-id: cc93042b95833b175955e6395c84cf41238a90d2
Summary:
Adds a SQLite storage implementation and makes it the
default engine for integration tests; this requires fewer resources
to run and the integration tests thus run faster and more reliably.
In the future we may add a configuration option to remember the
storage engine that was used as it is currently not "safe" to switch
from one to the other because the hgproxyhash data cannot be
recreated without re-importing a revision.
Reviewed By: simpkins
Differential Revision: D6919456
fbshipit-source-id: 3afbfafb190cca0e3c797cd9b7cd051768575a8c
Summary:
Update FileInode so that getattr() and setattr() both return a reasonable value
in st_blocks.
Previously we always returned 0 in st_blocks, which caused applications like
`du` to always report files as using 0 space in Eden mounts. Now we compute
st_blocks based on st_size, so that `du` will report reasonable estimates for
when scanning the size of subdirectories inside an Eden mount.
Reviewed By: chadaustin
Differential Revision: D6932098
fbshipit-source-id: bd29e46821176e510f420e6e2b6ce480b80d50ff
Summary:
Rename the existing `state_` variable to `runState_` to help distinguish it
from the new `serverState_` variable.
The information in `runState_` is all related to whether the EdenServer object
is starting, shutting down, or running normally.
Reviewed By: wez
Differential Revision: D6929864
fbshipit-source-id: ad7af381a8a291b12db9308668c7616ebd9b7f39
Summary:
Move all of the privhelper functionality into a PrivHelper class. The
ServerState object now stores the PrivHelper object to use, rather than having
a global singleton.
This will make it easier to stub out the PrivHelper functionality during unit
tests.
Reviewed By: wez
Differential Revision: D6929862
fbshipit-source-id: e3edcb0a03ba9afdf34554cb961fd74557cdd6e3
Summary:
Drop the --thrift_address flag, and always create the thrift socket using a
fixed name under the `.eden` directory. The location of the `.eden` directory
is still configurable with the `--edenDir` argument.
There isn't really much benefit to making the socket path be configurable
separately from the .eden directory path, and it adds to the code complexity.
For instance, while you can tell eden to listen on a TCP socket instead of a
Unix domain socket, that functionality has been broken since D4637285
introduced a `CHECK()` statement that crashes the code when using a TCP socket.
Reviewed By: wez
Differential Revision: D6929863
fbshipit-source-id: ee5f7341d01d3ce522cae936ef3c133bba3f18f7
Summary:
Update EdenMount::diff() to use the UserInfo object stored in the shared
ServerState rather than calling UserInfo::lookup() on each diff operation.
Reviewed By: wez
Differential Revision: D6929865
fbshipit-source-id: a68ab1fa9eb345b59972e67c3aac258b4dbcdab5
Summary:
Add a new ServerState class to store process-wide state that is shared across
multiple mounts. Up until now we have passed around the shared state data as
separate variables.
This is intentionally separate from the existing EdenServer class to allow unit
tests to create EdenMount objects without an EdenServer object.
Reviewed By: wez
Differential Revision: D6929861
fbshipit-source-id: 5f22efb6d79dfd70031be1dc37f494c2ad8af902
Summary:
To me, restart implied shutting down the daemon and restarting it
again. Perhaps instead of `eden daemon --takeover` we should have
`eden restart`. But if people typed `eden restart` I imagine they're
trying to debug a problem, so that's probably not the right verb.
Reviewed By: wez
Differential Revision: D6929166
fbshipit-source-id: d568a1940d67f755e4c3656098c58fc81e0a3156
Summary:
We no longer need this and I believe that this was contributing
to this source of flakiness in our CI; this stack trace triggers when
we get an aborted or short read from the kernel:
```
*** Aborted at 1517827659 (Unix time, try 'date -d 1517827659') ***
*** Signal 11 (SIGSEGV) (0x2d0) received by PID 153706 (pthread TID 0x7f63ea37d700) (linux TID 160573) (maybe from PID 720, UID 0) (code: address not mapped to object), stack trace: ***
@ 0000000002abfa2d folly::symbolizer::(anonymous namespace)::signalHandler(int, siginfo_t*, void*)
./folly/experimental/symbolizer/SignalHandler.cpp:413
@ 00007f646e96dacf (unknown)
@ 00007f646e96aa20 __pthread_kill
@ 0000000001873748 facebook::eden::fusell::FuseChannel::requestSessionExit()
./eden/fs/fuse/FuseChannel.cpp:483
@ 000000000187d2f7 facebook::eden::fusell::FuseChannel::processSession()
./eden/fs/fuse/FuseChannel.cpp:656
@ 000000000187dab9 facebook::eden::fusell::FuseChannel::fuseWorkerThread(unsigned long)
./eden/fs/fuse/FuseChannel.cpp:493
@ 00007f646f267170 execute_native_thread_routine
@ 00007f646e9637a8 start_thread
@ 00007f646e047a7c __clone
```
my theory is that we're allowing shutdownImpl to free things out from under other
threads before they've all seen this signal and wound down fully. This is slightly
speculative in that I haven't managed to reproduce this stack trace on my devserver.
We don't really need this additional signal any longer.
Reviewed By: simpkins
Differential Revision: D6907734
fbshipit-source-id: 0f0138b631a7201fc9a4a1c93c2cde846e869cbd
Summary: Small refactoring I should have done with the previous diff.
Reviewed By: simpkins
Differential Revision: D6927152
fbshipit-source-id: 1dcda01134c3d63c62169c5728dba24ca0eebd68
Summary:
This enables dropping in alternative implementations
of LocalStore and adds a MemoryLocalStore implementation for
use in our tests.
This diff doesn't change the default storage option for the
eden server. I'll look at adding such an option in a follow up diff.
Reviewed By: chadaustin
Differential Revision: D6910413
fbshipit-source-id: 018bf04e0bff101e1f0ab35e8580ca2a2622e5ef
Summary:
Change `LoggerDB::get()` to a reference instead of a pointer since this
function can never return null.
Reviewed By: yfeldblum
Differential Revision: D6893206
fbshipit-source-id: af47063918a79c851fd39b838d6c63755166e033
Summary: This should make it less flaky
Reviewed By: simpkins
Differential Revision: D6908454
fbshipit-source-id: 07622b469ec635972290c3a4e5d1ed4b05ddd51a
Summary:
Update hg_import_helper.py to parse the repository's .hg/hgrc file, but then
create the repository object with a fresh UI object that has not parsed that
config yet.
This more closely mimic's the behavior of mercurial's dispatch code invoked
when starting the `hg` command line. This behavior is required to ensure that
secondary repository objects that get created end up with the correct
configuration, and do not have settings from the original repository's hgrc
file.
The mercurial behavior of parsing the original repository's hgrc file twice
dates back to rHG741f64dfc04d1.
Reviewed By: quark-zju
Differential Revision: D6909449
fbshipit-source-id: 85073ab6ade4ab70247d48bc670c9924e9e6841f
Summary:
This was causing flakiness in the form of premature timeouts
generating failure status. Looking at the docs for hypothesis, this
option is deprecated and slated to be replaced with an infinite timeout
in the future:
http://hypothesis.readthedocs.io/en/latest/settings.html#hypothesis.settings.timeout
Those docs suggest using `hypothesis.unlimited` instead, but I couldn't import
that symbol (maybe our version is too old?) so I'm just using 0 which means
that no timeout will be applied.
Reviewed By: simpkins
Differential Revision: D6908200
fbshipit-source-id: 1143d4745fc84d9c14b29ff5a2906a4e776d14fe
Summary:
Update `TreeInode::diff()` to check if its hash matches the source control tree
it is being compared to, and return early if they are identical.
I'm surprised that I forgot to include this initially when implementing
`TreeInode::diff()`
This makes `hg status` faster when a large number of unmodified directories
have been loaded.
Reviewed By: chadaustin
Differential Revision: D6890615
fbshipit-source-id: 561630d0220b4875dbf3678161cdb41a8aa4fc82
Summary:
This re-orders some of the code in `TreeInode::diff()` slightly. This should
not affect the behavior of the code.
This moves the `isIgnored` check inside the main `contents_.wlock()` block.
This reduces the number of places where we grab the lock, and will help keep
things simple for an upcoming diff where I need to add some more checks in this
code with the lock held.
This also changes `inodeFuture` to use the new `Future::makeEmpty()`
constructor rather than having to use an `Optional<Future>`
Reviewed By: chadaustin
Differential Revision: D6890616
fbshipit-source-id: 354bbf6a6be6d356fd23e6c0fb6b534679bbe0bb
Summary: We don't think this is happening, but let's test it!
Reviewed By: simpkins
Differential Revision: D6888038
fbshipit-source-id: 754b2ec8f78ff513fd350a74505915e2a1e9ba3e
Summary: Small things I've needed in later diffs.
Reviewed By: wez
Differential Revision: D6877755
fbshipit-source-id: c9002eb0b92dbd8fe9c4f636d2ca79b25cde331f
Summary:
While working on timestamp storage, the fact that
InodeTimestamps was a member of InodeBase kept getting in the way.
Make it its own type.
Reviewed By: simpkins
Differential Revision: D6862835
fbshipit-source-id: 91d8984764f0586b9fa52e961eb5606a530e0416
Summary: This API seems like it should be const, as it does not modify the clock.
Reviewed By: chadaustin, zhupanov
Differential Revision: D6869719
fbshipit-source-id: c8bf4ccab34538b59e6baeedd0b0ff88b328236e
Summary:
A little bit of plumbing to enable the watchman<->eden integration tests on sandcastle.
I noticed that we were still referencing the lame thrift par that I removed a little while back,
so let's clean that up.
Reviewed By: simpkins
Differential Revision: D6863221
fbshipit-source-id: 026f3d316d9ac52fe359cedc4e3c88394ec5c7c4
Summary:
This diff moves the mount-time initialization handling
out of the main loop. This rationale for this is:
* We don't (and shouldn't!) need to process FUSE_INIT for takeover
processing, and this structure allows us to make stronger assertions
about our state.
* we can avoid spinning up multiple threads in the (rare!) case that
the FUSE_INIT fails
* It is now a little harder for exceptions during initialization to
escape our notice.
In rearranging this stuff, I found a race condition in the worker thread
shutdown; we could erroneously emit a completion event before all of
the threads had been torn down and this resulted in sporadic integration
test failures hitting the assertion for the number of joined threads
in the destructor.
Reviewed By: simpkins
Differential Revision: D6766330
fbshipit-source-id: 32afb5a7c739c75aebfdb0a8f896eec5f41ad33f
Summary:
If we fail to lstat a mount because the FUSE daemon is
unconnected, go ahead and unmount it.
Reviewed By: simpkins
Differential Revision: D6848115
fbshipit-source-id: df797b15956db24c4ac87dc6d4cd0a7177dd20ef
Summary:
This is a fairly minimal attempt at asking sandcastle to
run our tests as root. This is inspired by D6608746 but is for
the moment pretty coarse; I just want to see how successful this
is before finishing this off nicely, which means splitting our
contbuild config into two pieces along similar lines as D6608746;
one piece would run the tests as usual without root privileges
whilst the other would run just the integration tests with root
privs.
Reviewed By: chadaustin
Differential Revision: D6612669
fbshipit-source-id: 9a730b02424e939dd3509310b6265d50d1102e77
Summary: Rather than EPERM we get EACCESS when running as root
Reviewed By: chadaustin
Differential Revision: D6853303
fbshipit-source-id: c6743c8d036a24255d2d31c560034c972a0253c7
Summary:
in our sandcastle environment, eden is not currently
installed and the tests that look at the version are throwing
an error when the rpm query fails.
Don't do that!
Reviewed By: simpkins
Differential Revision: D6853305
fbshipit-source-id: d17cb1fb0253ef5ae5000695b6ec5bcda4a6a448
Summary:
This is the spiritual successor to D3302706 which originally
wanted to solve this by adding a python extension. That would prove
to be too painful for the opensource build so it was shelved.
We now need to be able to run our tests in an environment that doesn't
have the `attr` rpm installed so this is a good time to fix this
in a more portable way.
This diff adds a little wrapper around the functions that we already
have for consuming extended attribute information and augments them
with another to list attributes.
The utility emits output in json format and is intended to be fed
directly into the helper functions we have in `fs.py`.
Reviewed By: chadaustin
Differential Revision: D6851182
fbshipit-source-id: 3d1d1a351f2e01405645d45658d1c8bc61a659a4
Summary:
Dir's contents were represented as a vector of 64-bit
pointers to 48-byte structs. This change removes that layer of
indirection, reducing memory usage and slightly pessimizing insertion.
The diff is mostly mechanical outside of the TreeInode.h changes and
calls to emplace..
I'll run memory tests tomorrow, though it's a gamble as to whether
private bytes will show a difference. I may need to shrink the Entry
struct too.
Reviewed By: wez
Differential Revision: D6804957
fbshipit-source-id: b126656dbc7951565e74b6401adde6353e809056
Summary:
This adds version 3 nee 2 of the takeover serialization,
which uses thrift to represent the data. Even though it is logically
version 2 I'm naming it version 3 because we're taking advantage of
the MessageType values from Version 1 which allowed numerical values
1 or 2 to represent different data types.
In this diff we now formalize that first word as the protocol version
and to avoid ambiguity are starting the new version value at 3. I
did briefly consider using the name Version2 to refer to this and setting
the value in the enum to 3, but I didn't want to become known for
API hate crimes against my fellow engineers.
Reviewed By: simpkins
Differential Revision: D6733406
fbshipit-source-id: e2067365e4e8b388490440fd73ab504544011846
Summary:
Whilst chatting with simpkins we realized that we lost
the handshake portion of the takeover protocol during a refactor.
The handshake is important for a couple of reasons:
1. It prevents unmounting and loosing all the mounts in the case
that sometime decides to netcat or otherwise connect to the
socket
2. It gives us an opportunity to short circuit any heavy lifting
if we know that it will be impossible to succeed.
3. It allows us to rollback to earlier builds with older versions
of the takeover protocol.
This diff adds a little bit of machinery to enable passing a set of supported
takeover protocol version numbers. The intent is to retain support for
the two of these at a time; any time we change the encoding/protocol
for takeover we'll bump the version number and add supporting code
to handle the new format, retaining support for the prior version.
Retaining the ability to handle the prior version allows us to downgrade
to an earlier build gracefully if/when the need arises.
I opted to do this here rather than by bumping the `kProtocolID`
constant in `UnixSocket.h` becase we're not really changing the
lowest level of the protocol; just the takeover specific portions.
I haven't actually changed the takeover serialization in this diff,
but do have some work on that happening in D6733406; that diff will
be amended to take advantage and demonstrate how this versioning
scheme works.
A key thing to note about the implementation of this diff is that
the client sends the version number to the server, but doesn't
add any explicit version encoding in the response we receive.
This is deliberate and allows us to upgrade prior builds to
this new scheme. I'll add a more definitive check for this
situation when I actually rev the format in the following diff.
Reviewed By: simpkins
Differential Revision: D6743065
fbshipit-source-id: c991cebfee918daad098105ca6bcfef76374c0ff
Summary:
Tiny thing I noticed when reading code. Keep the entry name
as a StringPiece rather than bouncing through char*.
Reviewed By: zhupanov
Differential Revision: D6820080
fbshipit-source-id: 884e55f74094f44012efbe44b86d8e5903300967
Summary:
isSameAs calls getSha1 which was failing on symlinks. The
original concern was that asking for the SHA-1 of a symlink is
ambiguous: do you want the hash of the symlink or the target? But we
already check for whether you are requesting the SHA-1 of a symlink in
EdenServiceHandler, so it's redundant and incorrect to check in
FileInode too.
Reviewed By: simpkins
Differential Revision: D6847489
fbshipit-source-id: 13966da06bcde75c5c568e09fef14e735de47cfb
Summary: A minor convenience for mass mounting and unmounting.
Reviewed By: simpkins
Differential Revision: D6803003
fbshipit-source-id: 16c0d6982ba0ce2dba9900ee15013fcec8bc5ad5
Summary:
This adds support for the following flags for `hg status` in the Rust wrapper:
* `--all`
* `--modified`
* `--added`
* `--removed`
* `--deleted`
* `--clean`
* `--unknown`
* `--ignored`
* `--no-status`
* `--copies`
There is now a `create_print_config()` function that takes the parsed arguments
and distills them into a `PrintConfig`, which describes the net configuration
as a result of all the flags. (Note that some flags can interfere with each other,
e.g., `--no-status` forces `--copies` to be disabled.)
While here, I cleaned up a bunch of duplication in the code that prints the
status to stdout. Much of the duplication in the old implementation has been
encapsulated in a closure within the new `print_status()` function.
This also made it easier to implement `--copies` for all of the different status
types.
Incidentally, this helps us specify `listIgnored` correctly when calling the
`getScmStatus()` Thrift method as we now keep track of whether the user
specified `--ignored`. This is particularly important for the Eden integration
test suite because its `assert_status()` helper method calls
`hg status --print0 -mardui` by default, so `buck test eden/...` makes much
greater use of the Rust wrapper than it did before.
Reviewed By: wez
Differential Revision: D6770916
fbshipit-source-id: fea1a159addca6d16caa1b7019dc11dc2dcb3e1b
Summary:
In my benchmarks, using fbstring over std::string for PathMap is
worthwhile. There are a lot of entries stored in PathMap and the fact
that fbstring is a word smaller than std::string adds up.
Differential Revision: D6820204
fbshipit-source-id: 1ac34a81828891eb8125b13a074de6ed9715edad
Summary:
Added comments linking to the task with discussion of why we are postponing
proper handling of umask and permissions in general for mkdir and mknod.
Reviewed By: wez
Differential Revision: D6808696
fbshipit-source-id: a6d74ea08448c8578282dc146ac81da187d4409c
Summary:
My stale mounts check regressed eden doctor's handling of
unconfigured mounts. For now, just skip them, and add a test that
eden doctor doesn't crash.
Reviewed By: simpkins
Differential Revision: D6801445
fbshipit-source-id: 65d2f9028af651ef487a7ce6c334f387b541492d
Summary:
The prior implementation of StaleMountsCheck filtered by path
and did not correctly handle seeing the same FUSE mount multiple times
in the mount table. This occurred when an Eden mount was created
underneath a bind mount.
Now it only unmounts mounts where st_dev does not match the st_dev of
any active mounts, and where st_uid matches the current user.
Reviewed By: simpkins
Differential Revision: D6787618
fbshipit-source-id: 24e0f156cb74822500d91205349c0e6638c0340c
Summary:
Now that we've moved the hg source around, update eden
to point to the source of truth rather than the synced subdir.
I spoke with andrewjcg about the use of header_namespace here;
it is required to be able to reference the headers without changing
the code under scm/hg to be relative to the fbcode root. We can't
change that code in that way because it has to build standalone
for the opensource consumers.
Remove the duplicated code while we're in here to make sure
that we're not accidentally referencing it anywhere.
Reviewed By: simpkins
Differential Revision: D6792969
fbshipit-source-id: ec55e98c0bde2488c213b1fd9f8baae9868a350c
Summary:
It's interesting to see the total number of loaded files
vs. trees when the loaded inode count is high.
Reviewed By: wez
Differential Revision: D6765874
fbshipit-source-id: 178b30184428bd5cf5e005eb475e4f5a1476c385
Summary:
I'm not sure what was wrong with the old code, but I
simplified and clarified all of the time math and now `eden debug
unload` behaves as I'd expect it should.
Reviewed By: simpkins
Differential Revision: D6764962
fbshipit-source-id: 3ed359d4ab4652e95d1538a0982c24185999351c
Summary:
rdev doesn't add any value yet. We can add it back if we want
to implement support.
Reviewed By: simpkins
Differential Revision: D6792346
fbshipit-source-id: ce16317074f1daa456737c55804da8fb7f2b7a94
