Summary:
This puts the data into the takeover information during takeover
shutdown, but doesn't do anything to pull it out again (that will be in a follow on diff).
The serialization stuff could be done a little bit more efficiently (since we
will perform an extra thrift serialization step just to compute the length, and
repeat it again later), but we're planning on replacing this with thrift
serializing soon, once simpkins diff stack lands, so I'm not losing sleep over
it.
Reviewed By: simpkins
Differential Revision: D6668846
fbshipit-source-id: e6d01428bd506a9e93b427db499770fce0a0983a
Summary:
This fulfils a TODO but doesn't do anything useful
with that data at this time.
Reviewed By: simpkins
Differential Revision: D6552750
fbshipit-source-id: 0c441fd0c2ab43785b4d98c4ca6ff643a20629e0
Summary:
This adds some plumbing to thread the fuse device descriptor and
negotiated capabilities through to the takeover code.
I initially wanted to just make the
unmount future yield the device descriptor, but since that uses
`SharedPromise` it is not compatible with a move-only type like
`folly:File`, so I added an optional promise to deal with just that.
I'm also populating the takeover mount information (path, bind mounts)
for each mount point.
Reviewed By: simpkins
Differential Revision: D6494509
fbshipit-source-id: a90684292dc1d8e06ce2c0721eadd8d393377f33
Summary:
The unix `patch` command attempts to issue an `FS_IOC_GETFLAGS`
ioctl and blows up when we return `ENOSYS`. The man page for ioctl says:
```
ENOTTY The specified request does not apply to the kind of object that
the file descriptor fd references.
```
so let's return that error code to ioctl.
In addition, the integration test I added for this trips up when it calls
`llistxattr` on the file; turns out we don't need to insert the
`fuse_getxattr_out` when we're returning the attribute list and that it
is only needed when measuring up the required length, so let's move
things around to resolve this and make it clearer.
Reviewed By: chadaustin
Differential Revision: D6685568
fbshipit-source-id: 81963ffe9af30db5634e5e96b7a8aa1485859d65
Summary: D6161089 removes autoconf support in fbthrift. So let's switch to cmake.
Reviewed By: wez
Differential Revision: D6675252
fbshipit-source-id: 5d04d3e285f80aec4ce3b566c5046c6f2c3d76b1
Summary: A prior shelve contained this fix but never got landed.
Reviewed By: wez
Differential Revision: D6676206
fbshipit-source-id: b8c733be663ff56e1a0625f09ec505891d430084
Summary:
this comment is not accurate since removing libfuse.
The good news is that doing that meant that I didn't need to
touch DirList when removing libfuse.
Reviewed By: chadaustin
Differential Revision: D6672533
fbshipit-source-id: 54216685b03b8f6dd7ee96b7bc38bb91d1b33366
Summary:
We'll need a way to twiddle the refcount by more than
increment at a time for graceful restart. This mirrors the
same pattern we use to forget() by an arbitrary number.
Reviewed By: chadaustin
Differential Revision: D6668809
fbshipit-source-id: 6e5dc33b5e40f98f01293c89152bfe1e0879f572
Summary: We forgot to remove this when we moved to be fully inode based in the overlay
Reviewed By: chadaustin
Differential Revision: D6668810
fbshipit-source-id: b79af85a4bbbcefd9227ad69bb8d57b5274cdaed
Summary:
Switch from glog to folly logging. Also derive from folly::ColdClass to
indicate that the `EdenBug` class is only ever created on unexpected code
paths.
Reviewed By: wez
Differential Revision: D6609613
fbshipit-source-id: 33621c7bafc946df245224da9b6a559e3dfd58d6
Summary:
Update the integration tests to understand the new `eden list` output format
changes introduced in D6661303.
Reviewed By: chadaustin
Differential Revision: D6665272
fbshipit-source-id: 13f65a7a66f997e6b8135436811fe3fc396eb38f
Summary:
Update the TakeoverClient and TakeoverServer code to use the new UnixSocket
helper class for exchanging messages, file descriptors, and credential
information.
This does not change the message serialization code much yet, it merely changes
the code to use the UnixSocket class for I/O.
Reviewed By: wez
Differential Revision: D6494979
fbshipit-source-id: 3129fe8605b1b3b7a24e6e84e94dccf3ea2b4170
Summary:
Add new classes that help send data, file descriptors, and credential
information over unix domain sockets.
UnixSocket provides a low-level, raw callback API, similar to that provided by
the classes in folly/io/async/. UnixSocketFuture is a slightly higher-level
wrapper class that provides a Future-based API on top of this. I expect that
most places in eden will probably use the UnixSocketFuture API, but callers
that repeatedly wait for new messages (like the privhelper server) may want to
use the raw callback API instead.
This will help simplify several places in eden that communicate over unix
domain sockets. Both the privhelper code and the takeover code needs to send
file descriptors over unix domain sockets. They currently each use their own
separate message handling logic for this. We currently communicate with the
hg_import_helper.py script over a pipe, but this could easily be switched to
use this new UnixSocket class as well.
Reviewed By: wez
Differential Revision: D6494981
fbshipit-source-id: 80bd7f06e5b884fc4148162e1a8a3b478acce209
Summary:
Title says it all. Uses a switch statement so the compiler
might warn if a header's opcode isn't handled.
Reviewed By: wez
Differential Revision: D6664378
fbshipit-source-id: 51986c423c943bf3b51cbd342ea4bdedb5fe1188
Summary:
eden doesn't support ioctl which was spamming "unhandled fuse
opcode 39" into the log. Only log each unhandled opcode once.
Reviewed By: wez
Differential Revision: D6663175
fbshipit-source-id: c1bea671c8fb720ae0ac234c4f187251930a8bee
Summary:
`eden list` enumerates all of the mounts in the Eden config,
whether or not the daemon is running and whether or not the mount is
currently active or not. This diff adds an (active) suffix on a mount
if the daemon is running and the mount is currently active.
Reviewed By: simpkins
Differential Revision: D6661303
fbshipit-source-id: c098e90fc9a77f16c723c707cc4da3ee3d4c5abb
Summary:
It is no longer correct to assert that state->file is set if O_TRUNC happened
before blob import from hg finished. It surprises me we never saw a crash
because of that. Also, the O_TRUNC path after blob import finishes can never
complete a future, so don't try.
Reviewed By: wez
Differential Revision: D6656699
fbshipit-source-id: 5e245fc46185714e5f5d81c2680835a3497747ff
Summary:
Today, if a file is ever opened for read, each FileInode keeps a copy
of the data as long as the FileInode is around. This results in
excessive memory consumption under common mistakes like repo-wide grep
or `hg revert .`.
I will audit all of the state machine transitions and blob accesses
before landing this diff.
Reviewed By: wez
Differential Revision: D6598957
fbshipit-source-id: 1eb4aeb08057ce993a29a86d298e153675fee4a1
Summary:
This came up when I was auditing the rules about when it's
safe to read from a FileInode. read() must only be called while openCount > 0.
Reviewed By: wez
Differential Revision: D6604898
fbshipit-source-id: 829ddc335bd58201c2b456ee544cdc6253ebf66c
Summary:
In a follow-on diff, the constraint will be that state->blob
will only be guaranteed valid after ensureDataLoaded() while a
FileHandle is alive. Thus, ensureDataLoaded() must return a
FileHandle.
Reviewed By: wez
Differential Revision: D6586237
fbshipit-source-id: ccc269d322b8c725c93145df5de2add9a2b90207
Summary: Drive-by cleanup. `CompareString` is not used anywhere and `std::less<>` would be better anyway.
Reviewed By: wez
Differential Revision: D6655104
fbshipit-source-id: 0fee22172c93335493400e6317d92f9e1e77b40e
Summary: This is annoying because it isn't actually shadowing and because this isn't picked up in our default mode mode, or in sandcastle.
Reviewed By: chadaustin
Differential Revision: D6656548
fbshipit-source-id: e624cb563d6396c1ab6b93eae14651c16a8c0cd3
Summary:
Now that we have full control over the fuse session,
moving the threads here makes a lot of sense and makes things
easier from an overall state management perspective.
FuseChannel now provides a future that make it easier for EdenMount to wait
until all the threads and outstanding requests have completed.
There is also a future to determine when the first of the threads
has exited which is used to detect an error condition. We could
go a bit further with this and have the error condition propagate
out from our dispatcher, but that's a bit further away from my
goal of making the graceful restart stuff work.
I've removed the request counting stuff from Dispatcher as we
can now use the definitive signals from FuseChannel in its place.
Reviewed By: chadaustin
Differential Revision: D6580247
fbshipit-source-id: 6bcc3b8b531d59a3fdd0ca6fd09410ad64f8221a
Summary:
I'm mostly interested in this as the definitive way to know
how many outstanding fuse requests there are, which is important
for synchronization for graceful restart.
An important part of this diff is to use of the `RequestContextScopeGuard`.
Previously, `RequestData::create` would implicitly create a new
`folly::RequestContext` and associate it with the current thread each
time a new request was processed on the fuse worker thread. Critically,
it would never take any action to remove that association. What that
meant was that during shutdown some number of fuse worker threads were
extending the lifetime of some fuse requests and that leads either to
an use-after-free (as is the case in this diff stack up until this diff),
or with the stronger synchronization that we desire in graceful restarts,
a deadlock.
Reviewed By: chadaustin
Differential Revision: D6578933
fbshipit-source-id: f82f75e75a398d1f6beacb9466060e7bd99adbc1
Summary:
Use a map to lookup the handler for a given opcode.
While we're in here, let's reduce some more boiler plate by factoring
out the code that preps the request and associates it with the stats
histogram by moving that into the opcode map.
Reviewed By: chadaustin
Differential Revision: D6578934
fbshipit-source-id: 9e3f73beb9dc5597f095e81ef21adf1549420e9d
Summary:
This serves a few purposes:
1. We can avoid some conditional code inside eden if we know that
we have a specific fuse_kernel.h header implementation.
2. We don't have to figure out a way to propagate the kernel
capabilities through the graceful restart process.
3. libfuse3 removed the channel/session hooks that we've been
using thus far to interject ourselves for mounting and
graceful restarting, so we were already effectively the
walking dead here.
4. We're now able to take advtange of the latest aspects of
the fuse kernel interface without being tied to the implementation
of libfuse2 or libfuse3. We're interested in the readdirplus
functionality and will look at enabling that in a future diff.
This may make some things slightly harder for the more immediate
macOS port but I belive that we're in a much better place overall.
This diff is relatively mechanical and sadly is (unavoidably) large.
The main aspects of this diff are:
1. The `fuse_ino_t` type was provided by libfuse so we needed to
replace it with our own definition. This has decent penetration
throughout the codebase.
2. The confusing `fuse_file_info` type that was multi-purpose and
had fields that were sometimes *in* parameters and sometimes *out*
parameters has been removed and replaced with a simpler *flags*
parameter that corresponds to the `open(2)` flags parameter.
The *out* portions are subsumed by existing file handle metadata
methods.
3. The fuse parameters returned from variations of the `LOOKUP` opcode
now return the fuse kernel type for this directly. I suspect
that we may need to introduce a compatibility type when we revisit
the macOS port, but this at least makes this diff slightly simpler.
You'll notice that some field and symbol name prefixes vary as
a result of this.
4. Similarly for `setattr`, libfuse separated the kernel data into
two parameters that were a little awkward to use; we're now just
passing the kernel data through and this, IMO, makes the interface
slightly more understandable.
5. The bulk of the code from `Dispatcher.cpp` that shimmed the
libfuse callbacks into the C++ virtual methods has been removed
and replaced by a `switch` statement based dispatcher in
`FuseChannel`. I'm not married to this being `switch` based
and may revise this to be driven by an `unordered_map` of
opcode -> dispatcher method defined in `FuseChannel`. Regardless,
`Dispatcher.cpp` is now much slimmer and should be easier to
replace by rolling it together into `EdenDispatcher.cpp` in
the future should we desire to do so.
6. This diff disables dispatching `poll` and `ioctl` calls. We
didn't make use of them and their interfaces are a bit fiddly.
7. `INTERRUPT` is also disabled here. I will re-enable it in
a follow-up diff where I can also revise how we track outstanding
requests for graceful shutdown.
8. I've imported `fuse_kernel.h` from libfuse. This is included
under the permissive 2-clause BSD license that it allows for
exactly this integration purpose.
Reviewed By: simpkins
Differential Revision: D6576472
fbshipit-source-id: 7cb088af5e06fe27bf22a1bed295c18c17d8006c
Summary: eden rage crashes sometimes due to too much inode info in the info dump. Replacing per-inode info dump with the stats summary from the stats module.
Reviewed By: chadaustin
Differential Revision: D6641312
fbshipit-source-id: 16910aa21306db4e5533217bd2ffb7b37440a807
Summary: This was made possible by D6605465.
Reviewed By: philipjameson
Differential Revision: D6628867
fbshipit-source-id: 081cdbc3e2e3406e5b2cad07021b77009feaebc5
Summary:
We're unconditionally emitting a `--` above, so the `--`
down here doesn't get stripped out by the logic in `do_daemon`
in the CLI and that renders it invisible to gflags:
Here's our `eden cli` invocation:
```
['/data/users/wez/eden-fbsource/fbcode/buck-out/gen/eden/cli/cli.par', '--config-dir', '/var/tmp/eden_test.15oico01/homedir/local/.eden', '--etc-eden-dir', '/var/tmp/eden_test.15oico01/etc-ed
en', '--home-dir', '/var/tmp/eden_test.15oico01/homedir', 'daemon', '--daemon-binary', '/data/users/wez/eden-fbsource/fbcode/buck-out/gen/eden/fs/service/edenfs', '--foreground', '--', '--num
_hg_import_threads', '2', '--', '--logging=eden.fs.store.hg=DBG9,eden.strace=DBG7']
```
This is what `eden cli` passes to `edenfs`:
```
edenfs_args ['--num_hg_import_threads', '2', '--', '--logging=eden.fs.store.hg=DBG9,eden.strace=DBG7']
```
Reviewed By: bolinfest
Differential Revision: D6628253
fbshipit-source-id: 2c6806e69baff52d14ca64194f1bf7d916833844
Summary:
Easy to overlook this; the issue is that we need to explicitly
do something about the error case when we're stitching together Promises
by hand, otherwise we will silently drop exceptions.
Flat manifest imports are failing in `RestartTestHg` in master
at the moment. That error was silently being swallowed and the test would
hang until it timed out.
This is an uglyish hack to explicitly propagate the error condition so that
that test will error out.
This diff doesn't fix the source of the manifest import issue; that is addressed
in the next diff (turned out to be that the `--takeover` flag was not being
passed correctly)
Reviewed By: bolinfest
Differential Revision: D6627973
fbshipit-source-id: b7093890f543618a11682e939f8802f1309831d4
Summary:
My recent stack of diffs introduced a handful of oss bugs :(.
1. VLOG_EVERY_MS is an internal function.
fix: added an oss stub version
2. Forgot to add QsfpCache.cpp to CMakeLists.txt
fix: add it
3. vanilla gcc5.4 has a bug where it can't handle lambdas like:
[this]() {
memberFn();
}
and instead you need to write like:
[this]() {
this->memberFn();
}
fix: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67274 says it should
be fixed in gcc7, but I added 'this->' in the meantime.
4. The base fb303.thrift file we have in open source does not declare
aliveSince, which we now call on qsfp_service.
fix: add it to fb303.thrift
Reviewed By: ninas
Differential Revision: D6627705
fbshipit-source-id: 2100783df1ea0e9af0fed66e4e24ef85b71fc7e9
Summary:
This is a codemod to change from using @/ to // in basic cases.
- TARGETS files with lines starting with @/ (but excluding @/third-party:
- autodeps lines in source and TARGETS files ( (dep|manual)=@/ ), excluding @/third-party
- Targets in string macros
The only thing left of the old format should be @/third-party:foo:bar
drop-conflicts
Reviewed By: ttsugriy
Differential Revision: D6605560
fbshipit-source-id: 17d3a196b91045f0db5ee2a5afad467b6344be0b
Summary:
This is a codemod to change from using @/ to // in basic cases.
- TARGETS files with lines starting with @/ (but excluding @/third-party:
- autodeps lines in source and TARGETS files ( (dep|manual)=@/ ), excluding @/third-party
- Targets in string macros
The only thing left of the old format should be @/third-party:foo:bar
drop-conflicts
Reviewed By: ttsugriy
Differential Revision: D6605465
fbshipit-source-id: ae50de2e1edb3f97c0b839d4021f38d77b7ab64c
Summary: Previously, this would cause a crash when running `eden doctor`. Now fixed.
Reviewed By: simpkins
Differential Revision: D6607211
fbshipit-source-id: a0e077beaf6b7031d57efd72b947e90884369852
Summary:
Encountered a funny situation where running `hg clone src dest` from inside an
Eden mount where `src` was a directory that contained a non-Eden Hg repo would
fail with a stacktrace that ended with:
```
File "/usr/local/fb-mercurial/eden/hgext3rd/eden/__init__.py", line 195, in merge_update
conflicts = repo.dirstate.eden_client.checkout(
AttributeError: 'dirstate' object has no attribute 'eden_client'
```
This was very confusing because we had this check at the top of the function:
```
if not util.safehasattr(repo.dirstate, 'eden_client'):
why_not_eden = 'This is not an eden repository.'
```
So it seemed that we already verified that `repo.dirstate.eden_client` must be a
valid attribute. However, we followed this check with a new set of checks, the
final one being:
```
else:
why_not_eden = None
```
This one had the unintended effect of resetting the value of `why_not_eden` that
we set in the first `if`. Changing the `if` to an `elif` introduces the proper
decision tree.
Reviewed By: simpkins
Differential Revision: D6608867
fbshipit-source-id: 320e69925737135d84f9d6a46a7fb43437cc37e0
Summary:
Update the logic for dry-run checkouts to exit early whenever we hit a
directory that is unmodified from the original source control state. In this
case we can skip this entire subdirectory since we know there cannot be any
conflicts.
This should make dry-run checkout operations much faster in cases where there
are lots of changes between the source and destination source control trees,
but the working directory state has relatively few modified files.
Reviewed By: bolinfest
Differential Revision: D6605282
fbshipit-source-id: b5423749f3d47b10ed8d599ffaa0667c72fbaec2
Summary:
In making this change, I was hoping to remove `.encode()` calls, but the net
number is zero.
Reviewed By: chadaustin
Differential Revision: D6606997
fbshipit-source-id: ea1851d719aff0bae424f15c94a7cc48b8f516f0
Summary:
This additional debugging info should help when investigating user issues.
While here, I also fixed things up so that `eden rage` does not crash if Eden
is not running (it just prints less information and tells the user).
Reviewed By: chadaustin
Differential Revision: D6599309
fbshipit-source-id: cf5c63dcc6f4dbf224122579c3bd458730629a66
Summary:
Previously the checkout code always called `JournalDiffCallback::performDiff()`
to get the list of unclean files before starting the checkout operation. After
the checkout completed it called `JournalDiffCallback::performDiff()` to get
the unclean files after the checkout operation.
Making both diff calls should be unnecessary: the checkout code can only modify
files that were unclean before the checkout (in the case of a `FORCEe` checkout)
or files that were modified between the source and destination source control
trees. Therefore the `performDiff()` call after checkout completes should be
unnecessary, and can be removed.
This diff also eliminates the initial `performDiff()` call for `DRY_RUN`
checkouts. We do not add a journal entry at all for `DRY_RUN` operations, so
we can skip this diff computation.
Reviewed By: wez
Differential Revision: D6455969
fbshipit-source-id: 20e0ac0d16d1fde844c1d6165a96611bfb370597
Summary:
I almost added a copy in a later diff and realized it would
have let me, but done the wrong thing.
Reviewed By: wez
Differential Revision: D6585810
fbshipit-source-id: 15295d04b06df397113be6080e1f2f6b8a473745
