Summary:
Let's use new option in CLI. Unfortunately we can't easily accept commit ids in
named params so it has to be a postional one.
Differential Revision: D22234412
fbshipit-source-id: a9c27422fa65ae1c42cb1c243c7694507a957437
Summary:
If anything were to go wrong, we'd be happy to know which puts we ignored. So,
let's log them.
Reviewed By: farnz
Differential Revision: D22356714
fbshipit-source-id: 5687bf0fc426421c5f28b99a9004d87c97106695
Summary:
Eventually, I plan to make this the default, but for now I'd like to make it
something we can choose to turn on or off as a cmd argument (so we can start
with the experimental tier and Fastreplay).
Note that this mixes volatile vs. non-volatile pools when accessing the pools
for cacheblob. In practice, those pools are actually volatile, it's just that
things don't break if you access them as non-volatile.
Reviewed By: farnz
Differential Revision: D22356537
fbshipit-source-id: 53071b6b21ca5727d422e10f685061c709114ae7
Summary:
I canaried this on Fastreplay, but unfortunately that showed that sometimes we
just deadlock, or get so slow we might as well be deadlocked (and it happens
pretty quickly, after ~20 minutes). I tried spawning all the `get()` futures,
and that fixes the problem (but it makes gettreepack noticeably slower), so
that suggests something somewhere is creating futures, polling them a little
bit, then never driving them to completion.
For better or worse, I'd experienced the exact same problem with the
ContextConcurrencyBlobstore (my initial attempt at QOS, which also used a
semaphore), so I was kinda expecting this to happen.
In a sense, this nice because I we've suspected there were things like that in
the codebase for a while (e.g. with the occasional SQL timeout we see where it
looks like MySQL responds fast but we don't actually poll it until past the
timeout), and it gives us a somewhat convenient repro.
In another sense, it's annoying because it blocks this work :)
So, to work around the problem, for now, let's spawn futures to force the work
to complete when a semaphore is held. I originally had an unconditional spawn
here, but that is too expensive for the cache-hit code path and slows things
down (by about ~2x).
However, having it only if we'll query the blobstore isn't not as expensive,
and that seems to be fine (in fact it is a ~20% p99 perf improvement,
though the exact number depends on the number of shard we use for this, which I've had to tweak a bit).
https://pxl.cl/1c18H
I did find what I think is one potential instance of this problem in
`bounded_traversal_stream`, which is that we never try to poll `scheduled` to
completion. Instead, we just poll for the next ready future in our
FuturesUnordered, and if that turns out to be synchronous work then we'll just
re-enqueue more stuff (and sort of starve async work in this FuturesUnordered).
I tried updating bounded traversal to try a fairer implementation (which polls
everything), but that wasn't sufficient to make the problem go away, so I think
this is something we have to just accept for now (note that this actually has
some interesting perf impact in isolation: it's a free ~20% perf improvement on
p95+: https://pxl.cl/1c192
see 976b6b92293a0912147c09aa222b2957873ef0df if you're curious
Reviewed By: farnz
Differential Revision: D22332478
fbshipit-source-id: 885b84cda1abc15c51fbc5dd34473e49338e13f4
Summary: Like it says in the title. Those are useful!
Reviewed By: farnz
Differential Revision: D22332479
fbshipit-source-id: f9bddad75fcbed2593c675f9ba45965bd87f1575
Summary:
The goal of this blobstore is to dedupe reads by waiting for them to finish and
hit cache instead (and also to dedupe writes, but that's not relevant here).
However, this is not a desirable feature if a blob cannot be stored in cache,
because then we're serializing accesses for no good reason. So, when that
happens, we store "this cannot be stored in cache", and we release reads
immediately.
Reviewed By: farnz
Differential Revision: D22285269
fbshipit-source-id: be7f1c73dc36b6d58c5075172e5e3c5764eed894
Summary:
I'm going to store things that aren't quite the exact blobs in here, so on the
off chance that we somehow have two caching blobstores (the old one and this
one) that use the same pools, we should avoid collisions by using a prefix.
And, since I'm going to use a prefix, I'm adding a newtype wrapper to not use
the prefixed key as the blobstore key by accident.
Differential Revision: D22285271
fbshipit-source-id: e352ba107f205958fa33af829c8a46896c24027e
Summary:
This introduces a caching blobstore that deduplicates reads and writes. The
underlying motivation is to improve performance for processes that might find
themsleves inadvertently reading the same data concurrently from a bunch of
independent callsites (most of Mononoke), or writing the same bit of data over
and over again.
The latter is particularly useful for things like commit cloud backfilling in
WWW, where some logger commits include the same blob being written hundreds or
thousands of times, and cause us to overload the underlying Zippy shard in
Manifold. This is however a problem we've also encountered in the past in e.g.
the deleted files manifest and had to solve there. This blobstore is a little
different in the sense that it solves that problem for all writers.
This comes at the cost of writes being dropped if they're known to be
redundant, which prevents updates through this blobstore. This is desirable for
most of Mononoke, but not all (notably, for skiplist updates it's not great).
For now, I'm going to add this behind an opt-in flag, and later on I'm planning
to make it opt-out and turn it off there (I'm thinking to use the CoreContext
for this).
Reviewed By: farnz
Differential Revision: D22285270
fbshipit-source-id: 4e3502ab2da52a3a0e0e471cd9bc4c10b84a3cc5
Summary: This allowed me to compare two alternative approaches to queue draining, and generally seems like a useful thing to do.
Reviewed By: krallin
Differential Revision: D22364733
fbshipit-source-id: b6c76295c85b4dec6f0bfd7107c30bb4e4a28942
Summary: It's useful to derive all enabled derived data at once
Reviewed By: krallin
Differential Revision: D22336338
fbshipit-source-id: 54bc27ab2c23c175913fc02e6bf05d18a54c249c
Summary:
We've recently added an option to perform a stack move in megarepolib. A "stack
move" it's a stack of commits that move a files according to a mover. Now let's
expose it in the megarepotool
Reviewed By: ikostia
Differential Revision: D22312486
fbshipit-source-id: 878d4b2575ed2930bbbf0b9b35e51bb41393e622
Summary:
Printing it can take too much time.
Use a larger threshold than `list` or `set`, since `hg` commonly uses a dict
for command line options, and that can exceeds 8 items.
As we're here, also fix the traceback order so it's "most recent call last".
Reviewed By: kulshrax
Differential Revision: D22362003
fbshipit-source-id: 3d2f4664bec6b4cfaf42b8e5d2fc47b0f3d96411
Summary: It was moved by D22305173 (fdba0b98c2). Skip testing it.
Reviewed By: kulshrax
Differential Revision: D22364199
fbshipit-source-id: 3e205daa5aac517020664005a6f95d0292674bc3
Summary:
In order to do what the title says, this diff does:
1. Add the `eden/oss/.../third-party/rust/.../Cargo.toml` files. As mentioned in the previous diff, those are required by GitHub so that the third party dependencies that are local in fbsource are properly defined with a "git" dependency in order for Cargo to "link" crates properly.
2. Changes to `eden/scm/Makefile` to add build/install commands for getdeps to invoke. Those command knowing that they are called from withing getdeps context they link the dependencies brought by getdeps into their proper places that match their folder layout in fbsource. Those Makefile commands also pass a GETDEPS_BUILD env to the setup.py invocations so that it knows it is being called withing a getdeps build.
3. Changes to `eden/scm/setup.py` that add "thriftasset" that makes use of the getdeps.py provided "thrift" binary to build .py files out of thrift files.
4. Changes to `distutils_rust` to use the vendored crates dir provided by getdeps.
5. Changes to `getdeps/builder.py` and `getdeps/manifest.py` that enable more fine-grained configuratior of how Makefile builds are invoked.
6. Changes to `getdeps/buildopts.py` and `getdeps/manifest.py` to disable overriding PATH and pkgconfig env, so that "eden/scm" builds in getdeps using system libraries rather than getdeps-provided ones (NOTE: I've tried to use getdeps provided libraries, but the trickiest bit was that Rust links with Python, which is currently not providable by getdeps, so if you try to build everything the system provided Python libraries will collide with getdeps provided ones)
7. Added `opensource/fbcode_builder/manifests/eden_scm` for the getdeps build.
Reviewed By: quark-zju
Differential Revision: D22336485
fbshipit-source-id: 244d10c9e06ee83de61e97e62a1f2a2184d2312f
Summary:
The test case assumed that clone would return data in order from the server.
That is not a valid assumption and Mononoke doesn't return data in order.
Reviewed By: xavierd
Differential Revision: D22364636
fbshipit-source-id: abfcbe0074a08c9a76c42d351ce5c792eb65e24f
Summary:
Enable mutation, remotenames and narrow-heads (for clients).
Some `x` commits are shown because the server repo cannot hide them.
Reviewed By: DurhamG
Differential Revision: D22200501
fbshipit-source-id: 35abe5198025598f4f5ef8807c2eaa6f3b2f2318
Summary:
D21894320 (64585a5895) tries to disable features from hgsql. However tests can
run hg commands with hgsql extension turned on or off, and that
messes up things.
Let's test the hgsql requirement and disable related features unconditionally
so we have more confident that the related features won't be turned on
accidentally.
Previously, the `initclient` helper function will create repos with the `hgsql`
requirement. Since we're using the repo requirement to disable features, disable
`hgsql` temporarily during `initclient` so that the client repos can still have
modern features like visibility and narrow-heads.
The globalrevs test is affected and temporarily disabled. It will be fixed in
follow-ups.
Reviewed By: DurhamG
Differential Revision: D22200507
fbshipit-source-id: 3715464430a9115bb00122a8dfa03b1abf9d03ab
Summary: This helps debugging some hgsql issues especially in test-globalrevs.
Reviewed By: DurhamG
Differential Revision: D22200509
fbshipit-source-id: cddf79a00951d135dee20a5e9fb3a486abad2ff7
Summary:
Without the change the traceback looks like:
```
Traceback (most recent call last):
File "eden/scm/tests/testutil/dott/shobj.py", line 103, in __del__
out = self.output
File "eden/scm/tests/testutil/dott/shobj.py", line 67, in output
self._output = func(*args[1:], **kwargs) or ""
File "eden/scm/tests/test-globalrevs-t.py", line 499, in checkglobalrevs
isgreaterglobalrev("desc('h2')", "desc('g1')") == ""
File "eden/scm/tests/test-globalrevs-t.py", line 480, in isgreaterglobalrev
if getglobalrev(left) > getglobalrev(right):
File "eden/scm/tests/test-globalrevs-t.py", line 469, in getglobalrev
return int(s)
ValueError: invalid literal for int() with base 10: ''
```
There is no clue about who calls `checkglobalrevs`.
With this change the extra context is provided and it's easy to find out the code is around line 517:
```
Before executing:
File "eden/scm/tests/test-globalrevs-t.py", line 517, in <module>
sh % "cd ../master2"
File "eden/scm/tests/testutil/dott/shobj.py", line 161, in __mod__
return LazyCommand(command)
File "eden/scm/tests/testutil/dott/shobj.py", line 38, in __init__
_checkdelayedexception()
File "eden/scm/tests/testutil/dott/shobj.py", line 204, in _checkdelayedexception
traceback.print_stack()
```
Note: before D19649475 (a634526801) the traceback looks like more useful but it's hard to
create that traceback on Python 3:
```
Traceback (most recent call last):
File "eden/scm/tests/test-globalrevs-t.py", line 517, in <module>
sh % "cd ../master2"
File "eden/scm/tests/testutil/dott/shobj.py", line 161, in __mod__
return LazyCommand(command)
File "eden/scm/tests/testutil/dott/shobj.py", line 38, in __init__
_checkdelayedexception()
File "eden/scm/tests/testutil/dott/shobj.py", line 202, in _checkdelayedexception
exec("raise exctype, excvalue, tb")
File "eden/scm/tests/testutil/dott/shobj.py", line 103, in __del__
out = self.output
File "eden/scm/tests/testutil/dott/shobj.py", line 67, in output
self._output = func(*args[1:], **kwargs) or ""
File "eden/scm/tests/test-globalrevs-t.py", line 499, in checkglobalrevs
isgreaterglobalrev("desc('h2')", "desc('g1')") == ""
File "eden/scm/tests/test-globalrevs-t.py", line 480, in isgreaterglobalrev
if getglobalrev(left) > getglobalrev(right):
File "eden/scm/tests/test-globalrevs-t.py", line 469, in getglobalrev
return int(s)
ValueError: invalid literal for int() with base 10: ''
```
Reviewed By: DurhamG
Differential Revision: D22200508
fbshipit-source-id: 07088eac72763f890cc847b9991d79fed18ee0ef
Summary:
Similar to D7121487 (af8ecd5f80) but works for mutation store. This makes sure at the Rust
layer, mutation entries won't get lost after rebasing or metaeditting a set of
commits where a subset of the commits being edited has mutation relations.
Unlike the Python layer, the Rust layer works for mutation chains. Therefore
some of the tests changes.
Reviewed By: markbt
Differential Revision: D22174991
fbshipit-source-id: d62f7c1071fc71f939ec8771ac5968b992aa253c
Summary:
Enforcing 1:1 handling is causing trouble for multiple reasons:
- Test cases like "Metaedit with descendant folded commits" in
test-mutation.t requires handling non-1:1 entries.
- Currently operations like `metaedit-copy` uses two predecessors (edited,
copied). It is considered 1:1 rewrite (rewriting 1 commit to
another 1 commit). However, it does not use multiple records, therefore
cannot be distinguished from `fold`, which is not 1:1 rewrite and also
has multiple predecessors. We want to include the `metaedit-copy`
entries in the 1:1 DAG too.
Therefore lift the 1:1 restriction and let's see how it works.
Reviewed By: markbt
Differential Revision: D22175008
fbshipit-source-id: 84d870dbcc433a0d0e5611a83c93781bfa59d035
Summary:
This makes it easier to remove cycles in other places.
There are probably fancier and more efficient algorithm for this.
For now I just wrote one that is easy to verify correctness.
Reviewed By: markbt
Differential Revision: D22174975
fbshipit-source-id: 8a2dc755e4bc0b066eda5f42a51208c92409f2f9
Summary: Showing that it does not work for a chain of amends (or metaedits).
Reviewed By: DurhamG
Differential Revision: D22174968
fbshipit-source-id: ff942042b69f96e1cc7092a7003cf4608730a66f
Summary:
With narrow-heads (enabled in production and most tests) directaccess is a
no-op. Remove the extension.
Reviewed By: DurhamG
Differential Revision: D22174990
fbshipit-source-id: 662f800cebc3cc7614fa921e4367c4d1f8806eb4
Summary:
Switch to modern configs. It does not quite work right yet, which will be fixed
in follow-ups.
Reviewed By: DurhamG
Differential Revision: D22174966
fbshipit-source-id: d52234ec588a3a92512635291f64fa1819a4424b
Summary: It passes with some changes.
Reviewed By: DurhamG
Differential Revision: D22174988
fbshipit-source-id: 1fcefa81d979e01291cb9f646adcfdae46bb47f1
Summary: It passes with minor change.
Reviewed By: DurhamG
Differential Revision: D22175006
fbshipit-source-id: 6c5416fc64b065ea1d39eb2ed0cf3e4f873c4cb9
Summary:
This is one of the very few tests that didn't have visibility on.
The blocker was `hg rollback` behavior difference.
Reviewed By: DurhamG
Differential Revision: D22175000
fbshipit-source-id: 4067818c3cde8dade1a47671e8df6558ae348f6e
Summary:
Made some edits, mostly about `--hidden`, and phases to make the test
compatible.
Mutation store cannot represent undo (unamend) relationship and hidden
but non-obsoleted commits show up as `o`.
Reviewed By: DurhamG
Differential Revision: D22175001
fbshipit-source-id: 75dcc66238b4f465f5ac183278359d133758faf6
Summary:
With lots of edits, mostly maintaining bookmarks, the test is compatible with
modern configs.
Reviewed By: DurhamG
Differential Revision: D22174985
fbshipit-source-id: b88cd25da36ebcde908c1b63a15f90e82d1692cb
Summary: This makes it easier to see changes with modern configuration.
Reviewed By: DurhamG
Differential Revision: D22174957
fbshipit-source-id: c49f270281c06debd055435362b95509093c1e99
Summary: With some edits the test is compatible with modern configs.
Reviewed By: DurhamG
Differential Revision: D22174972
fbshipit-source-id: 971740f3a94048c15fcfc09436df7e09b3e169b8
Summary:
Stripping does break `histedit --abort`. Seems not worthwhile to support since
strip is rare. Edit the test so it's using modern configs.
Reviewed By: DurhamG
Differential Revision: D22174965
fbshipit-source-id: 090b92946311e192b9ee36ffa27718a62fc791ee
Summary: This test needs a few edits to be compatible with modern setup.
Reviewed By: DurhamG
Differential Revision: D22175002
fbshipit-source-id: 1b75b6535a46bd21d746259778b079f6c53a94d0
Summary: This test needs a few edits to be compatible with modern setup.
Reviewed By: DurhamG
Differential Revision: D22174969
fbshipit-source-id: 8f9c1b926d05070cfd8f17ca539693fa580de421
Summary:
This test needs some edits to be compatible with modern setup. Some
tests about legacy features are removed.
Reviewed By: DurhamG
Differential Revision: D22174983
fbshipit-source-id: e62de2b9571f3366b9c3c4bbfdb66cd7d990e317
Summary:
This test needs a lot manual edits to be compatible with modern setup. Some
tests about legacy features are removed.
Reviewed By: DurhamG
Differential Revision: D22174976
fbshipit-source-id: 09ef10f94438753469ed8725a4b8a5bdfed177cf
Summary:
This test needs some manual edits to be compatible with modern setup.
Some tests about legacy features are removed.
Reviewed By: DurhamG
Differential Revision: D22174997
fbshipit-source-id: 0423a45c4d085f4b1128b11b5f0b9876ed82e950
Summary: This test needs some manual editing to fully migrate off revision numbers.
Reviewed By: DurhamG
Differential Revision: D22174980
fbshipit-source-id: baf2ea5f591870fa7560b8a9d23c2a357851e028
