Summary: Currently, tree imports are queued regardless of whether they are in the `hgcache`. This adds unnecessary delay, especially if the queue is busy (importer takes a long time and causes queue to backlog). This diff adds the logic to check if the tree is in `hgcache` before enqueuing a tree import request.
Reviewed By: xavierd
Differential Revision: D30514871
fbshipit-source-id: eb23f64b7f059832571f957fb67d18c3821d2844
Summary:
Some code in the HgDatapackStore is overly complicated due to the fact that
revHash returns a owned Hash and this forces the code to thus copy it onto a
temporary vector. By having a method that can directly return a slice to the
hash, this issue disappears, thus let's add it.
Reviewed By: chadaustin
Differential Revision: D30582458
fbshipit-source-id: dc102117bc82ab72378293c0abfe9acfd862e9e6
Summary:
I noticed that every time we fetch blob from hg, we calculate sha hash and put it into metadata table.
Both calculating sha1 of content and writing it to rocks is fairly expensive, and it would be nice if we can skip doing so in some cases.
In this diff I use inexpensive cache check to see if we already calculated metadata for given blob and skip recalculation
In terms of performance, it reduces blob access time in hot case from **0.62 ms to 0.22 ms**.
[still need to do some testing with buck, but I think this should not block the diff since it seem farily trivial]
This is short-medium term fix, the longer term solution will be keeping hashes in mercurial and fetching them via eden api, but this will take some time to implement
Reviewed By: chadaustin, xavierd
Differential Revision: D30587132
fbshipit-source-id: 3b24ec88fb02e1ea514568b4e2c8f9fd784a0f10
Summary: Similarly to the enqueue benchmark, let's have a dequeue benchmark.
Differential Revision: D30560489
fbshipit-source-id: ae18f7e283e4bab228aaa0f58bff2e6f2cfa3021
Summary:
In order to enqueue and find an element in a hash table, the key needs to be
hashed. Hashing a HgProxyHash relies on hashing a string which is significantly
more expensive than hashing a Hash directly. Note that they both represent the
same data and thus there shouldn't be more collisions.
Reviewed By: chadaustin
Differential Revision: D30520223
fbshipit-source-id: 036007c445c28686f777aa170d0344346e7348b0
Summary:
Allocations are expensive, especially when done under a lock as this increase
the critical section, reducing the potential concurrency. While this yields to
a 1.25x speedup, this is more of a sideway improvement as the allocation is now
done prior to enqueuing. This also means that de-duplicating requests is now
more expensive, as no allocation would be done before, but at the same time,
de-duplication is the non-common code path, so the tradeoff is worthwhile.
Reviewed By: chadaustin
Differential Revision: D30520228
fbshipit-source-id: 99dea65e828f9c896fdfca6b308106554c989282
Summary: The F14 hashmap are significantly faster than the std::unordered_map.
Reviewed By: chadaustin
Differential Revision: D30520225
fbshipit-source-id: d986908c5eac17f66ae2c7589f134c430a3c656e
Summary:
When turning on the native prefetch, EdenFS will enqueue tons of blob requests
to the import request queue. The expectation is then that the threads will
dequeue batch of requests and run them. What is being observed is however
vastly different: the dequeued batches are barely bigger than 10, far lower
than the batch capacity, leading to fetching inefficiencies. The reason for
this is that enqueuing is too costly.
The first step in making enqueuing less costly is to reduce the number of times
the lock needs to be acquired by moving the de-duplication inside the enqueue
function itself. On top of reducing the number of times the lock is held, it
also halves the number of allocation done under the lock.
Reviewed By: chadaustin
Differential Revision: D30520226
fbshipit-source-id: 52f6e3c1ec45caa5c47e3fd122b3a933b0448e7c
Summary:
It turns out that we do want to use a Future to make sure that the tracebus and
watches are completed on the producer and not on the consumer of the future. We
could use a `.via(inline executor)` but the code becomes less readable, so
let's just revert the diff.
Reviewed By: chadaustin
Differential Revision: D30545721
fbshipit-source-id: 524033ab4dbd16be0c377647f7f81f7cd57c206d
Summary:
This change has the unintended effect of causing any Thrift calls to
potentially issue a recursive EdenFS call due to symlink resolution requiring
running `readlink` on the root of the repo itself.
Fixing this isn't really possible, thus let's revert the change altogether, we
can force clients to issue a realpath before issuing EdenFS Thrift calls.
Reviewed By: kmancini
Differential Revision: D30550796
fbshipit-source-id: 9494c8e08c8af2392eeb344879f156cb56f93ea6
Summary:
The documentation allows for not having to test in enqueue if the queue is
still running: if called in the destructor of the owner, no enqueue can
logically happen, and thus we do not need to protect against it.
Reviewed By: chadaustin
Differential Revision: D30520227
fbshipit-source-id: 9d6280ccd7fe875cd06b0746151a2897d1f98d61
Summary:
When trying to push thousands of requestst to the queue, the dequeue side only
manage to pull batches of ~10 requests at most. Let's measure the cost of
enqueue to optimize it.
Reviewed By: chadaustin
Differential Revision: D30503110
fbshipit-source-id: d06ae6741b13b831fa3711fb2dd0e38c3e54193c
Summary: Added a kill switch to enable/disable predictive prefetch profiles similar to the existing one for regular prefetch profiles (D24803728 (7dccb8a49f)). This can be set manually in a user's config or via the cli `eden prefetch-profile disable-predictive/enable-predictive` commands.
Reviewed By: genevievehelsel
Differential Revision: D30404139
fbshipit-source-id: 01900f4030ef6991124f89a67ea404ff2f07ffeb
Summary:
Added eden prefetch-profile activate-predictive/deactivate-predictive subcommands to activate and deactivate predictive prefetch profiles. This will update the checkout config to indicate if predictive prefetch profiles are currently active or not, and stores the overridden num_dirs if specified on activate (--num-dirs N). If activate is called twice with different num_dirs, the value is updated (only one is stored). Unless --skip-prefetch is specified, a predictive prefetch with num_dirs globs (or the default inferred in the daemon) is run.
Also added fetch-predictive [--num-dirs N], which will:
1. if num_dirs is specified: fetch num_dirs globs predictively
2. if num_dirs is not specified, and predictive fetch is active: get the active num_dirs from the checkout config and fetch globs predictively
3. if num_dirs is not specified, and predictive fetch is not active: fetch the default num_dirs (inferred in the daemon)
Added --if-active to fetch-predictive. If set, fetch will not run if predictive prefetch profiles have not been activated (predictive-prefetch-active in checkout-config). Used for post pull hook.
Reviewed By: genevievehelsel
Differential Revision: D30306235
fbshipit-source-id: ba02c2bc976128704c8ab0c3d567637265b7c95d
Summary:
Made changes to ensure that numResults is always a 32 bit unsigned int, and startTime and endTime are 64 bit unsigned ints. This is to ensure consistency across the smartservice and the endpoint in the daemon.
Also, updated the scuba query in the smartservice to only consider dirs with > 1 access (may update this later to accept a configurable lower bound on access count, but for now, including access=1 doesn't make sense).
Reviewed By: genevievehelsel
Differential Revision: D30396526
fbshipit-source-id: 10e7bd969928da91ab29d413280a1ff956db438c
Summary:
This is now only used in HgQueuedBackingStore::logBackingStoreFetch, and
manually inlining it allows for the lock to be taken once instead of once per
path, reducing the number of times the lock needs to be acquired.
Differential Revision: D30494771
fbshipit-source-id: 2d59d0343e48051e4d9c4fc196e66bcb79e7ac71
Summary: While `eden trace hg` already prints queue time when it's over 1ms, this diff adds fb303 counters for import tree/block queue time so that we can have the percentiles.
Reviewed By: xavierd
Differential Revision: D30492275
fbshipit-source-id: 3601aeb9b51b2f55f189a0e0a753fd6ef29d7341
Summary: Currently, the store loops through the requests, calls HgImporter, then waits with `getTry`. This diff makes the change to kickoff all tree imports from HgImporter then waits for future fulfillment with `collectAll`.
Reviewed By: xavierd
Differential Revision: D30486459
fbshipit-source-id: 918e52be818a2064cf04d24f455d23c1ca618434
Summary:
Instead of having 2 functions with one taking a single proxy hash, and the
other taking a vector, we can simply have a single function taking a
`folly::Range` and pass a range of one for the single proxy hash case.
Reviewed By: chadaustin
Differential Revision: D30490724
fbshipit-source-id: 5d57f5a5ffc2a5085369c61a2318edd54b24b448
Summary:
By default, atomics are using the most strict memory ordering forcing barriers
to be used. Since this atomic doesn't need any ordering, we can make it
relaxed.
Reviewed By: chadaustin
Differential Revision: D30459630
fbshipit-source-id: ff50aac919031d9bae8b870b41a6134331546a5f
Summary:
The recordFetch is an implementation detail of a BackingStore and thus we don't
need to explicitely make it virtual.
Differential Revision: D30459635
fbshipit-source-id: 34f847ca906f81924c99c26b4e8af646e91fd735
Summary:
When prefetching a large number of blobs, repeatly checking whether we should
log accesses to files can become expensive. Since the state of the config isn't
expected to change in the entire batch, we can simply test it once and bail if
logging isn't enabled.
Reviewed By: chadaustin
Differential Revision: D30458698
fbshipit-source-id: b48b9e0ad24585a76d8ce5948f5831db27e08eab
Summary: Looks like we never use this, thus let's simply remove it.
Differential Revision: D30454812
fbshipit-source-id: 28242a2144da4bab9d24debc1a60eeebcdcbaad5
Summary:
When a prefetch request is transformed into many blob requests, we query
RocksDB sequentially for all the proxy hashes, this can be quite expensive and
is also far less efficient than querying RocksDB concurrently with all the
hashes.
As a bonus, this also futurize the code a bit.
Reviewed By: chadaustin
Differential Revision: D30454068
fbshipit-source-id: 5fd238b752a662919e739451c0c1e92f66919ebf
Summary:
Since these are always used as SemiFuture, let's simply make them SemiFuture
from the get go.
Differential Revision: D30452901
fbshipit-source-id: b0863f363ce0cdb921a73d02c43fc82c1614a3dc
Summary:
Looking at strobelight when performing an `eden prefetch` shows that a lot of
time is spent copying data around. The list of hash to prefetch is for instance
copied 4 times, let's reduce this to only one time when converting Hash to a
ByteRange.
Reviewed By: chadaustin
Differential Revision: D30433285
fbshipit-source-id: 922e6e5c095bd700ee133e9bb219904baf2ae1ac
Summary:
Once the request has been dequeued, we no longer need to hold the lock, thus
let's release it to allow other threads to enqueue/dequeue requests.
Differential Revision: D30409797
fbshipit-source-id: a527c67a6bd9f47da5a3930364fd8fae0d1bc427
Summary: This will let ```SetPathRootId``` suppose files
Reviewed By: chadaustin
Differential Revision: D29978308
fbshipit-source-id: df22af8bce4a707a7db51ef543c0e3e78cdcef06
Summary: This diff renames ```SetPathRootId``` to ```SetPathObjectId``` as we want to support BLOB
Reviewed By: chadaustin
Differential Revision: D30404536
fbshipit-source-id: f34446ec20aeaf87f5f61e29e421a9bceb0b2a4a
Summary: This will add the same getTreeEntryForRootId to ObjectStore
Reviewed By: chadaustin
Differential Revision: D29920475
fbshipit-source-id: 15bfc6a2ba70cce2095dfcf1f434fd7087605e04
Summary: Add a new method to backingstore so we can get TreeEntry by rootID
Reviewed By: chadaustin
Differential Revision: D29889482
fbshipit-source-id: 93e63624e75c7d559c4de6f68821a8efa0e0c184
Summary: This is an untended pop and would throw if there is no stale apfs volumes (and would remove one less volume if there are stale volumes).
Reviewed By: xavierd
Differential Revision: D30432642
fbshipit-source-id: 193d9c15f393a66bc8b43b5f31579c1fe972a7f1
Summary:
The RelativePath is always built from a valid valid one, thus re-validating it
is not necessary.
Reviewed By: chadaustin
Differential Revision: D30410686
fbshipit-source-id: 3e46359f68b1693a0a2af310466fc73d105cf2c0
Summary:
This adds allowlisted configs to FS trace event sample, which would facilitate A/B testing and parameter tuning. For example, if we want to verify if a larger `hg:import-batch-size` would speed up read operations, we can:
1. split users into two groups, one having size of 16 and another having 32.
2. make sure `hg:import-batch-size` is included in `telemetry:request-sampling-config-allowlist` config.
3. wait for events to populate and compare the durations.
Reviewed By: xavierd
Differential Revision: D30322855
fbshipit-source-id: b3cbdcb64f78d35b8708948db495b2d956cab327
Summary:
The current fb303 counters only report aggregated latency while we want to track Eden performance under different version, os, channel, and configs. So I am setting up a new logging mechanism for this purpose.
This diff introduces the class `FsEventLogger` for sampling and logging. There are 3 configs introduced by this diff. The configs are reloaded every 30 minutes.
1. `telemetry:request-sampling-config-allowlist`
A list of config keys that we want to attach to scuba events.
2. `telemetry:request-samples-per-minute`
Max number of events logged to scuba per minute per mount.
3. `telemetry:request-sampling-group-denominators`
* Each type of operation has a "sampling group" (defaulted to 0, which is dropping all).
* We use this sampling group as index to look up its denominator in this config.
* The denominator is then used for sampling. e.g. `1/x` of the events are send to scuba, if we haven't reached the cap specified by #2.
Example workflow:
1. receive tracing event
2. look up denominator of the sampling group of the operation type
3. sample based on the denominator
4. check that we have not exceeded the logging cap per min
5. create sample and send to scribe
Reviewed By: xavierd
Differential Revision: D30288054
fbshipit-source-id: 8f2b95c11c718550a8162f4d1259a25628f499ff
Summary:
This adds the `eden redirect cleanup-apfs` command for deleting stale APFS volumes.
* An APFS is considered as stale if it's not currently mounted and not considered as under any of the checkouts managed by the eden instance.
* The command prints the list of such volumes and uses the APFS util to delete them if the user confirms.
Note: as the command is local to an eden instance, it will list not-mounted APFS volumes of a checkout managed by another eden instance as stale. This should rarely happen as in production we expect there to be a single eden instance. The prompt would also let the user abort if something is wrong.
Reviewed By: chadaustin
Differential Revision: D29940980
fbshipit-source-id: e784cb54d20198bb1f74cd5f15cee0e7546b227c
Summary: `edenfsctl rm` often breaks down due to long path on Windows. This diff fixes that issue.
Reviewed By: xavierd
Differential Revision: D30380003
fbshipit-source-id: e10faa357df932bdb49d7c62d04d9504c7885768
Summary:
By default, a prefetch will always go through the Mercurial importer, but
when store:use-eden-native-prefetch is set, prefetch is simply pushing tons of
blob requests to the HgQueuedBackingStore, and the internal batching takes care
of efficiently fetching.
The one drawback is that prior to pushing a blob request to the queue, getBlob
will query Mercurial to see if a blob is present locally. In the context of a
FUSE access, this is totally expected as this allows for low latency blob
access. But for a prefetch call, throughput matters significantly more and the
local check can negatively affect this.
Reviewed By: genevievehelsel
Differential Revision: D30404965
fbshipit-source-id: 113883993fa641caf7095a5bc8b7dd802f33348d
Summary:
As I'm looking through the code, it took me a bit of time to fully grasp what
the commented code was for, document it for future readers.
Reviewed By: chadaustin
Differential Revision: D30399723
fbshipit-source-id: bdf448b725192d7541b1d7de7e043ff97700dbce
Summary: This is never called, no need to keep it around.
Reviewed By: chadaustin
Differential Revision: D30399722
fbshipit-source-id: bbc169141b58976031fcae224f24ea23897c6f21
Summary:
If a wrong type is passed in, an exception would be thrown at runtime, while
the static_assert would fire at compiled time, reducing the time to find the
error for the developer.
Reviewed By: chadaustin
Differential Revision: D30398255
fbshipit-source-id: fd021f96063565f83c55a9bf3f175bf879afa6ed
Summary: This diff teaches eden doctor to generate a warning when user has SQLite overlay repo on disk and asking them to migrate.
Reviewed By: chadaustin
Differential Revision: D30345721
fbshipit-source-id: 95796ca77979f034904b87e3a38f149baddd720a
Summary:
This is a re-submit of D29915585 (6c5c7055ce), I've merely fixed the bug that it introduced,
thus the credit goes to markbt. Below is the original commit message:
If Mercurial asks EdenFS to update to a commit that it has just created, this
can cause a long delay while EdenFS tries to import the commit.
EdenFS needs to get the trees out of the Hg data store. But these also won't
know about the new trees until the data store is refreshed or synced.
To fix this, call the refresh method on the store if we fail to find the tree,
and try again. To make this work, we must first only look locally. To keep
things simple, we only do this for the root tree.
However, currently indexedlogdatastore doesn't actually do anything when you
ask it to refresh.
To fix these, we call flush(), which actually does a sync operation and loads
the latest data from disk, too.
Reviewed By: chadaustin
Differential Revision: D30387805
fbshipit-source-id: 3fdbd27b306f03df53b68a0bcc5ee5dc140326bb
Summary:
The SmartPlatform service that queries for a user's most used directories allows optional parameters of: os, startTime, endTime, and sandcastleAlias instead of user. This diff extends the current predictive prefetch option which queries based on the current user, mount repository, and a default numResults, to allow specification of all parameters including the optional ones.
If a user and/or repo is not specified these are determined from the server state and mount, respectively. If numResults is not specified, a default value is used (predictivePrefetchProfileSize, currently 10,000).
For sandcastle aliases, we check if the SANDCASTLE_ALIAS environment variable is set, and if so, use the value as a parameter. If a sandcastle alias is specified, the smartservice will ignore the user and query based on the alias, otherwise a user is assumed.
Differential Revision: D30160507
fbshipit-source-id: 174797f0a6f840bb33f669c8d1bb61d76ff7a309
Summary:
When a user reports a slow EdenFS and a high network traffic is suspected, the
lack of Thrift stats makes it hard to fully validate this. Thus, let's collect
some stats and put them in the rage.
The collected stats are the exact same ones that `eden top` uses.
Reviewed By: chadaustin
Differential Revision: D30355746
fbshipit-source-id: 519a8e2c8b0c458daecdcc0813a8d7416d5362d6
Summary:
In the case where the path to the mount has symlinks, EdenFS would only accept
the path to it that was specified at mount time, even though another path may
refer to the same directory.
To solve this, we can simply normalize paths in all the Thrift endpoint to make
sure that EdenFS always refers to a mount point under its non-symlinked path.
Reviewed By: chadaustin
Differential Revision: D30320515
fbshipit-source-id: e578d059a3b1307d6b24c4b9bdb1ceb3b534c460
Summary:
Take 2. This time with measured improvement (3x).
With WAL mode enabled in SQLite. The bottleneck focuses at flush time. WPA shows a lot of time are spent at flushing changes into the WAL file.
Setting `synchronous` to `OFF` in SQLite will make SQLite to not wait for flushing changes onto the disk. This saves us time and compromises with correctness only when the system crashes.
From SQLite documentation:
> With synchronous OFF (0), SQLite continues without syncing as soon as it has handed data off to the operating system. If the application running SQLite crashes, the data will be safe, but the database might become corrupted if the operating system crashes or the computer loses power before that data has been written to the disk surface. On the other hand, commits can be orders of magnitude faster with synchronous OFF.
https://sqlite.org/pragma.html#pragma_synchronous
Reviewed By: chadaustin
Differential Revision: D30314601
fbshipit-source-id: 87f6cd05149045313678328bb3fc7dd10e894e37
Summary:
Whenever a file is modified in an EdenFS mount and a watchman subscription is
active, watchman will be nodified and will issue a getFilesChangedSince Thrift
call. In order to do that, Watchman ends up always re-creating a new connection
to EdenFS, causing the .eden/socket or .eden/config to be re-read in order to
find EdenFS's socket.
For workloads with heavy write traffic to EdenFS, this readlink/read can add
up. On Windows, writing ~2.5GB worth of data lead Watchman to read over 650MB
worth of data from the .eden/config!
Reviewed By: kmancini
Differential Revision: D29508654
fbshipit-source-id: 60440d645340bc4fe16ea9618d7a5080740e4d87
Summary: The journal stream is disconnected at Watchman shutdown, which is the expected behavior. This changes the log level to INFO.
Reviewed By: chadaustin
Differential Revision: D30231657
fbshipit-source-id: 94909daeba786b1bed7497e4a21ffcfc52d6d9cb
Summary:
This moves some Prjfs logic into the channel code, which allows for
de-duplicating a bit of code. This will also make a subsequent change in the
rename code easier to do.
Differential Revision: D30023970
fbshipit-source-id: 7efa6dcc4318213e9d266932527b5a56edacefd7
Summary: Due to lifetime issues of FetchContext& w.r.t. background prefetching, we can just create the helper in `_globFiles` and use that to maintain lifetime
Reviewed By: xavierd
Differential Revision: D30175224
fbshipit-source-id: b2fccb76f9d4011139e80bd5bc52c40bbab08b94
Summary: Added a Thrift method that tells EdenFS to prefetch files from a user's most used directories using an endpoint that talks to the edenfs/edenfs_service SmartPlatform service to get the directory list. The default number of directories is set to 10,000.
Reviewed By: genevievehelsel
Differential Revision: D29909976
fbshipit-source-id: bfb1a411d50d7355ff604de5bc090a9e2c3100a0
Summary: This was an oversight. With this edenfsctl should be able to run on Windows and talk with EdenFS daemon via UNIX domain socket.
Differential Revision: D30089140
fbshipit-source-id: abe12aeee06d91f7a7323a6042d2b69c31963bbb
Summary:
Original commit changeset: 34fe02ddf580
We are seeing reports of EdenFS only showing partial directories to our user. Local testing shows this commit seems to be causing the issue. Reverting it for now.
Reviewed By: kmancini
Differential Revision: D30136949
fbshipit-source-id: 7fcc81506c132055a5b639a383b8c9be68118dc5
Summary:
This newly added config controls the default behavior of `eden clone` and
whether a new clone will use NFS or FUSE. This is intended to facilitate the
transition to NFS from FUSE on macOS.
Differential Revision: D30110056
fbshipit-source-id: ea6b493aa803297952b46434f6d11d8edf58e40b
Summary: As title. This was added as `thrift-py3` doesn't support CMake well but we can workaround by importing from py.
Reviewed By: xavierd
Differential Revision: D30078300
fbshipit-source-id: 277866f8b226f164b5e30231aa10b59c0aba5807
Summary:
This adds the options to `eden stats` for collecting only fast stats and printing in JSON.
`eden stats` can be slow especially due to collecting fb303 counters and private bytes. An example use case of this new lightweight endpoint is that Buck can poll it to display Eden related info in its cli (see [post](https://fb.workplace.com/groups/132499338763090/permalink/210396380973385/) for context).
Reviewed By: xavierd
Differential Revision: D29687041
fbshipit-source-id: a663e71231527c5dfb822acbf238af0ac6ce4a00
Summary: Currently we have to manually save the id returned from `start_recording`. After this, we can simply ask for the list of all active recorder sessions.
Reviewed By: genevievehelsel
Differential Revision: D30056117
fbshipit-source-id: 7fd69b70e7b04fcd0b3724f4ee16c5e5e86badaf
Summary:
This improves buck build(and likely other tools) execution time significantly on the cold cache, with eden api enabled(e.g. no import helper)
I have noticed during the tests that we don't urilize network well when accessing files not in cache.
Adding some instrumentation shows that we only do up to 8 parallel fetches on mercurial side.
This seem to be limited by number of hg queue workers, simply boosting this number from 8->32 improves performance a lot, and it is still manageable number of threads
We probably need some better solution to allow for greater parallelism.
With this diff time for `buck build fb4a` on the cold cache is reduced **from 121 to 95 minutes(-30%)**.
'Parsing buck files' stage is reduced **from 07:16 min to 05:53 min(-25%)**.
During buck build we get to actual parallelism of about 16 fetches which is limited by number of buck threads.
Bumping number of buck threads with `-j 32` further reduces build time to **54 minutes (-60% from original)**
Reviewed By: xavierd
Differential Revision: D30025720
fbshipit-source-id: 6bcae1f353a0d31d8ce632ccb991c2a02f3136fc
Summary:
* `FuseRequestContext` currently uses `fuseHeader_.opcode` to get the cause detail.
* However, `fuseHeader_.opcode` is set to 0 by `stealReq` to indicate that the request has been released.
* This lead to `getCauseDetail` returning `<unknown>` if we call it after the request is released (e.g. in the clean up future).
This adds the `opcode_` member so that `getCauseDetail` always return the correct cause detail regardless of whether the request has been released. This also removes `examineReq` as it's not used.
Reviewed By: xavierd
Differential Revision: D30050518
fbshipit-source-id: 0c3e4d31e59a5b5fd87967d6c976b573692a2af9
Summary:
Per comment in D30017261 (e9c039ab4a)
> [...] we can get rid of this ESTALE check. That dates back to when edenfs had a bug that returned ESTALE when reading the .eden directory sometimes.
Reviewed By: xavierd
Differential Revision: D30024979
fbshipit-source-id: 645097c8f689c916245845561fd3d824ff7df8b4
Summary: This is unused, no need to keep it around.
Reviewed By: genevievehelsel
Differential Revision: D30046503
fbshipit-source-id: 1d20d9b4ce672d5d79410203807dbc93b4bce31a
Summary: For `future_` endpoints, we wrap the final return statement with `wrapFuture(std::move(helper), ...` to ensure we keep the ThriftLogHelper alive through the whole call.
Differential Revision: D30018980
fbshipit-source-id: 2c63fe5d7b4504912cc46a32ca04f16e98b0805f
Summary: `eden mount` currently swallows all but one type of exceptions, which resulted in cli reporting false positive mount success when the thrift endpoint throws. The try-catch was added in D21934538 (51df752a46) to provide clear error message for a particular windows error and we can just re-raise so that the other exceptions are propagated.
Reviewed By: chadaustin
Differential Revision: D30017261
fbshipit-source-id: 6a8a44330a90275b3c044301ff644dce0d6dee13
Summary:
Today the globing code is not able to parse `\` as a path separator
which causes an error like this:
```
facebook.eden.ttypes.EdenError: Invalid glob (attempt to construct a PathComponent from a string containing a directory separator: tools\arcanist): tools\arcanist
```
Many other tools like hg files still use these separators, so it would be
nice if we could support `\`.
This change will mean that Windows globs will not support special characters.
But they were already broken so we can not have any users successfully using
them today.
Ideally we will not have to support special charcters on Windows because it is
icky, But I left a note in the code about how we could do this, if its ever
needed in the future
Reviewed By: chadaustin
Differential Revision: D29971586
fbshipit-source-id: 32f54986e801b2aa3b3b1c90c650bd45531e8c0a
Summary:
Copying PathPiece onto futures is a recipe for a use-after-free due to the
underlying string being likely freed when the future executes, let's instead
use plain Path and have the caller move/copy them onto the futures.
Differential Revision: D29989957
fbshipit-source-id: 2e6cebf8c9dfc8f55df8edbc410c2a7f681b8f22
Summary: Boost is not happy when it encounters UNIX domain socket file on Windows. Let's detect the file type manually instead of using boost.
Reviewed By: xavierd
Differential Revision: D29974282
fbshipit-source-id: e11558abdbc565014189ae763a5b3fb5486d38d7
Summary:
During a diff operation, files that have a case change should not reported as
changed for case insensitive mount. This is a follow up to D29150552 (37ccaa9231) where the
TreeInode code was taught to ignore case changes for case insensitive mounts.
Reviewed By: kmancini
Differential Revision: D29952464
fbshipit-source-id: e5fa8eccc1c1984a680838fd8589a0278989d9d0
Summary:
While not strictly necessary, we should be providing the correct `Key` for
trees when fetching them from the Hg datapack store. Pass the `path` through
to the backing store so it can be used to construct the `Key` for the tree.
Reviewed By: DurhamG
Differential Revision: D29933214
fbshipit-source-id: d9631ea37b5ffa3f7051112d12cf3161c7c677ef
Summary:
If Mercurial asks EdenFS to update to a commit that it has just created, this
can cause a long delay while EdenFS tries to import the commit.
EdenFS needs to get the trees out of the Hg data store. But these also
won't know about the new trees until the data store is refreshed or synced.
To fix this, call the refresh method on the store if we fail to find the tree,
and try again. To make this work, we must first only look locally. To keep
things simple, we only do this for the root tree.
However, currently indexedlogdatastore doesn't actually do anything when you
ask it to refresh.
To fix these, we call `flush()`, which actually does a `sync` operation and
loads the latest data from disk, too.
Reviewed By: DurhamG
Differential Revision: D29915585
fbshipit-source-id: 34fe02ddf5804be425d9cabe1a56069f22e5e4d4
Summary:
If Mercurial asks EdenFS to update to a commit that it has just created, this
can cause a long delay while EdenFS tries to import the commit.
EdenFS needs to resolve the commit to a root manifest. It does this via the
import helper, but the import helper won't know about the commit until it is
restarted, which takes a long time.
To fix this, we add an optional "root manifest" parameter to the checkout or
reset parents thrift calls. This allows the Mercurial client to inform EdenFS
of the root manifest that it already knows about, allowing EdenFS to skip this
step.
Reviewed By: chadaustin
Differential Revision: D29845604
fbshipit-source-id: 61736d84971cd2dd9a8fdaa29a1578386246e4bf
Summary:
There is no need to obtain the PrjfsChannel twice, especially if the first one
is unused.
Reviewed By: fanzeyi
Differential Revision: D29977555
fbshipit-source-id: 56428eae84a6abd8689b4f997173e0aa1501aede
Summary:
TSAN complains that pipes_ is read and written in different threads without
proper synchronization. To avoid this, we can simply close the FileDescriptor
without removing it from the pipes map as this achieve the same result: it
notifies the reader that the endpoint is closed.
Differential Revision: D29924043
fbshipit-source-id: be92630799bb5c78089dbe85f9c2f02f937300bc
Summary: This affects all platforms but more noticeable on Mac that tons of 100% printed (e.g. P409794954), probably due to some weirdness with cursor.
Reviewed By: fanzeyi
Differential Revision: D29922276
fbshipit-source-id: 987f6b9ef5a8a4ab738aa6edbd617184bbcb2d1c
Summary: As title. `RequsetContext` allows us to track metrics such as latency and count.
Reviewed By: genevievehelsel
Differential Revision: D29835813
fbshipit-source-id: 6b85fc8f11923f530fce6d871fa2253db21bfa98
Summary:
On Windows, there a commonly occuring issue where a checkout operation would
crash EdenFS as a conflict is being added for an unlinked inode, thus
triggering the XCHECK in the addConflict method.
From looking at the code, the comment that claims that inodes cannot be
unlinked during checkout isn't entirely accurate: EdenFS will unlink inodes
during checkout when their content changed. The code itself should properly
remove the unlinked inode from its parent TreeInode and thus I haven't fully
figured out the exact series of event that leads to a conflict being added for
an unlinked inode. Since the asumption from the comment is invalid, it should
be safe to not assert that the inode shouldn't be unlinked and use
InodeBase::getUnsafePath instead of InodeBase::getPath
Reviewed By: kmancini
Differential Revision: D29241901
fbshipit-source-id: 4239df576b3cbf716fb336fd4d6542939337a297
Summary:
In some cases, the code needs to have access to the path for an inode even if
that inode is unlinked. In such situation, neither getPath nor getLogPath are
suitable, thus let's introduce a getUnsafePath, which is intended for these
handful of places.
The only known use case for such method is when adding conflicts during checkouts.
Reviewed By: genevievehelsel
Differential Revision: D29241902
fbshipit-source-id: 7756a95813d6fd5e471538cf82d29604dd5b8e5e
Summary:
This adds debug commands for ActivityRecorder:
```
eden debug start_recording --output-dir <DIR>
* stdout: the id of the profile
eden debug stop_recording --unique <ID>
* stdout: the output file path
```
Users can record multiple profiles concurrently. Each profile is identified by the timestamp when it started.
Reviewed By: genevievehelsel
Differential Revision: D29666359
fbshipit-source-id: 487ca67de77378a8141bc4ac46b9abd1375ffd23
Summary:
We want to introduce two debug commands to record perf profiles such as files read. This can later be integrated to CI so that we can have this data for troubleshooting perf issues.
* `eden debug start_recording` starts recording perf metrics such as files read/written and fetch counts/latency for a given mount.
* `eden debug end_recording` stops recording and dumps the recorded profile to a local file.
This diff adds the boilerplate `ActivityRecorder` (borrowed heavily from `HiveLogger`'s implementation). The start command would create an instance of the recorder; the end command would destroy the recorder. The recording and dumping are handled by the implementing class.
Reviewed By: genevievehelsel
Differential Revision: D29506895
fbshipit-source-id: a927a363942a041d5ae54186a265576325dfeed5
Summary: This adds counters for memory and disk counts in addition to import count so that we can understand cache hit rates during local investigation or output this in ActivityRecorder.
Reviewed By: genevievehelsel
Differential Revision: D29805637
fbshipit-source-id: 34261f91c33d6bd4bcb4b85b17d2e68360410896
Summary: Combine `eden debug log` and `eden logs`. The logic from `eden logs` is moved to `eden debug log upload`.
Reviewed By: genevievehelsel
Differential Revision: D29801785
fbshipit-source-id: 6283a33a3180fec6934ac52fc8d5eed4a0a483b0
Summary: This change let Eden cli can ```clone``` and ```info``` on a RE Digest backed store
Reviewed By: chadaustin
Differential Revision: D28855458
fbshipit-source-id: 5582992acc5b3b3acb05b0b53d59a6768cc02491
Summary: We already have AccessType for FUSE, this adds the same categorization for NFS. This allows us to easily filter events in trace stream and ActivityRecorder.
Reviewed By: chadaustin
Differential Revision: D29771074
fbshipit-source-id: a437f0693f9062fb2df3b6f618a9d8860a05df12
Summary: We want to support different batch sizes for blob or tree. This diff moves the batch size read logic into `HgImportRequestQueue`, adds a new config `import-batch-size-tree` (added in D29714772), and updates tests accordingly.
Reviewed By: kmancini
Differential Revision: D29703450
fbshipit-source-id: b85666838a0a8c1857b9d1de4f6c47128063633a
Summary: This can be used on windows since it uses `shutil.rmtree` instead of `fm -rf`
Differential Revision: D29723916
fbshipit-source-id: 7d12ce8d265661698c1f5ecd17271d1c2e950a55
Summary: Add a new helper function 'print_env_variables' that reads the environment variables and prints them at the bottom of the rage report.
Reviewed By: genevievehelsel
Differential Revision: D29713709
fbshipit-source-id: 04e10597c93d11b58420f184048d9b55ad4e5166
Summary: Extended eden doctor to check if the PrivHelper is accessible and report when it is not.
Reviewed By: genevievehelsel
Differential Revision: D29593250
fbshipit-source-id: 2390e75b91c9d6f713db4b6084868af91a0b6623
Summary: `-s` requires session id, so get the `sid` from `pid`
Reviewed By: genevievehelsel
Differential Revision: D29627171
fbshipit-source-id: b2812a150fe56b6fd6cfa246298247164861fc9d
Summary: Added a --deep-clean option to eden du that removes .edeb/clients/x/fsck directories.
Reviewed By: genevievehelsel
Differential Revision: D29501641
fbshipit-source-id: 9c01dc76b54e151ada977c0ee0c28baafe761824
Summary:
subprocess.run doesn't capture the output of a command by default, thus the
buckversion is populated with a CompletedProcess, which cannot fit in the
environment.
Differential Revision: D29576149
fbshipit-source-id: 9d0e13477ac2ffc479e093ea7231eb552c31a5ec
Summary: There are a lot of places in user visible text such as command line help where EdenFS is mentioned as eden/Eden/edenfs/EdenFS. This will make it consistent to 'EdenFS' in most cases. In the places where it is referring to the process/daemon, 'edenfs' will be used.
Reviewed By: chadaustin
Differential Revision: D29419151
fbshipit-source-id: 7b8296f0a0c84fdcb566ff811f7fcedbe7079189
Summary:
Channel is the general term, and FS is the distinguishing term. Rename
cause to FS, with the option for more specificity later: FUSE vs. NFS
vs. Projected FS.
Reviewed By: xavierd
Differential Revision: D29467592
fbshipit-source-id: 78fe4663f7a9896cf21f7d067bd33ab859ecdec9
Summary:
The enable-tree-overlay is not a top-level config, and thus this code would
never read a boolean value, thus when the config would be read, the
enable_tree_overlay would always be false.
Reviewed By: fanzeyi
Differential Revision: D29494144
fbshipit-source-id: 1537f3e12bf043e154b81e4c0672abf80d54e16c
Summary: This diff connects the trace bus with trace fs command.
Reviewed By: chadaustin
Differential Revision: D29367135
fbshipit-source-id: f9217b286c1a21805d70b21282c10d4ad722a391
Summary: If the handler throws without returning a future, the finish event will not be published. This diff adds the `LiveRequest` to publish finish event in its destructor.
Reviewed By: chadaustin
Differential Revision: D29332452
fbshipit-source-id: 880a4b67ba47b737063a3955c9f4bdbf605f1a43
Summary:
This diff adds tracking for nfs, based on the implementation of that of `FuseChannel`.
Also commented below few questions on the implementation before working on integrating to `strace`.
Reviewed By: chadaustin
Differential Revision: D29279126
fbshipit-source-id: de6bb36dfbe2f550a91f2bf254616bbc639c0c3d
Summary:
This is adds file access logging in the Inode layer of the code. We log the cause (channel, thrift, unknown) and the cause detail (which is the FUSE opcode for fuse, function name for thrift). We do the work on a worker thread since we need to traverse the inode map to translate an inode number to a file path. The workerthread in this code is largely based off of the one in the ProcessNameCache.
At this moment, I am not worrying about logging inode creation as a file access. The nice thing about putting this in FileInode is that it should be able to be used by NFS for free.
I've left a TODO in the code about not logging files that match gitignore patterns since its not a hard blocker for this to land.
Reviewed By: kmancini
Differential Revision: D29128258
fbshipit-source-id: 3e08a3567fed937a381b58847ea83569d70f0ea2
Summary: NfsTaskQueue can be made more generic to be shared across the codebase, so this makes it its own target in `eden/fs/utils` w/ the name EdenTaskQueue.
Reviewed By: xavierd
Differential Revision: D29244762
fbshipit-source-id: 78348f2ff8fa66bc801aefe7d6b3905e0da278e8
Summary: Just to be more specific, lets be explicit about that this is the serverThreadPool that the mount is returning.
Reviewed By: xavierd
Differential Revision: D29244719
fbshipit-source-id: 015a2e8198c418c6fb5a89234274eefb329848fe
Summary: In order to access the kill switch for file access logging, we need to store a EdenConfig pointer in the HiveLogger.
Reviewed By: xavierd
Differential Revision: D29120227
fbshipit-source-id: c828bbbf551c7096877bfa763edc29ef133afb41
Summary: This creates a HiveLogger and threads it through to the ServerState.
Reviewed By: xavierd
Differential Revision: D29119501
fbshipit-source-id: d0b74733d8832d604f8f4cf0af28e767dbddeddf
Summary: This just adds a method to get the repoName out of the EdenMount for logging purposes.
Reviewed By: xavierd
Differential Revision: D29113121
fbshipit-source-id: 9ebb4ac588839a99f25f4e884a7a132f5793e49e
Summary:
See this graph - https://fburl.com/scuba/mononoke_blobstore_trace/77nyw174. It
shows that there was a huge jumps in the number of "hgmanifest" fetches from
scs that didn't exist in the manifold. This is not great for mononoke, because
it uses a significant chunk of our manifold qps capacity (~30K qps on average,
with spikes going up to 100K qps). The main suspect here is
repo_list_hg_manifest method, because few other scs methods (if any) fetch hg
data.
It correlates with the release of new eden version on June, 23, which added
getTreeBatch() method, and looks like it was passing manifest hash and eden id
in the wrong order. See https://fburl.com/code/tsjwss8a
```
proxyHash.revHash(), // this is really the manifest node
```
This is a comment in `getTree()` method which says that `proxyHash.revHash()`
is manifest id, which is exactly what we need to send to scs.
[ScsMetadataImport::getTreeMetadata](https://fburl.com/code/xammvq2k) suggests
that the second parameter should be manifestId (i.e. proxyHash.revHash()), but
we were passing them in the wrong order. Let's fix it.
Reviewed By: xavierd
Differential Revision: D29428581
fbshipit-source-id: d041008f00c7519504c6c67173ca85709e9dc415
Summary:
Since the overlay file contains a small header, we need to make sure to account
for it when calling fallocate, otherwise the file would be created 64-bytes too
small.
Reviewed By: kmancini
Differential Revision: D29382512
fbshipit-source-id: 24c49984b6cf2080f2a5b1fbb4796e4a1806f96a
Summary:
It looks like 20s might not be sufficient anymore as the test is failing too
frequently with `wait` completing without the process having completed:
```
eden/fs/service/test/StartupLoggerTest.cpp:352: Failure
Expected equality of these values:
"exited with status 0"
returnCode.str()
Which is: "running"
eden/fs/service/test/StartupLoggerTest.cpp:363: Failure
Value of: isWritablePipeBroken(fd)
Actual: false
Expected: true
```
Reviewed By: kmancini
Differential Revision: D29342002
fbshipit-source-id: d457b632a2666356e9053d2da66a3227e6524b68
Summary:
In D29079762 (ea2e2f8bbd), globbing was fixed to not match the recursive glob (**) against
the entire path, as this would lead some paths to be matched while they
shouldn't. It however introduced another bug: in some cases, recursive globs
would no longer match paths that should be matched.
To fix both, a partial revert of the original diff is done with a small tweak:
the path that is matched against no longer starts at the root of the
repository. This will prevent `a/b/**/b/c.txt` to match `a/b/c.txt` as
`**/b/c.txt` would only be matched against `c.txt`, and not `a/b/c.txt` like it
was previously.
Reviewed By: fanzeyi
Differential Revision: D29175333
fbshipit-source-id: 1a4137d6f64f6cb77c4be09bd143f72630aa58d5
Summary:
When buck kill fails, eden rm will also fail. This has caused some checkouts
to not be removed when they could be. Stopping aux processes is a nice thing
to do before we unmount. It ensures these processes close file handles in the
mount, but we force unmount anyways so open file handles should not be able to
block the umount call.
Reviewed By: xavierd
Differential Revision: D29205962
fbshipit-source-id: a899940efa5cc1d960cd14a775b7053c34f5d6f2
Summary: This is more confusing than really helpful, thus let's remove the log.
Reviewed By: rkjfb
Differential Revision: D29317007
fbshipit-source-id: 3aba1ab8de7906e193946938aa69b32a09b8e5de
Summary:
Buck v2 builds from the root of the repo, not the current cell. This means that
the inferred logger name ends up being different.
We're going to need to fix this generally because otherwise it'll change logger
names for everyone (I'm tracking this in T93776519), but in the interest of not
having one Eden test arbitrarily failing on Buck v2 let's update this
with a workaround for now.
Reviewed By: genevievehelsel
Differential Revision: D29270388
fbshipit-source-id: 6968d9b6195a5eed7bd4018b161e12d88f78a421
Summary:
I'm suspecting that it is causing some broken pipe errors that aren't
resolving themself.
Reviewed By: kmancini
Differential Revision: D29279304
fbshipit-source-id: cfbf2261f2ac7dd7ec8b3311d1e27a0b9e160de4
Summary:
Invalidation on Windows is tricky, and I got it wrong in subtle ways
previously. The main obvious issue is that when the on-disk invalidation fails,
the refcount shouldn't be decremented as the placeholder/file is still present
on disk. This could cause weird issues in later checkout. The second one is how
invalidating a directory doesn't remove a placeholder (it actually adds one),
and thus we shouldn't decrement the FS refcount. And lastly, the refcount
should be decremented regardless of whether the inode is loaded or unloaded. As
long as it is known by the InodeMap it needs invalidation.
Reviewed By: fanzeyi
Differential Revision: D28970899
fbshipit-source-id: 0d64cadae01fcd4e028c53de9357ece7d648cdd4
Summary:
A user may have some undesirable environment variables when calling `edenfsctl start`,
which we do not want to propagate to edenfs as this may affect EdenFS's
ability to run properly. Having PYTHONPATH set to inside a repository may for
instance lead to a deadlock when EdenFS is trying to setup redirections.
To avoid this, we need to sanitize the environment before calling edenfs. This
functionality already exist but was bypassed on Windows.
Reviewed By: chadaustin
Differential Revision: D29244358
fbshipit-source-id: bc96698732e71412296ed5e28842b59b2c758699
Summary: The original commit broke globbing more than it fixed it. D29175333 will fully fix it, but in the meantime, let's revert the change to get a release out.
Reviewed By: singhsrb
Differential Revision: D29231954
fbshipit-source-id: 7a42e980c6fc4de09bee713a3a4141d52272b6d1
Summary:
Now that the InodeMap is pre-populated with the Overlay, we no longer need the
`folly::kIsWindows` test during checkout as inodes will enter this condition
when unloaded (IsInodeRemembered).
Reviewed By: fanzeyi
Differential Revision: D25106153
fbshipit-source-id: 511d795ae947651e1eaf3c54b8f1ab83c77f5cc4
Summary:
On a non-EdenFS mount, a case change of a file or directory isn't reported as
the file can still be accessed with the same path, and it has the same content
as before. EdenFS had a different behavior whereas renaming a file to a
different case would report a missing file and an untracked one, with different
casing.
This can be surprising to users as the behavior is different from Mercurial,
and it's also hard to actually fix as 2 renames would need to happen, a single
one would not work due to the case insensitivity nature of the filesystem.
While I do believe that reporting the case change might be more desirable to
allow users to actually commit changes the case of files, Mercurial might be
broken in subtle ways today if we allow this, thus it's best to make EdenFS
behave similarly to Mercurial.
Reviewed By: genevievehelsel
Differential Revision: D29150552
fbshipit-source-id: 6cceaa4c9fa61c03f35fcd91a9c01554da252222
Summary:
On case insensitive mounts, updating between commits that have a different
casing for a file/directory would lead to the update failing due to EdenFS
believing that an untracked file is present in the mount. That conflict is
however bogus and EdenFS simply gets confused in
TreeInode::processCheckoutEntry about the entry with the different casing.
To fix this, we should avoid comparing paths in a case sensitive manner and
instead compare then in a case insensitive fashion. This allows the rest of the
checkout code to update the directory/file in place.
On Windows, there is one more subtlety: we can't change the casing of a
file/directory that is already a placeholder and thus we need to force the
entire hierarchy to be removed, this will also make the checkout fail in case
of untracked files in the hierarchy, which is also the behavior on case
sensitive systems.
Reviewed By: genevievehelsel
Differential Revision: D29121741
fbshipit-source-id: 3d2cdacf296a3d061fc828cd6d04d249542cb63f
Summary:
## This diff
* We are migrating `edenfsctl strace` to cpp due to segfaulting from Thrift-py3 (similar to {D25515377 (a152fa4585)}).
* This diff implements new subcmd `edenfsctl trace strace` for this cpp migration.
Reviewed By: xavierd
Differential Revision: D29164534
fbshipit-source-id: 4d8ed23393004f394159c36f71e0c78c077c7c73
Summary:
When running tests, we need a way for Eden to find its privhelper. Normally, it
finds it by looking next to the EdenFS binary itself, but while that works in
Buck 1, it does not work in Buck 2, where the binaries are in separate
directories
Compared to the previous diffs, this requires some extra care because on a real
system, EdenFS is running as a setuid binary, and we don't want to let people
just run whatever binary they want the EdenFS privhelper!
To that end, we reject this argument entirely if the binary is setuid.
Reviewed By: xavierd
Differential Revision: D29061439
fbshipit-source-id: bf9427211d4209cf0bea805b9ea3a53270ec455b
Summary:
In Buck v1 the path we receive for hgPath is absolute so we can use it across
chdirs and the likes.
However, in Buck v2, it isn't. This means that using `--hgPath` doesn't work if
we go and chdir later, which we do because we run from the repo directory.
This updates the unit tests to set the path to an absolute value when they
otherwise look for their hg binary (which they do for operations in HgRepo). If
possible I would have preferred to thread this through to the HgImporter
constructor, but we have quite a few overloaded constructors already and this
turned out to be very messy.
Finally, this also updates the tests themselves to set `hgPath` to `false` so
that if we try to actually run it without configuring it, it won't work.
Reviewed By: xavierd
Differential Revision: D29060828
fbshipit-source-id: 17ad615d7ff98e9ce8278386f018167ec589c336
