Summary:
These two perf counters proved to be not very convenient to evaluate the
volume of undesired file fetches. Let's get rid of them. Specifically, they are
not convenient, because they accumulate values and it's hard to aggregate over
them.
Note that I don't do the same for tree fetches, as there's no better way of
estimating those now.
Reviewed By: mitrandir77
Differential Revision: D23452913
fbshipit-source-id: 08f8dd25eece495f986dc912a302ab3109662478
Summary:
We want to be able to report more than just on one prefix. Instead, let's add a regex-based reporting. To make deployment easier, let's keep both options for now and later just remove prefix-based one.
Note: this diff also changes how a situation with absent `undesired_path_prefix_to_log` is treated. Previously, if `undesired_path_prefix_to_log` is absent, but `"undesired_path_repo_name_to_log": "fbsource"`, it would report every path. Now it won't report any, which I think is a saner behavior. If we do ever want to report every path, we can just add `.*` as a regex.
Reviewed By: StanislavGlebik
Differential Revision: D23447800
fbshipit-source-id: 059109b44256f5703843625b7ab725a243a13056
Summary: This is streaming clone warmup binary as per https://fb.quip.com/hfuBAdYnzr9M
Reviewed By: StanislavGlebik
Differential Revision: D23347029
fbshipit-source-id: f187a2f3529a7eae5998bab199228bfbe6057e6e
Summary:
Bookmark movement for service write will use different restrictions than hooks.
Move hook running to be controlled by an enum in preparation for adding service
write restrictions.
Reviewed By: StanislavGlebik
Differential Revision: D23287998
fbshipit-source-id: 30670d4d6666c341885b57a3f41246e52db541a2
Summary:
The error message for fast-forward failure is wrong. The correct way to allow
non-fast-forward moves is with the NON_FAST_FORWARD pushvar.
Reviewed By: StanislavGlebik
Differential Revision: D23243542
fbshipit-source-id: 554cdee078cd712f17441bd10bd7968b0674bbfe
Summary:
When bookmarks are moved or created, work out what additional changesets
should have the hooks run on them. This may apply to plain pushes,
force pushrebases, or bookmark only pushrebases.
At first, this will run in logging-only mode where we will count how many
changesets would have hooks run on them (up to a tunable limit). We can
enable running of hooks with a tunable killswitch later on.
Reviewed By: StanislavGlebik
Differential Revision: D23194240
fbshipit-source-id: 8031fdc1634168308c7fe2ad3c22ae4389a04711
Summary:
Move the running of hooks from in `repo_client` to in `bookmarks_movement`.
For pushrebase and plain push we still only run hooks on the new commits the client has sent.
Bookmark-only pushrebases, or moves where some commits were already known, do not run
the hooks on the omitted changesets. That will be addressed next.
The push-redirector currently runs hooks in the large repo. Since hook running has now been moved
to later on, they will automatically be run on the large repo, and instead the push-redirector runs them on
the small repo, to ensure they are run on both.
There's some additional complication with translating hook rejections in the push-redirector. Since a
bookmark-only push can result in hook rejections for commits that are not translated, we fall back to
using the large-repo commit hash in those scenarios.
Reviewed By: StanislavGlebik
Differential Revision: D23077551
fbshipit-source-id: 07f66a96eaca4df08fc534e335e6d9f6b028730d
Summary:
We already had a logic that prints if we are about to run an expensive
getbundle. However this logic prints a warning after we've fetched 1M commits
already, and user would have to wait for a long time to get this message.
However in some cases we can give this warning very quickly. For example, if
the lowest "heads" generation number is >1M commits away from highest "common"
generation number, then we can print the warning right away.
Differential Revision: D23213482
fbshipit-source-id: 67e2399ca958703129cf3c22d82ce48cbbdcd2d1
Summary:
Previously for undesired fetches of lfs files we were logging 0. Let's log the
real path instead
Reviewed By: ikostia
Differential Revision: D23209754
fbshipit-source-id: 7a893b257a89332a5169ab2072ecf48ae94b91e0
Summary:
Subscribers to the commit tailer categories would like to know when Mononoke
received the commit.
Reviewed By: StanislavGlebik
Differential Revision: D23162447
fbshipit-source-id: 747214f1964a643f59c491aa08cdbd5c8fe331c8
Summary:
Convert hook rejections from a tuple to a named struct. This will be used in
the bookmarks_movement public interface.
Reviewed By: krallin
Differential Revision: D23077550
fbshipit-source-id: a35476817660c38b8df879ba603b927a7e39be21
Summary:
Extract the calls to bookmarks_movement to separate functions to avoid duplication and
make the post-resolve action functions easier to read.
Reviewed By: StanislavGlebik
Differential Revision: D23057045
fbshipit-source-id: c6b5a8cdb2399e89c174c3df844529d4b5309edf
Summary: Refactor control of movement of non-scratch bookmarks through pushrebase.
Reviewed By: krallin
Differential Revision: D22920694
fbshipit-source-id: 347777045b4995b69973118781511686cf34bdba
Summary:
Some parts of the `pushrebase` public interface will be re-exported from `bookmarks_movement`.
Clean these up in preparation:
* Remove `OntoBookmarkParams` as it is now a simple wrapper around `BookmarkName` that
prevents us from using a reference.
* Make the bundle replay data `Option<&T>` rather than `&Option<T>`, allowing us to
use the former when available. The latter can be readily converted with `.as_ref()`.
* Rename `SuccessResult` to `Outcome` and `ErrorKind` to `InternalError`.
Reviewed By: krallin
Differential Revision: D23055580
fbshipit-source-id: 1208a934f979a9d5eb73310fb8711b1291393ecf
Summary:
Refactor control of movement of non-scratch bookmarks through force-pushrebase
or bookmark-only pushrebase. These are equivalent to ordinary pushes, and so
can use the same code path for moving the bookmarks.
This has the side-effect of enabling some patterns that were previously not
possible, like populating git mappings with a force-pushrebase.
Reviewed By: ikostia
Differential Revision: D22844828
fbshipit-source-id: 4ef71fa4cef69cc2f1d124837631e8304644ca06
Summary: Refactor control of movement of non-scratch bookmarks through plain pushes.
Reviewed By: krallin
Differential Revision: D22844829
fbshipit-source-id: 2f1a89e1d0f69880f74b7bc135144bfb305a918e
Summary:
Refactor control of movement of scratch bookmarks to a new `bookmark_movement` crate
that will contain all bookmark movement controls.
Reviewed By: krallin
Differential Revision: D22844830
fbshipit-source-id: 56d25ad45a9328eaa079c13466b4b802f033d1dd
Summary: Let's use new flag to enable/disable short history for getpack request
Reviewed By: krallin
Differential Revision: D23080200
fbshipit-source-id: 7aa0be6ded0601fa4d31d4b9ff8792a4f8d91b19
Summary:
In a repository with files with large histories we run into a lot of SqlTimeout
errors while fetching file history to serve getpack calls. However fetching the
whole file history is not really necessary - client knows how to work with
partial history i.e. if client misses some portion of history then it would
just fetch it on demand.
This diff adds way to add a limit on how many entries were going to be fetched, and if more entries were fetched then we return FilenodeRangeResult::TooBig. The downside of this diff is that we'd have to do more sequential database
queries.
Reviewed By: krallin
Differential Revision: D23025249
fbshipit-source-id: ebed9d6df6f8f40e658bc4b83123c75f78e70d93
Summary: The more contexts the better. Makes debugging errors much more pleasant.
Reviewed By: StanislavGlebik
Differential Revision: D22890940
fbshipit-source-id: 48f89031b4b5f9b15f69734d784969e2986b926d
Summary:
This is the (almost) final diff to introduce WarmBookmarksCache in repo_client.
A lot of this code is to pass through the config value, but a few things I'd
like to point out:
1) Warm bookmark cache is enabled from config, but it can be killswitched using
a tunable.
2) WarmBookmarksCache in scs derives all derived data, but for repo_client I
decided to derive just hg changeset. The main motivation is to not change the
current behaviour, and to make mononoke server more resilient to failures in
other derived data types.
3) Note that WarmBookmarksCache doesn't obsolete SessionBookmarksCache that was
introduced earlier, but rather it complements it. If WarmBookmarksCache is
enabled, then SessionBookmarksCache reads the bookmarks from it and not from
db.
4) There's one exception in point #3 - if we just did a push then we read
bookmarks from db rather than from bookmarks cache (see
update_publishing_bookmarks_after_push() method). This is done intentionally -
after push is finished we want to return the latest updated bookmarks to the
client (because the client has just moved a bookmark after all!).
I'd argue that the current code is a bit sketchy already - it doesn't read from
master but from replica, which means we could still see outdated bookmarks.
Reviewed By: krallin
Differential Revision: D22820879
fbshipit-source-id: 64a0aa0311edf17ad4cb548993d1d841aa320958
Summary:
The check does not practically work because the client sends `common=[null]`
if the common set is empty.
D22519582 changes the client-side logic to send `common=[]` instead of
`common=[null]` in such cases. Therefore remove the constraint to keep
tests passing. 13 tests depend on this change.
Reviewed By: StanislavGlebik
Differential Revision: D22612285
fbshipit-source-id: 48fbc94c6ab8112f0d7bae1e276f40c2edd47364
Summary:
The overall goal of this stack is to add WarmBookmarksCache support to
repo_client to make Mononoke more resilient to lands of very large
commits.
The code for managing cached_publishing_bookmarks_maybe_stale was already a bit
tricky, and with WarmBookmarksCache introduction it would've gotten even worse.
Let's move this logic to a separate SessionBookmarkCache struct.
Reviewed By: krallin
Differential Revision: D22816708
fbshipit-source-id: 02a7e127ebc68504b8f1a7401beb063a031bc0f4
Summary:
The overall goal of this stack is to add WarmBookmarksCache support to
repo_client to make Mononoke more resilient to lands of very large
commits.
The problem with large changesets is deriving hg changesets for them. It might take
a significant amount of time, and that means that all the clients are stuck waiting on
listkeys() or heads() call waiting for derivation. WarmBookmarksCache can help here by returning bookmarks
for which hg changesets were already derived.
This is the second refactoring to introduce WarmBookmarksCache.
Now let's cache not only pull default, but also publishing bookmarks. There are two reasons to do it:
1) (Less important) It simplifies the code slightly
2) (More important) Without this change 'heads()' fetches all bookmarks directly from BlobRepo thus
bypassing any caches that we might have. So in order to make WarmBookmarksCache useful we need to avoid
doing that.
Reviewed By: farnz
Differential Revision: D22816707
fbshipit-source-id: 9593426796b5263344bd29fe5a92451770dabdc6
Summary:
The overall goal of this stack is to add WarmBookmarksCache support to
repo_client to make Mononoke more resilient to lands of very large commits.
This diff just does a small refactoring that makes introducing
WarmBookmarksCache easier. In particular, later in cached_pull_default_bookmarks_maybe_stale cache I'd like to store
not only PullDefault bookmarks, but also Publishing bookmarks so that both
listkeys() and heads() method could be served from this cache. In order to do
that we need to store not only bookmark name, but also bookmark kind (i.e. is
it Publishing or PullDefault).
To do that let's store the actual Bookmarks and hg changeset objects instead of
raw bytes.
Reviewed By: farnz
Differential Revision: D22816710
fbshipit-source-id: 6ec3af8fe365d767689e8f6552f9af24cbcd0cb9
Summary: Same as a previous diff. Let's keep the top-level dir tidy.
Reviewed By: krallin
Differential Revision: D22691638
fbshipit-source-id: 7f9a21f307efd9bbe37f515f475409c89b99cd31
Summary:
Currently the repo lock is checked only once at the beginnig of unbundle future. That unbundle process take some time and during that time repo can be locked by someone.
We can reduce that possibility by creating additional future, which will check the repo in the loop and poll both futures for whoever will finish first.
Reviewed By: StanislavGlebik
Differential Revision: D22560907
fbshipit-source-id: 1cba492fa101dba988e07361e4048c6e9b778197
Summary:
This is the next step in exposing version, used to sync commits in read
queries. The previous step was to query this from DB, now let's also put it
into an enum payload. Further, I will add consumers of this in admin and
validation.
Note that ideally, `RewrittenAs` should always have a version associated with
it, but:
- right now this is not true in the DB (needs backfilling)
- even when I backfill everything, I would not like to error out just at
reading time, I would prefer the consumers to deal with the absense of a
version in rewrtitten commits.
Therefore, I decided to use `Option` here.
Reviewed By: StanislavGlebik
Differential Revision: D22476166
fbshipit-source-id: 5bc27bb21b7e59c604755ef35aa5d3d2c3df527e
Summary:
Separate out the `BundleReplayData` from the `BookmarkUpdateReason` enum. There's
no real need for this to be part of the reason, and removing it means we can
abstract away the remaining dependency on Mercurial changeset IDs from
the main bookmarks traits.
Reviewed By: mitrandir77, ikostia
Differential Revision: D22417659
fbshipit-source-id: c8e5af7ba57d10a90c86437b59c0d48e587e730e
Summary:
The goal is to make it easier to implement unit tests, which depend on `LiveCommitSyncConfig`. Specifically, `scs` has a piece of code, which instantiates `mononoke_api::Repo` with a test version of `CommitSyncConfig`. To migrate it to `LiveCommitSyncConfig`, I need to be able to create a test version of that. It **is** possible now, but would require me to turn a supplied instance of `CommitSyncConfig` back into `json`, which is cumbersome. Using a `dyn LiveCommitSyncConfig` there, instead of a concrete struct seems like a good idea.
Note also that we are using this technique in many places: most (all?) of our DB tables are traits, which we then implement for SQL-specific structs.
Finally, this diff does not actually migrate all of the current users of `LiveCommitSyncConfig` (the struct) to be users of `LiveCommitSyncConfig` (the trait), and instead makes them use `CfgrLiveCommitSyncConfig` (the trait impl). The idea is that we can migrate bits to use traits when needed (for example, in an upcoming `scs` diff). When not needed, it's fine to use concrete structs. Again, this is already the case in a a few places: we sometimes use `SqlSyncedCommitMapping` struct directly, instead of `T: SyncedCommitMapping` or `dyn SyncedCommitMapping`.
Reviewed By: StanislavGlebik
Differential Revision: D22383859
fbshipit-source-id: 8657fa39b11101684c1baae9f26becad6f890302
Summary:
Just knowing the number of fetched undesired files doesn't give the full
picture. e.g. fetching lots of small files is better than fetching single
multi-Gb file.
So knowing the size of files is helpful
Reviewed By: krallin
Differential Revision: D22408400
fbshipit-source-id: 7653c1cdceccf50aeda9ce8a4880ee5178d4b107
Summary: D22381744 updated the version of `futures` in third-party/rust to 0.3.5, but did not regenerate the autocargo-managed Cargo.toml files in the repo. Although this is a semver-compatible change (and therefore should not break anything), it means that affected projects would see changes to all of their Cargo.toml files the next time they ran `cargo autocargo`.
Reviewed By: dtolnay
Differential Revision: D22403809
fbshipit-source-id: eb1fdbaf69c99549309da0f67c9bebcb69c1131b
Summary:
Rework the bookmarks traits:
* Split out log functions into a separate `BookmarkUpdateLog` trait. The cache doesn't care about these methods.
* Simplify `list` down to a single method with appropriate filtering parameters. We want to add more filtering types, and adding more methods for each possible combination will be messier.
* The `Bookmarks` and `BookmarkUpdateLog` traits become `attributes` on `BlobRepo`, rather than being a named member.
Reorganise the bookmarks crate to separate out the bookmarks log and transactions into their own modules.
Reviewed By: krallin
Differential Revision: D22307781
fbshipit-source-id: 4fe514df8b7ef92ed3def80b21a16e196d916c64
Summary:
`bulk_add()` method was checking for conflicts correctly i.e. it wouldn't fail
if we try to insert the same mapping twice.
`bulk_add_git_mapping_in_transaction` wasn't doing this check i.e. it would
fail.
This caused us a few problems and this diff fixes them - now
`bulk_add_git_mapping_in_transaction` would do the same checks as bulk_add was
doing previously.
There is another important change in behaviour: if we try to insert two entries, one of them
has a conflict another don't then previously we'd insert the second entry.
Now we don't insert any, arguably that's a preferred behaviour.
Reviewed By: krallin
Differential Revision: D22332001
fbshipit-source-id: 86fff8c23c43eeca0fb36b01b10cdaa73b3ce4ab
Summary: Convert the bookmarks traits to use new-style `BoxFuture<'static>` and `BoxStream<'static>`. This is a step along the path to full `async`/`await`.
Reviewed By: farnz
Differential Revision: D22244489
fbshipit-source-id: b1bcb65a6d9e63bc963d9faf106db61cd507e452
Summary: This is the final diff of the stack - it starts logging pushed commits to scribe
Reviewed By: farnz
Differential Revision: D22212755
fbshipit-source-id: ec09728408468acaeb1c214d43f930faac30899b
Summary:
Failing push if we failed to log to scribe doesn't make a lot of sense. By that
time the ship has sailed - commit has already been pushed and by failing the
request we can't undo that. It will just create an annoyance by whoever is
pushing.
Instead let's log it to scuba
Reviewed By: farnz
Differential Revision: D22256687
fbshipit-source-id: 2428bbf1db4cef6fa80777ad65184fab1804fa9c
Summary:
At the moment we can't test logging to scribe easily - we don't have a way to
mock it. Scribe are supposed to help with that.
They will let us to configure all scribe logs to go to a directory on a
filesystem similar to the way we configure scuba. The Scribe itself will
be stored in CoreContext
Reviewed By: farnz
Differential Revision: D22237730
fbshipit-source-id: 144340bcfb1babc3577026191428df48e30a0bb6
Summary:
Eventually, we want everything to be `async`/`await`; as a stepping stone in that direction, switch the remaining lobstore traits to new-style futures.
This just pushes the `.compat()` out to old-style futures, but it makes the move to non-'static lifetimes easier, as all the compile errors will relate to lifetime issues.
Reviewed By: krallin
Differential Revision: D22183228
fbshipit-source-id: 3fe3977f4469626f55cbf5636d17fff905039827
Summary:
Eventually, we want everything to be `async`/`await`; as a stepping stone in that direction, switch some of the blobstore interfaces to new-style `BoxFuture` with a `'static` lifetime.
This does not enable any fixes at this point, but does mean that `.compat()` moves to the places that need old-style futures instead of new. It also means that the work needed to make the transition fully complete is changed from a full conversion to new futures, to simply changing the lifetimes involved and fixing the resulting compile failures.
Reviewed By: krallin
Differential Revision: D22164315
fbshipit-source-id: dc655c36db4711d84d42d1e81b76e5dddd16f59d
Summary:
Push supported multiple bookmarks in theory, but in practice we never used it.
Since we want to start logging pushed commits in the next diffs we need to decide what to do with
bookmarks, since at the moment we can log only a single bookmark to scribe
let's just allow a single bookmark push
Reviewed By: farnz
Differential Revision: D22212674
fbshipit-source-id: 8191ee26337445ce2ef43adf1a6ded3e3832cc97
Summary:
In the next diffs it will be passed to unbundle processing so that we can use
scribe category to log pushed commits
Reviewed By: krallin
Differential Revision: D22212616
fbshipit-source-id: 17552bda11f102041a043f810125dc381e478611
Summary: That was like 50% of the point of this change, and somehow I forgot to do it.
Reviewed By: farnz
Differential Revision: D22231923
fbshipit-source-id: 4a4daaeaa844acd219680907c0b5a5fdacdf535c
Summary:
This fn is not used anywhere except tests, and its only difference from
`backsync_all_latest` is in the fact that it accepts a limit. So let's rename
`backsync_all_latest` into `backsync_latest` and make it accept a limit arg.
I decided to use a custom enum instead of `Option` so that people don't have to
open fn definition to understand what `BacksyncLimit::Limit(2)` or
`BacksyncLimit::NoLimit` mean.
Reviewed By: StanislavGlebik
Differential Revision: D22187118
fbshipit-source-id: 6bd97bd6e6f3776e46c6031f775739ca6788ec8c
Summary:
This diff enables `unbundle` flow to start creating `push_redirector` structs from hot-reloaded `CommitSyncConfig` (by using the `LiveCommitSyncConfig` struct).
Using `LiveCommitSyncConfig` unfortunately means that we need to make sure those tests, which don't use standard fixtures, need to have both the `.toml` and the `.json` commit sync configs present, which is a little verbose. But it's not too horrible.
Reviewed By: StanislavGlebik
Differential Revision: D21962960
fbshipit-source-id: d355210b5dac50d1b3ad277f99af5bab56c9b62e
Summary:
Due to Thrift design of "include" statements in fbcode the thrift structures has to be contained in folders that are identical to the folder layout inside fbcode.
This diff changes the folder layout on Cargp.toml files and in fbcode_builder, there will be a next diff that changes this for ShipIt as well.
Reviewed By: ikostia
Differential Revision: D22208707
fbshipit-source-id: 65f9cafed2f0fcc8398a3887dfa622de9e139f68
Summary:
If a commit changes modes (i.e. executable, symlink or regular) of a lot of files but
doesn't change their content then we don't need to put these filenodes to the
generated bundle. Mercurial stores mode in manifest, so changing the mode
doesn't change the filenode.
Reviewed By: ikostia
Differential Revision: D22206736
fbshipit-source-id: f64ee8a34281cd207c92653b927bf9109ccbe1b4
Summary: `HgPhase` type is redundant and was adding dependency on mercurial in phases crate.
Reviewed By: farnz
Differential Revision: D22162716
fbshipit-source-id: 1c21841d34897d0072ff6fe5e4ac89adddeb3c68
Summary: DangerousOverride is moved into a separate crate. Not only it is usually not needed but it was introducing dependencies on mercurial crate.
Reviewed By: StanislavGlebik
Differential Revision: D22115015
fbshipit-source-id: c9646896f906ea54d11aa83a8fbd8490a5b115ea
Summary: Move all mercurial changeset generation logic to `blobrepo_hg`. This is preliminary step is required to decouples BlobRepo from mercurial, and in later stages it will be moved to derived data infra once blobrepo is free of mercurial.
Reviewed By: StanislavGlebik
Differential Revision: D22089677
fbshipit-source-id: bca28dedda499f80899e729e4142e373d8bec0b8
