Summary:
While looking at the previous diff, I noticed that a significant amount of
memory was used by the loading promise. Since this loading promise is only used
when an inode is being loaded, its usage is the uncommon case: there should be
significantly more inodes being loaded or unloaded than loading at any given
time in EdenFS's lifetime.
Reviewed By: chadaustin
Differential Revision: D28477391
fbshipit-source-id: e80269506b980d7f217f6bdf59034ef003c4b3fc
Summary:
Similarly to the NFS change, this moves all the folly::Future handling in the
FuseChannel itself instead of inside the dispatcher. This should allow the
inode code to be converted to using ImmediateFuture instead of folly::Future.
Reviewed By: genevievehelsel
Differential Revision: D28372252
fbshipit-source-id: 7aae29d4a32500513c0062dfa42b2c7a3be038db
Summary:
Lambda returning no values would cause the compiler to try to instantiate an
ImmediateFuture<void>, which doesn't compile. We could try special casing the
void type, but it's easier if we simply consider these lambda to return unit.
Reviewed By: genevievehelsel
Differential Revision: D28372253
fbshipit-source-id: 1368ae5dc5e2d4d6a5c3e31bc87ed7d230027c3a
Summary:
The InodeMap can be extremely hot when issuing tons of requests to EdenFS.
Unfortunately, it still needs to do memory allocation due to its use of
folly::Future. By switching to ImmediateFuture we can avoid the memory
allocation, speeding it up slightly.
For the NFS dispatcher, this moves the call to `semi()` inward, allowing us to
target specific inode methods to convert next. For the Fuse dispatcher, I've
simply converted the ImmediateFuture into a Future directly, keeping the rest
of the code unchanged, a subsequent change will convert the dispatcher code to
ImmediateFuture.
Reviewed By: chadaustin
Differential Revision: D28302480
fbshipit-source-id: 4e097a721443f0d52f34a337a96f8a63a9a7cd7c
Summary:
This is to mimic the folly::Future API which allows the `get` and `getTry` to
throw if the Future isn't ready. One difference is that the ImmediateFuture API
has a default argument of a max duration instead of having a separate method.
Since chrono durations are expressed as intmax_t (64-bits on systems supported
by EdenFS) a max duration is virtually infinite.
Reviewed By: chadaustin
Differential Revision: D28424053
fbshipit-source-id: 319493174f31367184dbe0aa811a97145b0310cf
Summary:
DurhamG discovered that setting max_background in fuse_init_out allows
a larger number of concurrent FUSE requests. The documentation
indicates it's intended to affect background requests like readahead,
but empirically you can observe increased live FUSE requests with `rg
-j200` and `eden top`. Default to 1000 because EdenFS can handle a
large amount of concurrency and we want to avoid blob and tree fetches
to block FUSE IO when not necessary.
Reviewed By: xavierd
Differential Revision: D27526032
fbshipit-source-id: 0d3fa383772f719524a9be84b73fa2eb599580d7
Summary:
`getConfig` was ambiguous, so differentiate EdenConfig and
CheckoutConfig across the implementation.
Reviewed By: genevievehelsel
Differential Revision: D28398762
fbshipit-source-id: 9490e4b31c5d1b0570ee5615e5a3197400397993
Summary: To avoid inconsistency on an unimportant degree of freedom, disallow configs with underscores.
Reviewed By: genevievehelsel
Differential Revision: D28398510
fbshipit-source-id: 7720bd79129102d668d229979733a90547e275da
Summary:
EdenFS doesn't have a mononoke backend anymore, so the configs are no
longer necessary.
Reviewed By: genevievehelsel
Differential Revision: D28398369
fbshipit-source-id: 63f352bd82bc14b745535c227cb213e01f15afe8
Summary:
As EdenConfig grows more knobs, some structure is warranted. I'd
prefer something like anonymous structs or maybe even an
EdenSettingGroup type, but for now add some comments and at least put
them near each other.
Reviewed By: genevievehelsel
Differential Revision: D28398306
fbshipit-source-id: 169c4d2e0eeeb36d1812cdb73c9ac61d11652e09
Summary:
The structure had 2 padding of 4 bytes each due to the alignment requirement of
the EdenMount pointer and of the location_ field. Moving the EdenMount pointer
allows the padding to go away.
Reviewed By: fanzeyi
Differential Revision: D28476945
fbshipit-source-id: b521b4184d3924480ef54f840389156faab3988d
Summary: if this option is enabled, the server will be asked to add them
Reviewed By: markbt
Differential Revision: D28412810
fbshipit-source-id: d1531ecf97615cdb5e32d72c8c31598e6a406956
Summary:
Like it says in the title. This updates various streams so that they yield
after 10+ ms on CPU, thus ensuring that they don't starve other tasks of
runtime.
Reviewed By: johansglock
Differential Revision: D28441879
fbshipit-source-id: 78a8cdaefbe37d4768211dfa8a87c045ed898d57
Summary: The new and new_with_ttl constructors were causing duplication. Combine them.
Reviewed By: farnz
Differential Revision: D28471162
fbshipit-source-id: c7095a9a337d0ccbf5cd15ac3650cd5b361aaebf
Summary:
manual_scrub was using the ordered form of buffered so that checkpoint was written correctly.
This diff switches to buffered_unordered which can give better throughput. To do so checkpoint uses a tracker to know what keys have completed, so it can save the latest done key which has no preceding keys still executing.
Reviewed By: farnz
Differential Revision: D28438371
fbshipit-source-id: 274aa371a0c33d37d0dc7779b04daec2b5e1bc15
Summary: There isn't much point to having a trivial setter method for a public field.
Reviewed By: krallin
Differential Revision: D28427464
fbshipit-source-id: e0800fae6f635612e72945e570ce4de692dfb7bb
Summary:
These tests were hitting prod LFS. I have fixed them in our post-fork codebase here: D28093406 (490cbbf0c3)
We don't need them on hg servers so just delete them.
Reviewed By: krallin
Differential Revision: D28438295
fbshipit-source-id: 49961a4bb5965bd350d97b946c8c9fbeeefdebcc
Summary: Let's verify what we know can go wrong.
Reviewed By: StanislavGlebik
Differential Revision: D28419082
fbshipit-source-id: f1f50de7bff4cf3dc1469ecfbd3744624f26c08d
Summary:
This adds the ability to spawn sync_changeset() request using scs. We don't
have async reqeusts implementation, so it's quite hacky - it doesn't return the
actual synced changeset id. we'll add it later once async queue is implemneted
Reviewed By: ikostia
Differential Revision: D28441052
fbshipit-source-id: 5dd32c1638246442c6a9b1e21d7a8d73bfe762ab
Summary:
One more attempt to mitigate S231752.
From the debugging we did locally it seems that warning about expensive
getbundle might prevent keep alives from being send to the client.
adding yield seem to mitigate the issue locally.
We are not 100% sure it will actually mitigate the sev, but it seems like a
cheap and safe thing to try.
Reviewed By: krallin
Differential Revision: D28438570
fbshipit-source-id: 14f613222e8f69c4cbe490d8fa608d79c30b1a4d
Summary: This is useful to get an idea of what the scrub is doing
Reviewed By: farnz
Differential Revision: D28417785
fbshipit-source-id: 1421e0aae13f43371d4c0d066c08aee80b17e9c0
Summary:
During S231236 the LFS servers were using a lot of CPU and were
compressing blobs sent to clients. Thomas hotfixed the servers to disable
compression in order to save some CPU. Instead of having to rebuild and
redeploy the LFS server, update it to be able to disable compression using
configerator.
The `disable_compression` option will disable compression of downloads
globally. The `disable_compression_identities` allows us to disable compression
for some group of identities.
I also refactored some of the shared code from `download` and `download_sha256`
into a new function, `download_inner`.
Reviewed By: krallin
Differential Revision: D28382405
fbshipit-source-id: 792f10a9e3bb32b56ef87aa8e0b2c4b098567579
Summary: Write mostly stores are often in the process of being populated. Add an option to control whether scrub errors are raised for missing values in write mostly stores.
Differential Revision: D28393689
fbshipit-source-id: dfc371dcc3b591beadead82608a747958b53f580
Summary:
Couple of small changes I originally did during next diff split out on their own.
Save a loop over the scrub stores if there are relevant entries on the queue and remove unnecessary repetition of type.
Reviewed By: farnz
Differential Revision: D28411617
fbshipit-source-id: 2015b5b1d68f870b09fbd8929a59c21fe4f57c87
Summary:
This was broken by my recent change to have mergetools respect HGPLAIN
instead of ui.formatted.
Reviewed By: andll
Differential Revision: D28423783
fbshipit-source-id: 00831a6cc47acc11574fcf67462a1dccdde21fda
Summary:
This allows the macro to be used for both folly::Future, but also with
ImmediateFuture, reducing the cost of migrating to the latter.
Reviewed By: chadaustin
Differential Revision: D28302478
fbshipit-source-id: d8470428ecaa6aaccf2aa884437ca97d4b45806f
Summary:
Now that both implementation are using ImmediateFuture, we can move the
ImmediateFuture one layer up.
Reviewed By: kmancini
Differential Revision: D28302479
fbshipit-source-id: 3c2c164a90ffb42a0e7da8528f976af34fc87315
Summary:
With the Mountd code being converted to ImmediateFuture, we can now convert
Nfsd3 to use ImmediateFuture too. For now, this isn't expected to perform
better due to the InodeMap still using on folly::Future, which forces the
ImmediateFuture code to store a SemiFuture. A future change will look at
switching the InodeMap to ImmediateFuture to solve this.
Reviewed By: kmancini
Differential Revision: D28297422
fbshipit-source-id: 8b85103657e877b0f102130f2117bbe60598ed52
Summary:
Since lambda are default capturing by const, Func was also captured that way,
which made it impossible to call if Func was actually a mutable lambda.
Reviewed By: chadaustin
Differential Revision: D28297423
fbshipit-source-id: b9c6bdcf024c2e219ec0f8f5be2379e51df24db0
Summary:
As a first towards converting the code to use ImmediateFuture more broadly,
let's start with a small self contained part of the code.
This is mostly a sed except for the dispatchRpc method that needs to call
.semi().via().
Reviewed By: kmancini
Differential Revision: D28297421
fbshipit-source-id: e706e91fc8f132d4ef742ae98af9bb8304e0bf36
Summary:
This code initially was for thenValue, but when I converted it for thenTry I
forgot to also remove the hasException case. We were never calling the callback
on a Try that had an exception.
Reviewed By: chadaustin
Differential Revision: D28302477
fbshipit-source-id: 755fb2c8928627fbe1883f3863cafc360e34a038
Summary:
When the passed in callback returns an ImmediateFuture, the code had a small
typing bug where we would try to return an ImmediateFuture<ImmediateFuture<T>>
while the function signature expected an ImmediateFuture<T>. This is due to the
SemiFuture code not coalescing a SemiFuture<ImmediatureFuture<T>> into a
SemiFuture<T>.
Reviewed By: chadaustin
Differential Revision: D28293942
fbshipit-source-id: 1bc8f58d387766f29e573499fb37402ff9b49c83
Summary:
Mercurial has gotten stricter about respecting interactive vs
non-interactive commands lately, and now is failing to automatically open the
editor for conflicts during arc pull. Let's force Mercurial to treat the
invocation as an interactive one.
Reviewed By: skotchvail
Differential Revision: D28358999
fbshipit-source-id: 551713a78abfe170f04e8e55318af6e157bae7da
Summary:
When you spawn a task, Tokio puts it on a "LIFO slot" associated with the
current thread. While the task is in the LIFO slot, it is not eligible to be
run by other threads.
If the thread that just spawned `fwd` above goes and does some expensive
synchronous work in `request_handler` (we'd like to avoid that but sometimes
that happens), then that will delay `fwd`.
This means that notably keepalives will not be sent (you can repro by putting a
`std:🧵:sleep` right after we spawn `fwd`). To mitigate this, we spawn
another dummy taks here. This task will take `fwd`'s place in the LIFO slot,
thus pushing `fwd` onto a task queue where other runtime threads can claim it.
This way, even if this thread goes do some expensive CPU-bound work, we won't
delay keepalives.
This doesn't solve the fact that we probably shouldn't be doing such CPU-bound
work to begin with, but at least it might mitigate the issue such that we don't
have I/O delayed even though the runtime isn't visibly overloaded.
Reviewed By: johansglock
Differential Revision: D28412114
fbshipit-source-id: b56c0156ac6cf991cc899a82e3d2a96c63216fda
Summary: Add a write mostly store to the scrub test cases. This in preparation for next diff where the write mostly case has a new scrub option added.
Reviewed By: krallin
Differential Revision: D28393690
fbshipit-source-id: f7878f5e25814a7d327b1a80d4f96c0867944a14
Summary: Log the blobstore stack being used for the scrub
Reviewed By: farnz
Differential Revision: D28408340
fbshipit-source-id: 2299f7f7397f48d70b9a8295f0aa28c89bbf5809
Summary: These will be used in the next diffs, let's add initialization here.
Reviewed By: ikostia
Differential Revision: D28409734
fbshipit-source-id: f656db8259f28559df52562f0590382d89f1f0c0
Summary:
This is an attempt to split D28357210 to make it a bit smaller, so this
new function will be used later.
Reviewed By: krallin
Differential Revision: D28386004
fbshipit-source-id: 42c9ead0668fb78747dcb0fc7c89b6f181f7d9e6
Summary:
We have clients mysteriously timing out despite the fact that we should
normally be sending keepalives. To understand this, add some logging so that if
a client disconnects, we log a lot more detailed information to Scuba.
Reviewed By: mitrandir77
Differential Revision: D28383122
fbshipit-source-id: 0e49262bcc08c75f8e06eae17c882742b58ea51d
Summary:
I'd like to have some socket level logging, and it's good to capture the
metadata there so that we have client identities, session ids, etc.
But I can't do this if I don't have the metadata! This updates our code so that the
metadata is available where this logging will go.
This means we can't rely on loggers in `try_convert_preamble_to_metadata`, but
given this is now only used for hgcli, I don't think that matters, so I just removed it.
Reviewed By: mitrandir77
Differential Revision: D28383121
fbshipit-source-id: 27532f021a9082e74403bba73cad4fc6d0a6d97c
Summary:
I'd like to have some socket level logging, and it's good to capture the
metadata there so that we have client identities, session ids, etc.
To do so I need to not move it into the session. This does that.
Reviewed By: mitrandir77
Differential Revision: D28383123
fbshipit-source-id: 3fd10c3720465824dbcb2528227cbb3521d3068a
Summary:
gtest includes some windows headers that will have conflicts with the
folly portability versions. This caused some issues in my in-memory tree
cache diffs (D27050310 (8a1a529fcc)).
We should probably generally be using the folly portable gtests so we can
avoid such issues in the future.
see here for more details: bd600cd4e8/folly/portability/GTest.h (L19)
I ran this with codemod yes to all
- convert all the includes with quotes:
`codemod -d eden/fs --extensions cpp,h '\#include\ "gtest/gtest\.h"' '#include <folly/portability/GTest.h>'`
- convert all the includes with brackets
`codemod -d eden/fs --extensions cpp,h '\#include\ <gtest/gtest\.h>' '#include <folly/portability/GTest.h>'`
- convert the test template
`codemod -d eden/facebook --extensions template '\#include\ <gtest/gtest\.h>' '#include <folly/portability/GTest.h>'`
then used `arc lint` to clean up all the targets files
Reviewed By: genevievehelsel, xavierd
Differential Revision: D28035146
fbshipit-source-id: c3b88df5d4e7cdf4d1e51d9689987ce039f47fde
Summary:
There are a few tests that we don't currently run on both git and hg,
this means we need a new decorator to run the tests on nfs.
Reviewed By: xavierd
Differential Revision: D27953136
fbshipit-source-id: f6e1fab1f763c9b878315336b2cdaa529d36ffe5
Summary:
getdeps builds are failing on certain versions of Mac because they
choose a system python, which causes setup.py to use a hard coded library
location which isn't correct in our environment. Earlier I changed
pick_python.py to prefer the homebrew python, but it turns out getdeps doesn't
actually use pick_python. This diff fixes that and also instructs python3-sys to
use the correct version, by setting the PYTHON_SYS_EXECUTABLE environment
variable.
Reviewed By: quark-zju
Differential Revision: D28388150
fbshipit-source-id: 9b09e7472733f7a779c6212ae012116cad657b5d
Summary:
In order to get Watchman to recognize EdenFS mounted as NFS, we previously set
the f_mntfromname to be "edenfs". Unfortunately, Apple decided that when the
NFS server is accessed via a unix socket it would overwrite that and instead
use the path to the socket.
Digging in the source code, I did find that the f_fstypename can be overwritten
by calling fsctl with the right set of arguments. Thus, let's do just that.
Reviewed By: kmancini
Differential Revision: D28270605
fbshipit-source-id: f6be4e394d814806aa03ec3e82b8bc2faf364ea7
Summary:
Due to NFS being designed as a network filesystem, it default to binding on a
TCP socket. For EdenFS, since we're not expecting to mount an actual remote
filesystem, we bind these sockets to localhost. Unfortunately, TCP sockets have
some inherent overhead due to being designed to be reliable over a non-reliable
medium.
On macOS, Apple provides a way to mount an NFS server that is listening on a
unix domain socket. Thanks for unix socket being reliable, the TCP overhead
goes away leading to some higher throughput and lower latency for the NFS
server. For EdenFS, timing `rg foobar` over a directory containing 27k files gives:
NFS over TCP:
rg foobar > /dev/null 0.80s user 5.44s system 567% cpu 1.100 total
NFS over UDS:
rg foobar > /dev/null 0.77s user 5.27s system 679% cpu 0.888 total
osxfuse:
rg foobar > /dev/null 0.87s user 5.59s system 662% cpu 0.975 total
The main drawback of going through a unix socket is that D27717945 (bcf6aa465c) appears to
no longer be effective due to
8f02f2a044/bsd/nfs/nfs_vfsops.c (L3739)
Reviewed By: kmancini
Differential Revision: D28261422
fbshipit-source-id: 25dc1dc78cdb50d6c6550a86ef01ea2c894c110f
