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:
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:
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
Summary:
this is a backport of the same fix from D6578933 and helps
to avoid noise with our new default ASAN enabled build settings.
The root of the problem is that we didn't guarantee to clear the
association between the fuse worker thread and a folly request
associated with a fuse request. This meant that the lifetime
of the RequestData was being extended until after the EdenMount
was destroyed and then the destructor of RequestData would trigger
an access inside the EdenMount instance.
Adopting the `RequestContextScopeGuard` type helps to avoid this,
at the cost of perhaps prematurely heap allocating a request context
for some opcodes. D6578933 can avoid that cost.
Reviewed By: chadaustin
Differential Revision: D6602416
fbshipit-source-id: 80785b860af906c0fcf02b42f0235796377c265f
Summary:
Noticed this bug while working on our own fuse lowlevel stuff.
Taking the min here means that we always tell the kernel to use 4k writes
instead of the larger buffer that we allocate here.
This is also addressed in D6576472
Reviewed By: chadaustin
Differential Revision: D6602417
fbshipit-source-id: d2deaaad7a727433ab021d042e372d06b2acb798
Summary:
My devvm was running out of memory when running `buck test eden/...`.
Because Buck farms out tests across cores and each Eden integration
test launches 8 hg import daemons, there were over a hundred importer
processes live on the system.
Reviewed By: bolinfest, wez
Differential Revision: D6598276
fbshipit-source-id: d26916af79c24aa73abfa4c3ef9be3178657b6e7
Summary:
`eden stats` used to show system memory usage which was not very
interesting (and can be gleaned from top). Instead read the contents
of /proc/self/smaps and sum the Private_Dirty fields to get a number
that more accurately reflects impact on the rest of the system.
Reviewed By: wez
Differential Revision: D6575595
fbshipit-source-id: 9badc5cd5a1b56d3ccb27edd1a2d20ee74ec34ae
Summary:
When I updated D6577031 in response to code review feedback,
I manually tested with `--qf` but at the last minute changed to the
suggested long-form `--queryfmt` without verifying it was the
appropriate name.
Reviewed By: simpkins
Differential Revision: D6579002
fbshipit-source-id: ac2a96e7340099eb83336cb11d16dd711b0d4f9f
Summary:
I forgot that `subprocess.check_output()` returns a `bytes` instead of a `str`.
This caused `eden doctor` to return false positives because the installed and
running versions of `edenfs` would be the same, but the check would fail because
it was comparing a `bytes` to a `str`.
Reviewed By: simpkins
Differential Revision: D6577031
fbshipit-source-id: 681ea22ef79604a3dfb278d713e5c68c54d8ecd5
Summary:
This reports the difference between what is returned by the `aliveSince()`
Thrift method and the current time.
Reviewed By: chadaustin
Differential Revision: D6566127
fbshipit-source-id: 449103bcb31a87e4efd780299131eb38a45a6bd7
Summary: Added type identification capability to InodeBase and its descendants FileInode and TreeInode.
Reviewed By: simpkins
Differential Revision: D6564902
fbshipit-source-id: ce9300102d6d6d1c42616eb1e32042f21f6e6cce
Summary:
`eden debug buildinfo` lists the exported values from the Thrift daemon that
start with `build_`.
Reviewed By: simpkins
Differential Revision: D6565668
fbshipit-source-id: 62009d7a23211765039a5045a517113043b5d8a9
Summary:
During edenfs startup, use `realpath()` or `normalizeBestEffort()` to resolve
symlinks in the input configuration paths if possible.
Reviewed By: chadaustin
Differential Revision: D6527494
fbshipit-source-id: 4377099e8c65217fd128c06de77d50f4316f4fc7
Summary:
The RocksDB location is relative to the .eden directory. Given that the
location of the .eden directory can be controlled from the command line it
doesn't seem worthwhile to make the RocksDB location independently controllable
too.
Reviewed By: chadaustin
Differential Revision: D6527495
fbshipit-source-id: dab8c22f3f1a74de908ea33d0b20c4115c28ce31
Summary:
Add a new function that attempts to normalize a path with `realpath()`, but
falls back to `canonicalPath()` if that fails. This lets us attempt to resolve
symlinks if possible, but still perform best-effort normalization if that
fails.
Reviewed By: chadaustin
Differential Revision: D6527493
fbshipit-source-id: 9137d517452ca7fd825852c1f60ade07f1ee78fa
Summary:
This attempts to repro an internal bug report, though note the integration test
passes without any changes to Eden. It is possible that the person who reported
the bug was stuck on an old version of the Eden daemon while using a newer
version of the Mercurial extension, which could account for the unexpected
behavior.
Reviewed By: simpkins
Differential Revision: D6536375
fbshipit-source-id: 1bc4c50ee5f616502dc06f8ed0167817c566e179
Summary:
For new folks and myself, here's a bit of prose that describes our
threading strategy.
Reviewed By: simpkins
Differential Revision: D6513572
fbshipit-source-id: a48e0152692aa63540f0be27f943fd6f29bb5fb2
Summary:
Add EdenCPUThreadPool and UnboundedQueueThreadPool types to make it clearer
that it's always okay for prefetch, deferred diff entry, and hg import to
shuttle work back to the main thread pool.
This diff changes no behavior - it just makes some invariants explicit.
Reviewed By: wez, simpkins
Differential Revision: D6504117
fbshipit-source-id: 3400ad55c00b3719ecba31807fd992442f622cd9
Summary:
I am seeing occasional timeouts on my smaller devvm when
running `buck test eden/integration/...`.
Reviewed By: bolinfest
Differential Revision: D6541864
fbshipit-source-id: 401deb8b44adae8cc362bbba8b638fe08abb9b1e
Summary: The same as < 500 references, except this time for targets with < 1600 references.
Reviewed By: yfeldblum
Differential Revision: D6540895
fbshipit-source-id: 40fa46c32abd6bc1c3c652a0396d6478b947f69b
Summary: Ensure everything remaining in dependencies of `folly:folly` that has < 500 references is explicit referenced.
Reviewed By: yfeldblum
Differential Revision: D6540137
fbshipit-source-id: 0a2ae5cf775278eedcccdb914688890acd12dab7
Summary: And then, there was 1, left all alone, preventing the whole thing from collapsing in on itself.
Reviewed By: yfeldblum
Differential Revision: D6469584
fbshipit-source-id: 4ea1fbf97ad466bc34f2e682394d328c97e539ba
Summary:
Added to Eden capability to incorporate default user and general system level gitignore files.
NOTE: Work in progress, sending the review out to calibrate/ensure I am on right track.
Reviewed By: simpkins
Differential Revision: D6482863
fbshipit-source-id: 9834ca1a577a9599a1f8cb2243dca4e714866be8
Summary:
Use an unbounded queue for edenfs's main thread pool. This fixes a
crash where DeferredDiffEntry multigets a batch of trees and pushes
the completion callbacks back onto the server thread pool. If the
server thread pool is bounded and throws when the queue is full, then
the import fails.
There is a slight performance hit relative to LifoSemMPMCQueue but
hopefully it isn't a big deal. An unbounded lock-free queue would be
nicer.
Reviewed By: simpkins
Differential Revision: D6490979
fbshipit-source-id: bc55dd6526f0ceb9d8b5e43a1a275250a9838aca
Summary:
There's no technical reason to block an open() request until the data
load / materialization returns. This change returns immediately from
open() and then waits if necessary in a subsequent write() call.
Reviewed By: wez
Differential Revision: D6391486
fbshipit-source-id: 862f87e3c3a0d760bacb0f8ca7acc479037fec2f
Summary:
Follow-up to comments in D6466209. All access to the clock goes
through the Clock interface, making time deterministic in unit tests.
Reviewed By: simpkins
Differential Revision: D6477973
fbshipit-source-id: 24e51bdb52d0d079b34d91598d2e787d361f2525
Summary: Follow-up from D6366189. First use of the new FakeClock!
Reviewed By: simpkins
Differential Revision: D6466209
fbshipit-source-id: 4d4d8a9a83df2bee11149e7a0cbddaaf734d0e04
Summary:
Introduce a Clock seam. This will allow us to write tests around
ctime, mtime, and atime logic.
Reviewed By: wez
Differential Revision: D6392543
fbshipit-source-id: 1721d76d2364b135b4ef5c078ef60f7f8526259e
Summary:
open() called materializeInParent unconditionally, and setattr never
called it, implying it was possible to truncate a file without
materializing the parent. This change makes sure to precisely call
materializeInParent whenever the state transitions to materialized.
Reviewed By: wez
Differential Revision: D6389794
fbshipit-source-id: 1e740e133a83d5090a6b9801154b7eaeccb07f22
Summary:
To make the materialization code paths a bit clearer, this decouples
materialization from a blob and truncation. It also caches opened
files if openCount > 0 in both the truncation and materialization from
blob paths.
Reviewed By: wez
Differential Revision: D6388318
fbshipit-source-id: c95a85f5bdaa405130f2f7260143592cdc90d45e
Summary:
Make the calls async by using a user-provided event base.
It uses quite scary error management, see the comments for details.
Reviewed By: wez
Differential Revision: D6435696
fbshipit-source-id: a148c57aa116a6dfe6254ac7c14790101b8ecd4f
Summary:
In the initial implementation of `eden doctor`, if you had a mount point like:
/data/users/mbolin/eden-fbsource
then it looked for a pair of Watchman subscriptions for
`/data/users/mbolin/eden-fbsource` with the names:
hg-repository-watchman-subscription-primary
filewatcher-/data/users/mbolin/eden-fbsource
as a heuristic that Nuclide was being used to edit files in that Eden mount.
However, if the user is using Nuclide to edit files in a subdirectory of that
Eden mount (as determined by the Nuclide connection profile), then the
subscriptions look like this:
hg-repository-watchman-subscription-primary
filewatcher-/data/users/mbolin/eden-fbsource/<path-to-subdirectory>
In this case, `eden doctor` was returning a false positive because it thought
that the `filewatcher-/data/users/mbolin/eden-fbsource` subscription was missing
when in fact everything was just fine.
This revision changes the logic to look for a subscription to the root or one of
its subdirectories.
(Note: this ignores all push blocking failures!)
Reviewed By: simpkins
Differential Revision: D6467113
fbshipit-source-id: bfe5fafaa388405f2e59e61315c5a2084c8adc56
