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: 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:
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:
Remove the `repo_id` parameter from the `Bookmarks` trait methods.
The `repo_id` parameters was intended to allow a single `Bookmarks` implementation
to serve multiple repos. In practise, however, each repo has its own config, which
results in a separate `Bookmarks` instance for each repo. The `repo_id` parameter
complicates the API and provides no benefit.
To make this work, we switch to the `Builder` pattern for `SqlBookmarks`, which
allows us to inject the `repo_id` at construction time. In fact nothing here
prevents us from adding back-end sharing later on, as these `SqlBookmarks` objects
are free to share data in their implementation.
Reviewed By: StanislavGlebik
Differential Revision: D22437089
fbshipit-source-id: d20e08ce6313108b74912683c620d25d6bf7ca01
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: 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: 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:
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: This diff introduces `BlobRepoHg` extension trait for `BlobRepo` object. Which contains mercurial specific methods that were previously part of `BlobRepo`. This diff also stars moving some of the methods from BlobRepo to BlobRepoHg.
Reviewed By: ikostia
Differential Revision: D21659867
fbshipit-source-id: 1af992915a776f6f6e49b03e4156151741b2fca2
Summary: Cover as much as remining code with `Cargo.toml`s, for the rest create an exlusion list in the autocargo config.
Reviewed By: krallin
Differential Revision: D21383620
fbshipit-source-id: 64cc78a38ce0ec482966f32a2963ab4939e20eba
Summary: The new MembershipChecker and PermissionChecker traits will generalize access to various permission/acl systems (like LDAP) and leave the implementation details hidden behind an object trait.
Reviewed By: StanislavGlebik
Differential Revision: D21067811
fbshipit-source-id: 3bccd931f8acdb6c1e0cff4cb71917c9711b590b
Summary: Not in use any more - all hooks are now Bonsai form - so remove it.
Reviewed By: krallin
Differential Revision: D20891164
fbshipit-source-id: b92f169a0ec3a4832f8e9ec8dc9696ce81f7edb3
Summary: Running on Mercurial hooks isn't scalable long term - move the consumers of hooks to run on both forms for a transition period
Reviewed By: krallin
Differential Revision: D20879136
fbshipit-source-id: 4630cafaebbf6a26aa6ba92bd8d53794a1d1c058
Summary: We want all hooks to run against the Bonsai form, not a Mercurial form. Create a second form of hooks (currently not used) which acts on Bonsai hooks. Later diffs in the stack will move us over to Bonsai only, and remove support for Mercurial changeset derived hooks
Reviewed By: krallin
Differential Revision: D20604846
fbshipit-source-id: 61eece8bc4ec5dcc262059c19a434d5966a8d550
Summary: This is helpful to draw conclusions as to how fast it is.
Reviewed By: StanislavGlebik
Differential Revision: D20872108
fbshipit-source-id: d323358bbba29de310d6dfb4c605e72ce550a019
Summary:
Migrate the configuration of sql data managers from the old configuration using `sql_ext::SqlConstructors` to the new configuration using `sql_construct::SqlConstruct`.
In the old configuration, sharded filenodes were included in the configuration of remote databases, even when that made no sense:
```
[storage.db.remote]
db_address = "main_database"
sharded_filenodes = { shard_map = "sharded_database", shard_num = 100 }
[storage.blobstore.multiplexed]
queue_db = { remote = {
db_address = "queue_database",
sharded_filenodes = { shard_map = "valid_config_but_meaningless", shard_num = 100 }
}
```
This change separates out:
* **DatabaseConfig**, which describes a single local or remote connection to a database, used in configuration like the queue database.
* **MetadataDatabaseConfig**, which describes the multiple databases used for repo metadata.
**MetadataDatabaseConfig** is either:
* **Local**, which is a local sqlite database, the same as for **DatabaseConfig**; or
* **Remote**, which contains:
* `primary`, the database used for main metadata.
* `filenodes`, the database used for filenodes, which may be sharded or unsharded.
More fields can be added to **RemoteMetadataDatabaseConfig** when we want to add new databases.
New configuration looks like:
```
[storage.metadata.remote]
primary = { db_address = "main_database" }
filenodes = { sharded = { shard_map = "sharded_database", shard_num = 100 } }
[storage.blobstore.multiplexed]
queue_db = { remote = { db_address = "queue_database" } }
```
The `sql_construct` crate facilitates this by providing the following traits:
* **SqlConstruct** defines the basic rules for construction, and allows construction based on a local sqlite database.
* **SqlShardedConstruct** defines the basic rules for construction based on sharded databases.
* **FbSqlConstruct** and **FbShardedSqlConstruct** allow construction based on unsharded and sharded remote databases on Facebook infra.
* **SqlConstructFromDatabaseConfig** allows construction based on the database defined in **DatabaseConfig**.
* **SqlConstructFromMetadataDatabaseConfig** allows construction based on the appropriate database defined in **MetadataDatabaseConfig**.
* **SqlShardableConstructFromMetadataDatabaseConfig** allows construction based on the appropriate shardable databases defined in **MetadataDatabaseConfig**.
Sql database managers should implement:
* **SqlConstruct** in order to define how to construct an unsharded instance from a single set of `SqlConnections`.
* **SqlShardedConstruct**, if they are shardable, in order to define how to construct a sharded instance.
* If the database is part of the repository metadata database config, either of:
* **SqlConstructFromMetadataDatabaseConfig** if they are not shardable. By default they will use the primary metadata database, but this can be overridden by implementing `remote_database_config`.
* **SqlShardableConstructFromMetadataDatabaseConfig** if they are shardable. They must implement `remote_database_config` to specify where to get the sharded or unsharded configuration from.
Reviewed By: StanislavGlebik
Differential Revision: D20734883
fbshipit-source-id: bb2f4cb3806edad2bbd54a47558a164e3190c5d1
Summary:
As suggested in D20680173, we can reduce the overall need to copy things by
storing refs in the resolver.
Reviewed By: krallin
Differential Revision: D20696588
fbshipit-source-id: 9456e2e208cfef6faed57fc52ca59fafdccfc68c
Summary:
This diff focuses on the following:
- replaces clones with references, both when this decreases the total sum of
clones, and when it causes the only clone to be on the boundary with the
compat code. This, when those boundaries are pushed further, we can only fix
one place in resolver
- removes a weird wrapping of a closure into an `Arc` and just calls
`upload_changesets` directly instead
- in cases when `BundleResolver` methods take `ctx` as an argument removes it
and makes those methods use the one stored in the struct
Reviewed By: StanislavGlebik
Differential Revision: D20680173
fbshipit-source-id: c397c4ade57a07cbbc9206fa8a44f4225426778c
Summary:
As the name indicates, this updates unbundle_replay to run hooks. Hook failures
don't block the replay, but they're logged to Scuba.
Differential Revision: D20693851
fbshipit-source-id: 4357bb0d6869a658026dbc5421a694bc4b39816f
Summary:
Setting up a derived data tailer for this is a better approach (see D20668301
for context).
Reviewed By: StanislavGlebik
Differential Revision: D20693270
fbshipit-source-id: 7a06ffe059c41c4e100f8b0f8837978717293829
Summary:
Since we do those concurrently, it makes sense to do them on their own task.
Besides, since those are still old futures that need ownership, there is
effectively no tradeoff here.
Differential Revision: D20691373
fbshipit-source-id: 1a45e43ec857d91bed1614568b4354d56a2b0848
Summary: This will make it easier to compare performance.
Differential Revision: D20674164
fbshipit-source-id: eb1a037b0b060c373c1e87635f52dd228f728c89
Summary: This adds some Scuba reporting to unbundle_replay.
Differential Revision: D20674162
fbshipit-source-id: 59e12de90f5fca8a7c341478048e68a53ff0cdc1
Summary:
This updates unbundle_replay to do things concurrently where possible.
Concretely, this means we do ingest unbundles concurrently, and filenodes
derivation concurrently, and only do the actual pushrebase sequentially. This
lets us get ahead on work wherever we can, and makes the process faster.
Doing unbundles concurrently isn't actually guaranteed to succeed, since it's
*possible* that an unbundle coming in immediately after a pushrebase actually
depends the commits created in said pushrebase. In this case, we simply retry
the unbundle when we're ready to proceed with the pushrebase (in the code, this
is the `Deferred` variant). This is fine from a performance perspective
As part of this, I've also moved the loading of the bundle to processing, as
opposed to the hg recording client (the motivation for this is that we want to
do this loading in parallel as well).
This will also let us run hooks in parallel once I add this in.
Reviewed By: StanislavGlebik
Differential Revision: D20668301
fbshipit-source-id: fe2c62ca543f29254b4c5a3e138538e8a3647daa
Summary:
pushrebase_errmsg is NULL when we have conflicts, but we still shouldn't replay
the entry (because it'll fail, with conflicts). Let's exclude those.
Reviewed By: StanislavGlebik
Differential Revision: D20668304
fbshipit-source-id: a058bb466e0a8a53ec81e41db7ba138d6aedf3f9
Summary:
This updates unbundle_replay to support sleeping when watching for updates in a
bookmark and said bookmark isn't moving. This will be useful so it can run as a
service.
Reviewed By: StanislavGlebik
Differential Revision: D20645157
fbshipit-source-id: 6edeb66b65b2ef8b88c8db5e664982756acbfaf1
Summary:
I accidentally forgot to insert the entry, so that made this test a bit
useless. Let's make it not useless.
Reviewed By: StanislavGlebik
Differential Revision: D20645158
fbshipit-source-id: 0f0eb0cf9d16e8c346897088891aa3277b4d9c07
Summary:
This adds support for replaying the updates to a bookmark through unbundle
replay. The goal is to be able to run this as a process that keeps a bookmark
continuously updated.
There is still a bit of work here, since we don't yet allow the stream to pause
until bookmark update becomes available (i.e. once caught up, it will exit).
I'll introduce this in another diff.
Note that this is only guaranteed to work if there is a single bookmark in the
repo. With more, it could fail if a commit is first introduced in a bookmark that
isn't the one being replayed here, and later gets introduced in said bookmark.
Reviewed By: StanislavGlebik
Differential Revision: D20645159
fbshipit-source-id: 0aa11195079fa6ac4553b0c1acc8aef610824747
Summary:
I'm going to update this to run in a loop, so to do that it would be nice to
represent the things to replay as a stream. This does that change, but for now
all our streams have just one element.
Reviewed By: StanislavGlebik
Differential Revision: D20645156
fbshipit-source-id: fce7536d0ccbc1911335704816b71c17e80f2116
Summary:
We normally derive those lazily when accepting pushrebase, but we do derive
them eagerly in blobimport. For now, let's be consistent with blobimport.
This ensures that we don't lazily generate them, which would require read traffic,
and gives a picture a little more consistent with what an actual push would look like.
Reviewed By: ikostia
Differential Revision: D20623966
fbshipit-source-id: 2209877e9f07126b7b40561abf3e6067f7a613e6
Summary:
This makes it easier to realize if you used the wrong entry ID when replaying
(instead of telling you the bookmark isn't at `None` as expected, it tells you
the Hg Changeset could not be mapped to a Bonsai).
Reviewed By: ikostia
Differential Revision: D20623847
fbshipit-source-id: aaa66e7825f12373742efd4f779ae20ff21f0b46
Summary:
This updates unbundle_replay to account for pushrebase hooks, notably to assign
globalrevs.
To do so, I've extracted the creation of pushrebase hooks in repo_client and
reused it in unbundle_replay. I also had to update unbundle_replay to no longer
use `args::get_repo` since that doesn't give us access to the config (which we
need to know what pushrebase hooks to enable).
Reviewed By: ikostia
Differential Revision: D20622723
fbshipit-source-id: c74068c920822ac9d25e86289a28eeb0568768fc
Summary:
This adds a unbundle_replay Rust binary. Conceptually, this is similar to the
old unbundle replay Python script we used to have, but there are a few
important differences:
- It runs fully in-process, as opposed to pushing to a Mononoke host.
- It will validate that the pushrebase being produced is consistent with what
is expected before moving the bookmark.
- It can find sources to replay from the bookmarks update log (which is
convenient for testing).
Basically, this is to writes and to the old unbundle replay mechanism what
Fastreplay is to reads and to the traffic replay script.
There is still a bit of work to do here, notably:
- Make it possible to run this in a loop to ingest updates iteratively.
- Run hooks.
- Log to Scuba!
- Add the necessary hooks (notably globalrevs)
- Set up pushrebase flags.
I would also like to see if we can disable the presence cache here, which would
let us also use this as a framework for benchmarking work on push performance,
if / when we need that.
Reviewed By: StanislavGlebik
Differential Revision: D20603306
fbshipit-source-id: 187c228832fc81bdd30f3288021bba12f5aca69c
