Summary: Also added a TryShared future to futures_ext. The problem with regular Shared is that if you want to share anyhow::Result the Error part of it is not cloneable. This TryShared will work nicely when returning anyhow::Result, which most of our code does.
Reviewed By: aslpavel
Differential Revision: D25223317
fbshipit-source-id: cf21141701884317a87dc726478dcd7a5a820c73
Summary:
This commit adds a new eden configuration option that
controls whether we try to load our edenfs.kext in preference to
an alternative fuse implementation on macOS.
The majority of this diff is plumbing to convey the configuration
value through to the privhelper, which is relatively restrictive
due to its root-ness.
I've also updated watchman and mercurial to be aware of the new
filesystem type that shows up in the mount table.
Reviewed By: genevievehelsel
Differential Revision: D25065462
fbshipit-source-id: 4f35b9440654298e2706a0d0613d97eb63451999
Summary:
Like it says in the title. This is helpful to measure the number of SQL queries
we make. This required actually threading in a CoreContext, which we didn't
have before.
Reviewed By: StanislavGlebik
Differential Revision: D25336069
fbshipit-source-id: 35677c55550e95b5126de29c2a824b4eda32092c
Summary: Like it says in the title.
Reviewed By: StanislavGlebik
Differential Revision: D25336068
fbshipit-source-id: 113050215c28a28c820d938348a0a3e54c14c3ee
Summary:
Like it says in the title, this adds a caching layer around Globalrevs using
our existing `GetOrFillMultipleFromCacheLayers` abstraction.
Note: I've opted to not track dedicated metrics for this (compare to the hg
mapping to see them), since I don't believe we really ever look at them.
I'd like to do a little bit of refactoring of
`GetOrFillMultipleFromCacheLayers` to a) track them without having to ad-hoc
code it, b) convert it 0.3 futures, and c) require less ceremony to call it.
However, I'll do so in another diff.
Reviewed By: StanislavGlebik
Differential Revision: D25334478
fbshipit-source-id: 1385298b8fdf1cd081b6e509c6c3e03b3abbfa5b
Summary: This lib.rs is getting too big. Split it.
Reviewed By: StanislavGlebik
Differential Revision: D25333510
fbshipit-source-id: ea15664d2de21a24ee107162e030b7762b1d413e
Summary:
I'd like to add a caching variant for this. Might as well not have to rewrite
those methods on an ad-hoc basis.
Reviewed By: StanislavGlebik
Differential Revision: D25333461
fbshipit-source-id: 632c0307189fe15a926d808c1eeca1e3f240eb19
Summary: Like it says in the title.
Reviewed By: StanislavGlebik
Differential Revision: D25333450
fbshipit-source-id: 49ad4b1964a4dfd9f3e0108fa421d451ef905256
Summary: If response is not HTTP/1.1 or isn't Weboscket upgrade we should report error.
Reviewed By: HarveyHunt
Differential Revision: D25368729
fbshipit-source-id: 62dcd93240902924312f4de2965f58357c46c98b
Summary:
Further down the following check is executed:
priority > bestpriority
When priority are strings, this leads to wrong expectations:
$ python3
>>> a = "90"
>>> b = "100"
>>> a > b
True
Reviewed By: krallin
Differential Revision: D25334682
fbshipit-source-id: 48287d57ae6a938e9f2619babf49d073534f46d7
Summary:
Now that we don't ship Python 2 rpms, let's remove the build and test
targets.
Reviewed By: singhsrb, xavierd
Differential Revision: D25313325
fbshipit-source-id: 876385ccb6cb7675fef8d93978b372a3085764b0
Summary:
if you have no cache to destroy `kdestroy` will not succeed
and thus the `kinit` portion won't run.
Reviewed By: genevievehelsel
Differential Revision: D25340727
fbshipit-source-id: f20d882b95face17c5ee4b0b5b1b1267fd4bb2c8
Summary:
T53602763 is about hg shelve and other similar
move-off-then-back-on-a-commit situations to get reported incorrectly to
watchman on eden checkouts. This can cause issues with buck and other tools.
The bug is in Eden, but let's try to save people time debugging by telling them
of the risk.
Reviewed By: fanzeyi
Differential Revision: D25337932
fbshipit-source-id: e293c43b5c87bea26564e1efd45b7a983862a442
Summary:
This makes logs go through a `Drain` which queries `ObservabilityContext` (introduced in a previous diff) for current logging level. ATM I did not add any tests, and it's pretty easy to add a unit-test checking that the drain indeed respects the level, but it's so simple that I am not 100% convinced that test would be all that valuable.
Note that currently `ObservabilityContext` is enabled in a `Static` variation.
Reviewed By: mitrandir77
Differential Revision: D25232400
fbshipit-source-id: 7499916e0a3ddab43538343e6ed215818517eaf7
Summary:
`ObservabilityContext` is a structure that helps logging facilities within Mononoke to make logging decisions. Specifically, the upcoming `DynamicLoggingDrain` and already existing `MononokeScubaSampleBuilder` will have this structure as a component and decide whether a particular logging statement (slog or scuba) should go ahead.
Internally, `ObservabilityContext` is a wrapper around data received from a [configerator endpoint](https://www.internalfb.com/intern/configerator/edit/?path=scm%2Fmononoke%2Fobservability%2Fobservability_config.cconf).
This diff makes a few unobvious decisions about how this is organized. My goals were:
1. to have production (i.e. reading from configerator), static (i.e. emulating current prod behavior) and test variants of `ObservabilityContext`
1. to avoid having consumers know which of these variants are used
1. to avoid making all consumers of `ObservabilityContext` necessarily generic
1. to avoid using dynamic dispatch of `ObservabilityContext`'s methods
Points 3 and 4 mean that `ObservabilityContext` cannot be a trait. `enum` is a common solution in such cases. However, if `ObservabilityContext` is an `enum`, consumers will know which variant they are using (because `enum` variants are public). So the solution is to use a private enum wrapped in a struct.
Reviewed By: mitrandir77
Differential Revision: D25287759
fbshipit-source-id: da034c71570137e8a8fb7749b1e4ad43be482f66
Summary:
A user ran into an issue where some buck/build related automation had
added a redirection named `../../../Users/username/reponame/.lsp-buck-out`.
The valid name for this is `.lsp-buck-out` because redirections must always be
repo-root-relative.
This commit adds missing validation for this, and makes it slightly more
convenient for consuming tools to work with by allowing an absolute path to the
redirection to be specified.
Why not simply resolve the path from the cwd and use that? The command has
always been defined in terms of repo-root-relative paths, and there are a
number of cases where CI and other build tools can start in unexpected
subdirectories and that means that paths like `../buck-out` can have an
ambiguous or surprising resolution. There are tools that follow the
documentation and correctly specify repo-root-relative paths that would either
be broken outright or left with newly ambiguous behavior if we suddenly started
to accept and resolve relative paths for ourselves.
I'm not opposed to changing the behavior in that way in the future, but it
requires a bit more of a coordinated effort than I have time to wrangle right
now, so this diff is aimed at surfacing breakage rather than magically
resolving it.
This commit fixes up the behavior so that paths like `../foo` are explicitly
checked for and generate an error, but absolute paths like
`/Users/username/reponame/foo` (which are unambiguous) are now resolved to the
appropriate repo-root-relative path automatically.
That makes it a bit easier for consuming tools to swing through a behavior
change; they can now simply pass the absolute path rather than messing around
with trying relativize the paths to the repo root.
Reviewed By: kmancini
Differential Revision: D25261490
fbshipit-source-id: 1706b54fc15c2dad334cdf6c75cca5e6e44ed97a
Summary:
A while back, we saw that concurrent directory creation would lead to EdenFS
being confused and failing to record some of the created directories. This then
caused EdenFS to no longer being in sync with what was on disk. To handle this
case, we've had to manually creating these directories recursively.
What I didn't realize at the time was that these concurrent notifications could
also happen on removal this time, and if a directory removal notification wins
the race against the removal of its last children, that directory wouldn't be
removed and EdenFS would once again be confused about the state of the
repository.
Fixing this is a bit trickier than directory creation as it's more racier.
Consider a directory that is being removed, and then immediately recreated with
a file in it in a different process. The naive approach of simply force
removing all of the children of a directory when handling the removal
notification would clash with the file creation. We could argue that nobody
should be doing this, but there would be an unhandled race, and thus a bug
where data would potentially be lost[0].
We can however fix this bug slightly differently. For file/directory removal,
we can actually hook onto the pre-callback, ie: one that happens before the
file/directory is no longer visible on disk. This inherently eliminate the race
altogether as the callback will be guaranteed to run when none of its children
are present, and if a race happens with a file creation in it, we can simply
fail the removal properly.
The only tricky bit is for the renaming logic, as renaming a file is logically
a removal followed by a creation. For that reason, I've moved part of the
renaming bits to the pre-callback too.
In theory, this change may negatively affect workloads that do concurrent
directory removal as the duration during which a file/directory is visible
ondisk now includes the EdenFS callback while it didn't before. Such workflows
should be fairly rare and/or redirected to avoid EdenFS altogether if
performance matters.
[0]: This left-over file that EdenFS wouldn't be aware of would also later
cause the checkout code to fail due to invalidation failures triggered when
trying to invalidate that directory. This would be fairly hard to debug.
Reviewed By: fanzeyi
Differential Revision: D25112381
fbshipit-source-id: 9300499ce872ad93d0a687f0e61b7e2a9caf9556
Summary:
On Windows, the FS refcount is used to indicate that ProjectedFS knows about
this inode and either has a placeholder on disk, or a plain file. The first
event only occurs on lookup (similarly to Linux/macOS), while the second one
happens when files are created by the user and we receive a notification about
it.
In order to avoid races and to miss necessary invalidation, the refcount has to
be incremented after the placeholder has been created, and the refcount is
decremented before the invalidation is performed. This is straightforward to
achieve for notifications, but requires passing a callback to the PrfsChannel.
Reviewed By: chadaustin
Differential Revision: D24800819
fbshipit-source-id: 0e7ea7ed3a9ca0414e3e727fba975045546d82d1
Summary:
On Windows, the refcount will be increased when a placeholder is written on
disk, and decremented when it is invalidated. Since the invalidation completely
clears the inode on disk, we need to make sure we don't keep an inode loaded
after it is invalidated. For that reason, let's just allow the refcount to be
either 0 or 1.
Reviewed By: chadaustin
Differential Revision: D24764567
fbshipit-source-id: 5d09fb9f53bf36d517d0bfb083107f176c33c2a7
Summary: Can just pass on the iterator
Reviewed By: ikostia
Differential Revision: D25216892
fbshipit-source-id: 79c08737477ac7ed1f824c50105d5977ee592126
Summary: Its now the default for this binary, might as well shorten the test command lines
Reviewed By: ikostia
Differential Revision: D25219717
fbshipit-source-id: 8074145c6f05f26ab7fa18d2ff399482ad592885
Summary: Reduces boilerplate for binaries usually run in this mode, notably the walker
Reviewed By: ikostia
Differential Revision: D25216883
fbshipit-source-id: e31d2a6aec7da3baafd8bcf208cf79cc696752c0
Summary: This is useful to prevent accidentally consuming too much. Enabled it for the walker
Reviewed By: ikostia
Differential Revision: D25216880
fbshipit-source-id: e80f490d6ece40d64cc8609e7d6b80d0ecbb1671
Summary: Reduces boiler plate on command line for binaries like walker that want different default
Reviewed By: krallin
Differential Revision: D25216876
fbshipit-source-id: 0df474568d28e0726be223e9dc0a760523063d21
Summary: Darkisilon cell consists of multiple hosts which shares underlying storage, so write to one of them is visible for all hosts. Lets spread requests between all these hosts. I'll get list of hosts from the smc tier and will randomly connect to one on each request.
Reviewed By: krallin
Differential Revision: D25163782
fbshipit-source-id: b28085dd37b15972469b7334a47def473e10f34e
Summary: This allows us to be more flexible in choosing authentication and expands variables used in configuration.
Reviewed By: singhsrb
Differential Revision: D25304008
fbshipit-source-id: 636893a9eaec31ca5acfa02f72931d5e56b695d0
Summary:
Update pywatchman/bser.c to define `PY_SSIZE_T_CLEAN` before including
`Python.h` and then receive parsed string lenghts as `Py_ssize_t` rather than
`int`. This eliminates some runtime Python warnings about the old `int`
behavior being deprecated.
Reviewed By: chadaustin
Differential Revision: D25294778
fbshipit-source-id: 7db678ed918db3bf4745ba716585ed6475d1c910
Summary:
EdenFS can now show notifications to the user in case something wrong is
happening.
Reviewed By: chadaustin
Differential Revision: D24864354
fbshipit-source-id: fabc30f14bc022b4367af562481235fe984df458
Summary:
One of the main sub-par user experience on Windows is the lack of notification
of any kind when EdenFS can't reach the Mercurial servers. Prior to this diff,
the callbacks would never return, causing commands to simply hangs for the
user.
As a first step, let's add a timeout, a later step will hook the notification
mechanism used on macOS/Linux to display a notification when timeouts occurs.
The only callback that doesn't have a proper timeout is the notification one,
as timing out on these would mean that EdenFS won't have registered that some
files/directories have been materialized which will lead to inconsistencies
later.
Reviewed By: kmancini
Differential Revision: D24809645
fbshipit-source-id: 0ddd9d443a17db405a3edbaa8edecf3764c31d37
Summary:
As described in the previous diff, unmounting a repo while a request is pending
would lead to a use after free. To solve this, we can wrap the inner channel
with a shared_ptr, and set it to NULL whenever unmount is in progress.
While this solution has a fairly large overhead due to requiring at least 2
atomics per callbacks (one for the lock, the second one for the shared_ptr
copy), it is correct. A future improvement will swap these with an RCU pointer
to reduce the callback cost to almost nothing.
Reviewed By: chadaustin
Differential Revision: D25071423
fbshipit-source-id: 77d14a38403bef3e276d3e5e48e6fd95dd641964
Summary:
There is currently a race condition where unmounting a repo can happen
concurrently with a ProjectedFS notification/callback. Depending on who wins
that race, this can lead to a use-after-free as the PrjfsChannel/EdenMount
would be freed but the callback would still have reference to it.
To solve this, we need to keep track of inflight requests, and in particular
make sure that memory isn't freed before all the pending callbacks have
completed. And that effectively means that we need to refcount the channel used
by these callbacks so we only free the memory when nobody else is using it.
The first step towards this is splitting the channel in 2 halves.
Reviewed By: chadaustin
Differential Revision: D25071422
fbshipit-source-id: 743f38c9b19ba534961d06ea6f2ddc96b685fe19
Summary: convert blobrepo:blobrepo_common to new type futures
Reviewed By: StanislavGlebik
Differential Revision: D25245426
fbshipit-source-id: d3db0e417545b2b0043e48a536737586005ac4bb
Summary:
I'd like to experiment with splitting this into its own service. To do that, I
don't want to have to include a path, since it's only used for reporting an
error that will never occur (because for that service I'll be using the
"generate" variant of the filenode id). Let's just make it optional.
Reviewed By: lukaspiatkowski
Differential Revision: D25220901
fbshipit-source-id: 6d3cf70a63b077de18a7d43f3b65766b453c425e
Summary: Like it says in the title. Let's turn this into an async fn.
Reviewed By: lukaspiatkowski
Differential Revision: D25220902
fbshipit-source-id: b5de783adaca05919eb5cd6858c8b0aaf03ddfc2
Summary:
This returns a Result of a tuple, but:
- This never errs.
- Nothing ever reads the left side of the tuple.
So let's stop doing that.
Reviewed By: StanislavGlebik
Differential Revision: D25219887
fbshipit-source-id: f33dcf6f6e68cb17b40c4638470312afae0662e6
Summary:
Like it says in the title. We've had this comment about potentially using this
for a couple of years now. It seems a bit unlikely at this point that we're
going to use this, but currently it makes the code that provides uploading hg
entries more complex than it needs to be, so let's just get rid of this.
Reviewed By: lukaspiatkowski, StanislavGlebik
Differential Revision: D25219728
fbshipit-source-id: 61d254edef16d24a4c29f96f983f894863b5232a
Summary:
This modifies our object popularity mechanism to account for the size of the
objects being downloaded. Indeed, considering our bottleneck is bandwidth, we
forcing similar routing for 10 downloads of a 10MB object and 10 downloads of a
1GB object doesn't make that much sense.
This diffs updates our counting so that we now record the object size instead
of a count. I'll set up routing so that we disallow consistent routing when a
single object exceeds 250MiB/s of throughput ( = 1/4th of a task).
It's worth noting that this will be equivalent to what we have right now for
our most problematic objects (GraphQL schemas in Fbsource, 35M each), given
that we "unroute" at 150 requests / 20 seconds (`150 * 35 / 20 = 262`).
The key difference here is that this will work for all objects.
This does mean LFS needs to cache and know about content metadata. That's not
actually a big deal. Indeed, over a week, we serve 25K distinct objects
(https://fburl.com/scuba/mononoke_lfs/a2d26s1a), so considering content
metadata is a hundred bytes (and so is the key), we are looking at a few MBs of
cache space here.
As part of this, I've also refactored our config handling to stop duplicating
structures in Configerator and LFS by using the Thrift objects directly (we
still have a few dedicated structs when post-processing is necessary, but we
no longer have anything that deserializes straight from JSON).
Note that one further refinement here would be to consistently route but to
more tasks (i.e. return one of 2 routing tokens for an object that is being downloaded
at 500MiB/s). We'll see if we need that.
Reviewed By: HarveyHunt
Differential Revision: D24361314
fbshipit-source-id: 49e1f86cf49357f60f1eac298a753e0c1fcbdbe5
Summary:
D24761026 (caa684450f) formatted the default cachelib size with the specifier
`{:3}`. This specifier pads the left side of the string with spaces if there
are less than 3 digits.
Unfortunately, this means that attempting to parse the string into an `f64`
fails. Here's a minimal example:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=11a1e819f9919f7d02565cb8fa561b85
Remove the format specifier and instead call `.to_string()`.
Reviewed By: ahornby
Differential Revision: D25302079
fbshipit-source-id: 461dd628a312a967f6cf5958d2e5d51b72b0ffd8
Summary:
Right now this is the same as our Filestore threshold so we always attempt to
compress Filestore chunks even though they were designed to fit in cache.
Whoops.
Reviewed By: ikostia
Differential Revision: D25274080
fbshipit-source-id: f17b54710fc36ca7c11c74247d038bf73777f7f9
Summary:
Cachelib wasn't initialized so a production configuration would crash because
the cache pools wouldn't be found.
Reviewed By: singhsrb
Differential Revision: D25291880
fbshipit-source-id: 382db0efda072b11da587e863566e816bd5393ca
Summary: Convert `ChangesetFetcher` to new type futures
Reviewed By: StanislavGlebik
Differential Revision: D25244213
fbshipit-source-id: 4207386d81397a930a566db008019bb8f31bf602
Summary:
The `repo.names` namespace lookup should not return `[None]`. Detect and avoid
that to avoid crashes.
Reviewed By: singhsrb
Differential Revision: D25261462
fbshipit-source-id: c7a0bba557717ab292a05d753569c68112e7f0b0
Summary:
If skeleton manifests are not enabled for a repository, ignore the tunables
that delay casefolding checks until land-time.
Reviewed By: StanislavGlebik
Differential Revision: D25271607
fbshipit-source-id: dcaf9291da31d0f57b3b632888ed688ecd6cecda
Summary:
Create `BlobstoreMapping` as a trait with the common implementations for
derived data mappings that are stored in the blobstore.
Reviewed By: StanislavGlebik
Differential Revision: D25099915
fbshipit-source-id: 8a62fbb809918045336944c8cd3584b109811012
Summary:
Batch derivation was disabled as it caused problems for other derived data types.
This was because the default batch implementation was wrong: it attempted to derive
potentially linear stacks concurrently, which would cause O(n^2) derivations.
Fix the default implementation to be a naive sequential iteration, and re-enable
batch derivation for fsnodes and skeleton manifests.
Reviewed By: StanislavGlebik
Differential Revision: D25218195
fbshipit-source-id: 730555829f092cc36e1c81cf02c2b4962a3904da