Summary: This makes it easier to make DagAlgorithm async.
Reviewed By: sfilipco
Differential Revision: D25345234
fbshipit-source-id: 5ca4bac38f5aac4c6611146a87f423a244f1f5a2
Summary: This will make it easier to use `.await` if part of `dag` becomes async.
Reviewed By: sfilipco
Differential Revision: D25345237
fbshipit-source-id: 7f07cdaa9c2e0468667638066611fabe3a3f7f28
Summary: `impl Trait` does not work with `async_trait`.
Reviewed By: sfilipco
Differential Revision: D25345238
fbshipit-source-id: e7890dbaeb162d44e072ea4428d045004608719b
Summary: This makes it easier to migrate to async.
Reviewed By: sfilipco
Differential Revision: D25345228
fbshipit-source-id: e819f0de5f805377a6977325216ef11b14d68c1d
Summary:
Marking IdConvert Sync makes it possible to be used as a trait object with async-trait.
See https://docs.rs/async-trait/0.1.41/async_trait/#dyn-traits
`dag` uses a lot `dyn DagAlgorithm`. In the future when async is used more, the
trait object will be required to be Send or Sync. Just require it on the trait
to make our life easier.
Marking `IdDagStore` as Send + Sync makes async migration easier.
Reviewed By: sfilipco
Differential Revision: D25345231
fbshipit-source-id: 45b96057907cbe2a1d38fd424e7d4c963dd1b245
Summary: Use async function for the PrefixLookup trait.
Reviewed By: sfilipco
Differential Revision: D24840820
fbshipit-source-id: d22cac9f11b06e3127fa956e3f116cf232214125
Summary: This makes the trait objects slightly easier to use.
Reviewed By: sfilipco
Differential Revision: D24840821
fbshipit-source-id: 22fcdf13b62420302b562c309874e08360d02372
Summary: This makes `dyn IdConvert` include `PrefixLookup`.
Reviewed By: sfilipco
Differential Revision: D24840819
fbshipit-source-id: 8d4e25c534f6e4397ec6f643eb3aa116bff12a2c
Summary:
In the future, when async APIs are used, Python bindings will have lifetime
issues. Make it possible to clone the IdMap so the Python bindings can be made
to work.
Reviewed By: sfilipco
Differential Revision: D24840822
fbshipit-source-id: 6aa4e369c877c428ed39d2cbea79e6943836afa8
Summary: This makes NameSet more friendly for async use-cases, interface-wise.
Reviewed By: sfilipco
Differential Revision: D24806695
fbshipit-source-id: 6e640ba2666872a9128d6460e8b53d6a0e595e56
Summary:
Change the main API of NameSet to async. Use the `nonblocking` crate to bridge
the sync and async world for compatibility. Future changes will migrate
Iterator to async Stream.
Reviewed By: sfilipco
Differential Revision: D24806696
fbshipit-source-id: f72571407a5747a4eabe096dada288656c9d426e
Summary:
This will make it easier to incrementally migrate APIs to async, and make sync
programs able to use sync interface if they'd like to. For example, the dag
crate is likely going to be async-ize. Its async APIs are only useful for
places where the dag is lazy. It's still possible to have a non-lazy dag
that a sync interface is suitable.
Reviewed By: ikostia
Differential Revision: D24777747
fbshipit-source-id: 6d56149fbd1f9b29f5fc62387cbf194800755d12
Summary:
```
warning: attribute should be applied to a function or static
--> pyhgtime/src/lib.rs:70:13
|
70 | #[no_mangle]
| ^^^^^^^^^^^^
71 | static timezone: c_long;
| ------------------------ not a function or static
|
= note: `#[warn(unused_attributes)]` on by default
```
Reviewed By: ikostia
Differential Revision: D25345233
fbshipit-source-id: 4b6f753544af1e3e8479dceed908299f6dc57ad5
Summary: Makes it a bit easier to use.
Reviewed By: markbt
Differential Revision: D25439113
fbshipit-source-id: 2c5da338a7000573ac92435c8982f5adff71bf42
Summary:
I'd like to lower the bar to entry as much as possible to use caching in e.g.
mappings. One thing that's a little annoying to setup right now is stats. Let's
unify those.
This does mean the stats names for mappings that had them will change, but I
think that's probably OK here.
A few notes about the implementation:
- I made the stats objects singletons, because ultimately that *is* what stats
are under the hood.
- I made a macro to define the singleton because that was otherwise really
verbose.
Reviewed By: farnz
Differential Revision: D25461182
fbshipit-source-id: f30d0908c40d8e0010c546ec95a385a06d557757
Summary:
This adds support for replacing things in the small repo with the things from
the large repo. This is useful when changing the bind.
This diff slightly changes how `rsync` is called: now `--source-csid`,
`--target-csid`, `--from-dir` and `--to-dir` are all specified before the
`copy`/`remove-excessive-files` subcommands.
There's still an ability to use this within a repo, as you can pass identical
source and target repos.
Reviewed By: StanislavGlebik
Differential Revision: D25087893
fbshipit-source-id: 6e5881f80d91ef4b794a967cf9f26dd3af7f56c9
Summary:
We had a small bug in blobimport. If mononoke repo is empty and blobimport
tries to import a single revision whose rev number is 0 then it will
successfull import it but it will report that no revision were imported
i.e. it will print "didn't import any revision" to stderr and won't update the
manifold latest imported revision marker.
That was because we didn't update max_rev_and_bcs_id if rev is equal to
max_rev. This diff fixes it.
Reviewed By: ahornby
Differential Revision: D25421164
fbshipit-source-id: 639ead0ac326a14051d3a4faba568ecb797857a2
Summary: See D25396203 for discussion. 100 is better than nothing.
Reviewed By: StanislavGlebik
Differential Revision: D25460398
fbshipit-source-id: 608a2dca9c381c78daf0e7d9bcbd1a32f201030a
Summary:
I had to have two of those while I was refactoring away the old style of
doing get or fill, but now that that's gone, we can have a single one and clean
up.
Reviewed By: StanislavGlebik
Differential Revision: D25396201
fbshipit-source-id: 459e9ec7e44e8b349c585212c2758d64077e56d1
Summary:
All the code that needed it basically gone. Might as well push the compat()
calls a little further down and be done with 0.1 futures here.
Reviewed By: StanislavGlebik
Differential Revision: D25396202
fbshipit-source-id: ae85f61c03cb2c38eabbaf0d45387f9d4422b336
Summary: All the callsites are gone now.
Reviewed By: StanislavGlebik
Differential Revision: D25396205
fbshipit-source-id: 74d0595c4528dc739d254f5dc950157e087b00dd
Summary:
Like it says in the title. The upshot here as well is a lot less cloning.
While in there, I removed the "caching" module since it basically only
contained a couple of things that were all needed in the sql module anyway.
Reviewed By: StanislavGlebik
Differential Revision: D25396208
fbshipit-source-id: aa6381c78f45a94fecd04544196180d2a918f97d
Summary: We need this in Phases, so let's add it there.
Reviewed By: StanislavGlebik
Differential Revision: D25396207
fbshipit-source-id: 34174f205028b95c9aa382c343b1344265391df2
Summary:
Like it says in the title. This reduces cloning. Unlike our cached mappings,
changesets have only way of being queried, so other than cloning, it doesn't
make a huge difference.
Reviewed By: StanislavGlebik
Differential Revision: D25396206
fbshipit-source-id: 45d3ebd403142a3f1d9e3ba7de5de2bf18317165
Summary:
Like it says in the title. I need the new futures module in there later in this
stack so this makes it cleaner.
Reviewed By: StanislavGlebik
Differential Revision: D25396200
fbshipit-source-id: 0148003c83b3dd0da5142eb468cf3a6ae2f74b7a
Summary:
Like it says in the title, this updates bonsai_hg_mapping to a futures 0.3
implementation of get_or_fill.
The upshot is that this requires less cloning (in fact, no cloning at all if
the rest of the code was 0.3 here), and in this particular instance it also
lets us completely get rid of the `from_bonsai` flag we were threading through
the whole method and checking many times.
Reviewed By: StanislavGlebik
Differential Revision: D25396199
fbshipit-source-id: f8126c96aad8d982c3deb535530484bec841929f
Summary:
Like it says in the title. Let's update this to the new get_or_fill
implementation that uses 0.3 futures.
Reviewed By: StanislavGlebik
Differential Revision: D25396204
fbshipit-source-id: 06bf449a0d15bd19625acfdcbb4578948e57cde7
Summary:
Like it says in the title, this adds a futures 0.3 variant of
GetOrFillMultipleFromCacheLayers. However, I didn't just port the code as-is,
since the code as it stands wouldn't have been very idiomatic if I did.
Instead, I refactored this to be a function and a few traits. I've also kept
the old code for now, and I'll remove it once I've converted all the callistes.
The upshot of the proposed refactor here is that it should be easier to use
this without having to heavily duplicate the instantiation of the "cacher" in
places where we have multiple variants that are cached (e.g. mappings), all the
while being able to leverage the type checker. See D25334478 (13255301b0) for a discussion
on this. This new approach also makes it much easier to work with the tests for
this (you can just mutate the store and access its fields).
Reviewed By: StanislavGlebik
Differential Revision: D25396203
fbshipit-source-id: d706729a800faa4b12fcf5e608c6dee93c5a909e
Summary: Something is wrong with this which causes Unity to freak out.
Reviewed By: fanzeyi
Differential Revision: D25453230
fbshipit-source-id: 89f61fd97817403fa65071ddac022a226b775e53
Summary:
We already have a config to write shared history to an indexedlog. Let's
add a similar config for local history.
Reviewed By: quark-zju
Differential Revision: D23910457
fbshipit-source-id: eca21db0350895f573a60e4e1a6b05514e70f1c7
Summary:
In a future diff we'll use the indexedlog stores for local history. We
want those to exist forever, so let's move IndexedLogHgIdHistoryStore to use a
Store under the hood, and add an enum for distinguishing between the two types
at creation time.
Reviewed By: quark-zju
Differential Revision: D25429675
fbshipit-source-id: 5f2dc494e1175d4c1dc74992d3311d2e55d784ca
Summary: Switching to specify derived data types other than hg explicitly on the command line
Reviewed By: farnz
Differential Revision: D25367323
fbshipit-source-id: 0e0aea1aab46b43b325486ed6161ea322f7cec4b
Summary: It's useful sometimes, like in the next diff.
Reviewed By: mitrandir77
Differential Revision: D25422597
fbshipit-source-id: 0ebb5dcc349bbaacac3dddf03f19e5e092042468
Summary:
This is useful data to see how much we are over the limit. I noticed
the missing data while resolving
https://fb.workplace.com/groups/scm/permalink/3450652774984318/ as the oncall.
Reviewed By: StanislavGlebik
Differential Revision: D25414427
fbshipit-source-id: ec4bbca9c21a4bf0e675ec1cd82e4e703cd88631
Summary: Previously, the EdenAPI client (and Mercurial's `http-client`) required both a client certificate and private key to be specified individually in order to use TLS mutual authentication. However, libcurl supports having both the certificate and key concatenated together into a single PEM file. This diff makes it possible to simply specify the combined file as the `cert`, leaving the `key` unset.
Reviewed By: quark-zju
Differential Revision: D25323786
fbshipit-source-id: 1800be3ef82ec4dfa89d725f5860190172994c89
Summary: Treat empty paths as `None`, which allows certificate and key paths to be unset via a `--config` flag (e.g. `hg debughttp --config auth.edenapi.key=`). This would normally require adding an `%unset` line to the appropriate hgrc, which adds friction to ad-hoc command line usage.
Reviewed By: quark-zju
Differential Revision: D25416531
fbshipit-source-id: 15aae78d9b3a82227278f33def45fa960aa65a98
Summary:
The f64 compatibility added by D25079001 (e91552fefe) changes the `{rev}` template output.
However, the revset object is not aware of the rev mapping so the following
pattern will stop working:
ifcontains(rev, revset('.'), '@', 'o')
# rev: mapped, but not contains in `revset('.')`
Fix it by teaching `ifcontains(a, b, ...)` to disable f64 compatibility when
evaluating `a`, since `a` is not going to be printed out.
This fixes an issue in VSCode ISL that "You are here" is missing on non-master
commits.
Reviewed By: singhsrb
Differential Revision: D25404673
fbshipit-source-id: 3e53a2ce1f135f8825c195c5a3061dad0359c4b2
Summary: In addition to tracking profile activation, it would be nice to see when people deactivate profiles.
Reviewed By: kmancini
Differential Revision: D25372256
fbshipit-source-id: e6bfefe3c838465ba9ed10d622342bdf3df1c40a
Summary: It would be interesting to track which profiles are activated most often. Also, it may be helpful to track which users are using this feature and generally track usage.
Reviewed By: kmancini
Differential Revision: D25372255
fbshipit-source-id: 2107cdce2a997180547dc63ec654c8d72ced5eb4
Summary:
The code still took a dependency on Mercurial's old manifest code to parse
manifests. It turns out the manifests have a very simple format that we could
parse directly.
This avoids various copies, conversions, std::list, removes ~1k lines of code,
at the expense of adding ~100 lines of code (some of them being C++
boilerplate).
Reviewed By: fanzeyi
Differential Revision: D25385018
fbshipit-source-id: 90d4cda2b7797584bc48c086d5592a7ecaa05dfc
Summary:
We already have a config to write shared data to an indexedlog. Let's
add a similar config for local data.
Reviewed By: xavierd
Differential Revision: D23909569
fbshipit-source-id: 87317554beb6bef8237e6a900403701662c3c0d0
Summary:
We temporarily dropped repair support when transitioning to using Store instead
of a raw RotateLog. Let's add that back now.
Reviewed By: xavierd
Differential Revision: D25371622
fbshipit-source-id: e28fc425a6ffb50c93540672b0df75a172ebbe9c
Summary:
In a future diff we'll use the indexedlog stores for local data. We
want those to exist forever, so let's move IndexedLogHgIdDataStore to use a
Store under the hood, and add an enum for distinguishing between the two types
at creation time.
Reviewed By: xavierd
Differential Revision: D23915622
fbshipit-source-id: 296cf6dfcd53e5cf1ae7624fdccedf0a60a77f22
Summary:
In a future diff we'll be moving the IndexedLogHgIdDataStore to use the Store
type (that hides the differences between IndexedLog and RotateLog). To do so, it
needs to support repairs. Let's do some minor refactoring to enable this.
Reviewed By: xavierd
Differential Revision: D25371623
fbshipit-source-id: 846cb5f8c21f1e6b550a45259cc8da24cc65b13b
Summary:
Same as development branch. Without configuration changes, nothing changes for
the production codepath.
Reviewed By: quark-zju
Differential Revision: D25405026
fbshipit-source-id: aff705aa5f96814f1f1d7552454ab1d0c13afd92
Summary:
The end goal is to have clients using a sparse IdMap. There is still some work
to get there though. In the mean time we can test repositories that don't use
any revlogs. The current expections for those repositories are that they have
a full idmap locally.
Reviewed By: quark-zju
Differential Revision: D25075341
fbshipit-source-id: 52ab881fc9c64d0d13944e9619c087e0d4fb547c
Summary:
The client dag cannot currently be instantiated with a sparse idmap (aka
universal commit idmap). Is should be usable with a full idmap. To test
repositories that use segmented changelog exclusively we add the capability of
cloning the full idmap.
I currently see StreamCloneData as an experiment. I am open to suggestions
around what structure we should have for the regular long term clone endpoint.
That said, I am leaning towards converting clone_data to return
StreamCloneData. Overall, Segmented Changelog has a few knobs that influence
how big the IdMap ends up being so the code that is more flexible will be more
useful long term. To add to that, we transform data higher in the stack using
streaming and this data does similar fetching, it seems that we should have a
stream idmap exposed by clone_data.
Reviewed By: quark-zju
Differential Revision: D24966338
fbshipit-source-id: 019b363568e3191280bd5ac09fc15062711e5523
Summary:
It uses the Python iterator -> Rust stream code path, which can might
cause deadlock with async_runtme::block_on_exclusive. Avoid running
main Python logic in multiple threads.
The prefetching is only useful for lazy commit text backend, which hasn't been
rolled out yet. So disabling it won't cause pref regressions.
Note: This should not affect Phabricator Status prefetching.
Reviewed By: sfilipco
Differential Revision: D25350916
fbshipit-source-id: 49e9c4fea5ff48e1f579aa602693d38c7dd96fc7
Summary:
From wez's description (slightly edited):
a symlink direction resolves outside the repo, and the new validation logic doesn't like that. the test is doing a redirect add bind then redirect add symlink. The second add is intended to implicitly delete the first one if the config is different, but it's not smart enough to realize that that is happening.
This adds an explicit `redirect del` before we do our second `redirect add`
Reviewed By: xavierd
Differential Revision: D25405011
fbshipit-source-id: 107fd272bbe830d8b23e437286ced00460902d91
Summary:
All the bookmarks is *a lot* of bookmarks. Don't do them all at once. Also, add
some logging output so we can tell how far along we are.
Reviewed By: HarveyHunt
Differential Revision: D25397297
fbshipit-source-id: c19b99123f88e05e99bff61e2399a62d378a6671
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
