Summary:
Let's split logic from WarmBookmarksCache into a separate builder. This builder
will configure which warmers we'd like to use.
This will make it easier to introduce a new warmer later in the stack
Reviewed By: krallin
Differential Revision: D23053785
fbshipit-source-id: 32acc9da98d32624ca0dc00277910443f3d86f66
Summary:
Previously we were unconditionally adding hg changesets, but that's a bit
strange and there's no reason to do it. Let's do the same check we do for other
derived data types. Note that there should be no change in behaviour - all our
repos have "hgchangesets" derived data type enabled.
Reviewed By: krallin
Differential Revision: D23053786
fbshipit-source-id: 0b3ea99f649bc89ea9b216f368fee11fa25e153f
Summary: I want to add a new warmer in the next diffs which won't do any deriving.
Reviewed By: krallin
Differential Revision: D23053787
fbshipit-source-id: 4c7febb60ab7e835302db746c670d656bd9d1989
Summary:
Related diff: D22816538 (3abc4312af)
In repo_import tool once we move a bookmark to reveal commits to users, we want to check if hg_sync has received the commits. To do this, we extract the largest log id from bookmarks_update_log to compare it with the mutable_counter value related to hg_sync. If the counter value is larger or equal to the log id, we can move the bookmark to the next batch of commits. Otherwise, we sleep, retry fetching the mutable_counter value and compare the two again.
mutable_counters is an sql table that can track bookmarks log update instances with a counter.
This diff adds the functionality to extract the mutable_counters value for hg_sync.
======================
SQL query fix:
In the previous diff (D22816538 (3abc4312af)) we didn't cover the case where we might not get an ID which should return None. This diff fixes this error.
Reviewed By: StanislavGlebik
Differential Revision: D22864223
fbshipit-source-id: f3690263b4eebfe151e50b01a13b0193009e3bfa
Summary:
Follow up from D22819791.
We want to use bookmark update delay only in scs, so let's configure it this
way
Reviewed By: krallin
Differential Revision: D22847143
fbshipit-source-id: b863d7fa4bf861ffe5d53a6a2d5ec44e7f60eb1a
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 overall goal of this stack is to add WarmBookmarksCache support to
repo_client to make Mononoke more resilient to lands of very large
commits.
We'd like to use WarmBookmarkCache in repo client, and to do that we need to be
able to tell Publishing and PullDefault bookmarks apart. Let's teach
WarmBookmarksCache about it.
Reviewed By: krallin
Differential Revision: D22812478
fbshipit-source-id: 2642be5c06155f0d896eeb47867534e600bbc535
Summary:
This method will be used in the next diff to add a test, but it might be more
useful later as well.
Note that `update()` method in BookmarkTransaction already handles publishing bookmarks correctly
Reviewed By: farnz
Differential Revision: D22817143
fbshipit-source-id: 11cd7ba993c83b3c8bca778560af4a360f892b03
Summary:
Added a new query function to get the largest log id from bookmarks_update_log.
In repo_import tool once we move a bookmark to reveal commits to users, we want to check if hg_sync has received the commits. To do this, we extract the largest log id from bookmarks_update_log to compare it with the mutable_counter value related to hg_sync. If the counter value is larger or equal to the log id, we can move the bookmark to the next batch of commits.
Since this query wasn't implemented before, this diff add this functionality.
Next step: add query for mutable_counter
Reviewed By: krallin
Differential Revision: D22816538
fbshipit-source-id: daaa4e5159d561e698c6e1874dd8822546c699c7
Summary: Use `pub(crate)` visibility to share the `SelectBookmark` query between modules.
Reviewed By: StanislavGlebik
Differential Revision: D22464059
fbshipit-source-id: 269561f5ab936b730ce2052e50173134ce241ff8
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:
The dbbookmarks crate is getting too large for a single file. Split it up into
a `store` module, which implements the bookmarks traits, and a `transaction`
module, which handles bookmark transactions.
Reviewed By: krallin
Differential Revision: D22437088
fbshipit-source-id: 629b62de151400cdbf56d502aef061df46c3da81
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:
Add a new parameter, `pagination`, to the `list` method of the `Bookmarks` trait.
This restricts the returned bookmarks to those lexicographically after the
given bookmark name (exclusive). This can be use to implement pagination:
callers can provide the last bookmark in the previous page to fetch the
next page of bookmarks.
Reviewed By: krallin
Differential Revision: D22333943
fbshipit-source-id: 686df545020d936095e29ae5fee24258511f4083
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:
The LIKE pattern used by bookmark prefixes needs to be escaped, otherwise
users looking for bookmarks containing `\`, `_` or `%` will get the
wrong results.
Reviewed By: krallin
Differential Revision: D22336716
fbshipit-source-id: 99b0ad6097f096358e66042752e4d153359935be
Summary:
Whether a bookmark is publishing or not is not specific to Mercurial - it also affects
whether a commit is draft, so it is interesting to the Bonsai world.
Rename `BookmarkHgKind` to `BookmarkKind` to make this clear.
Since negatives are more awkward to work with, rename `PublishingNotPullDefault` to
`Publishing` and `PullDefault` to `PullDefaultPublishing` to make it clearer that
pull-default bookmarks are also publishing.
We can't rename the database column, so that remains as `hg_kind`.
Reviewed By: StanislavGlebik
Differential Revision: D22307782
fbshipit-source-id: 9e686a98cc5eaf9af722fa62fac5ffd4844967fd
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: 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:
Remove unused dependencies for Rust targets.
This failed to remove the dependencies in eden/scm/edenscmnative/bindings
because of the extra macro layer.
Manual edits (named_deps) and misc output in P133451794
Reviewed By: dtolnay
Differential Revision: D22083498
fbshipit-source-id: 170bbaf3c6d767e52e86152d0f34bf6daa198283
Summary:
I observed that for whatever reason our setting of `use_try_shorthand = true` in rustfmt.toml was causing entire functions to not get processed by rustfmt. Even files that contain neither `try` nor `?`. Remove it and reformat fbsource.
Documentation of that config:
- https://github.com/rust-lang/rustfmt/blob/master/Configurations.md#use_try_shorthand
We don't particularly care about the value anymore because nobody writes `r#try!(...)` in 2018 edition code.
Minimized:
```
fn f() {
g(
)
// ...
.h
}
```
This function gets formatted only if use_try_shorthand is not set.
The bug is fixed in the rustfmt 2.0 release candidate.
Reviewed By: jsgf
Differential Revision: D21878162
fbshipit-source-id: b028673c0eb703984d24bf0d2983453fc2a8c212
Summary:
Something was up yesterday with the warm bookmarks cache. It started failing on
some hosts, and went out of sync for > 1 hour on some hosts. The logs reported
a lot of failures, but without any context they weren't super useful:
P130438422.
This adds a bit more logging. If this happens again, we'll be able to better
understand what happened.
Reviewed By: StanislavGlebik
Differential Revision: D21447043
fbshipit-source-id: 67a3924c4486991df5e4d38a995ff8054c145cf9
Summary:
We have a number of error enums that wrap an existing errors, but fail to
register the underlying error as a `#[source]`. This results in truncated
context chains when we print the error. This fixes that. It also removes a
bunch of manual `From` implementation that can be provided by thiserror's
`#[from]`.
This also required updating the `Display` implementation for those errors. I've
opted for not printing the underlying error, since the context chain will
include it. This does mean that if we print one of those errors without the
context chain (i.e. `{}` as opposed to `{:#}` or `{:?}`), then we'll lose out a
bit of context. That said, this should be OK, as we really shouldn't ever being
do this, because we'd be missing the rest of the chain anyways.
Reviewed By: StanislavGlebik
Differential Revision: D21399490
fbshipit-source-id: a970a7ef0a9404e51ea3b59d783ceb7bf33f7328
Summary: Let's make it tunable, more context in D21300885
Reviewed By: ikostia, krallin
Differential Revision: D21347636
fbshipit-source-id: 4bc91effdd212a3f5c4a0c3d4952f52a1cf417d7
Summary:
Microwave doesn't normally allow writes, which can cause cache warmup to fail
if master has underived commits. So, let's go back in bookmarks history to
whatever is most recent and derived. We can do so using the existing logic we
use in the warm bookmarks cache.
Reviewed By: farnz
Differential Revision: D21325485
fbshipit-source-id: 11e758cd512a22e02704ac34458fead18c284c20
Summary:
This is needed because the tonic crate (see the diff stack) relies on tokio ^0.2.13
We can't go to a newer version because a bug that affects mononoke was introduced on 0.2.14 (discussion started on T65261126). The issue was reported upstream https://github.com/tokio-rs/tokio/issues/2390
This diff simply changed the version number on `fbsource/third-party/rust/Cargo.toml` and ran `fbsource/third-party/rust/reindeer/vendor`.
Also ran `buck run //common/rust/cargo_from_buck:cargo_from_buck` to fix the tokio version on generated cargo files
Reviewed By: krallin
Differential Revision: D21043344
fbshipit-source-id: e61797317a581aa87a8a54e9e2ae22655f22fb97
Summary:
We had accumulated lots of unused dependendencies, and had several test_deps in deps instead. Clean this all up to reduce build times and speed up autocargo processing.
Net removal is of around 500 unneeded dependency lines, which represented false dependencies; by removing them, we should get more parallelism in dev builds, and less overbuilding in CI.
Reviewed By: krallin, StanislavGlebik
Differential Revision: D20999762
fbshipit-source-id: 4db3772cbc3fb2af09a16601bc075ae8ed6f0c75
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: I'd like to get the timestamps here without needing to clone them.
Reviewed By: StanislavGlebik
Differential Revision: D20603308
fbshipit-source-id: 2d8f72b4fb3a3eed33b58dc2f0fb1a857bb3f5b9
Summary: This diff makes it possible to relay on the thrift structures from configerator in OSS.
Reviewed By: ahornby
Differential Revision: D20279457
fbshipit-source-id: 39df1c7a6f78b10a0a5bdeeebe476249ab674cc8
Summary: Most of our keys are prefixed, let's prefix this as well
Differential Revision: D20536231
fbshipit-source-id: 9a2ffecfc7de46d109a9fba2444212735cacbebf
Summary:
Let's log what's the current max staleness in warm bookmark cache. The staleness is reported by bookmark updaters using the timestamp from bookmark update log.
To do that I extended `live_updaters` to also include the state of the updater, which bookmark coordinator collects, extracts staleness and logs it to ods
Reviewed By: krallin
Differential Revision: D20468423
fbshipit-source-id: 7f9aacc2ab5bc62c2aed123b8a58d9fc6d49c63c
Summary:
We are planning to expand our usage of warm bookmark cache beyond scs server.
In particular we'd like to use it in mononoke server. But before we do that let's
make warm bookmark cache a bit more reliable and predictable.
In particular, let's make sure that slow update of a single bookmark doesn't
block progress of all other bookmarks - for example, we don't want to block
updating master if we have problems with a single release bookmark. See more
discussion about it in D20161458. [1]
In order to do that we now update every bookmark separately - bookmarks updater
job spawns updater for a single bookmark and makes sure there's no more than a
single updater for a given bookmark.
A few caveats:
1) After this diff we no longer preserve an order of updates of a bookmark i.e.
even if bookmark A was updated first then bookmark B, warm bookmark cache might
see updated in different order. We expect it to not matter much with the only
caveat being stable and master boomarks - it'd be weird to have stable being
descendant of master. This glitch should only happen for a very brief time
period of time, so hopefully it shouldn't matter in practice.
2) Current implementation doesn't stop single bookmark updaters if the main
updater was cancelled. TBH, I don't think it's necessary.
In the next diff I'll add ods counters to track the delay between warm bookmark cache and actual
values of bookmarks
[1] Previously we wanted to update bookmarks in bookmark update log order, but
we decided it's not a great idea. See D20161458 for more details.
Reviewed By: krallin
Differential Revision: D20335195
fbshipit-source-id: 0b1242faa1a9ef286929132c2350c299a2594467
Summary:
Previously we could have "Started ..." before "Starting ..."
This diff fixes it.
Reviewed By: krallin
Differential Revision: D20277406
fbshipit-source-id: 3c2f3fa1723c2e0852c6b114592ab7ad90be17ff
Summary:
Context: https://fb.workplace.com/groups/rust.language/permalink/3338940432821215/
This codemod replaces all dependencies on `//common/rust/renamed:tokio-preview` with `fbsource//third-party/rust:tokio-preview` and their uses in Rust code from `tokio_preview::` to `tokio::`.
This does not introduce any collisions with `tokio::` meaning 0.1 tokio because D20235404 previously renamed all of those to `tokio_old::` in crates that depend on both 0.1 and 0.2 tokio.
This is the tokio version of what D20213432 did for futures.
Codemod performed by:
```
rg \
--files-with-matches \
--type-add buck:TARGETS \
--type buck \
--glob '!/experimental' \
--regexp '(_|\b)rust(_|\b)' \
| sed 's,TARGETS$,:,' \
| xargs \
-x \
buck query "labels(srcs, rdeps(%Ss, //common/rust/renamed:tokio-preview, 1))" \
| xargs sed -i 's,\btokio_preview::,tokio::,'
rg \
--files-with-matches \
--type-add buck:TARGETS \
--type buck \
--glob '!/experimental' \
--regexp '(_|\b)rust(_|\b)' \
| xargs sed -i 's,//common/rust/renamed:tokio-preview,fbsource//third-party/rust:tokio-preview,'
```
Reviewed By: k21
Differential Revision: D20236557
fbshipit-source-id: 15068b93a0a944d6249a1d9f63840a4c61c9c1ba
Summary:
Implementation of derivation logic for the changeset info.
BonsaiDerived is implemented for the ChangesetInfo. `derive_from_parents` just derives an info and BonsaiDerivedMapping then puts it into the blobstore.
```
ChangesetInfo::derive(..) -> ChacgesetInfo
```
Reviewed By: krallin
Differential Revision: D20185954
fbshipit-source-id: afe609d1b2711aed7f2740714df6b9417c6fe716
Summary:
Previously warm bookmark cache tried to derive all bookmarks on startup. It slows down the startup time and in some cases it might prevent scs server from starting up at all.
Let's change how warm bookmark cache initializes the bookmarks - instead of trying to derive all of them let's move underived bookmarks back in history.
Reviewed By: krallin
Differential Revision: D20195211
fbshipit-source-id: 5cb5d8599d3035973175d3063186a7c01536889a
Summary:
Context: https://fb.workplace.com/groups/rust.language/permalink/3338940432821215/
This codemod replaces *all* dependencies on `//common/rust/renamed:futures-preview` with `fbsource//third-party/rust:futures-preview` and their uses in Rust code from `futures_preview::` to `futures::`.
This does not introduce any collisions with `futures::` meaning 0.1 futures because D20168958 previously renamed all of those to `futures_old::` in crates that depend on *both* 0.1 and 0.3 futures.
Codemod performed by:
```
rg \
--files-with-matches \
--type-add buck:TARGETS \
--type buck \
--glob '!/experimental' \
--regexp '(_|\b)rust(_|\b)' \
| sed 's,TARGETS$,:,' \
| xargs \
-x \
buck query "labels(srcs, rdeps(%Ss, //common/rust/renamed:futures-preview, 1))" \
| xargs sed -i 's,\bfutures_preview::,futures::,'
rg \
--files-with-matches \
--type-add buck:TARGETS \
--type buck \
--glob '!/experimental' \
--regexp '(_|\b)rust(_|\b)' \
| xargs sed -i 's,//common/rust/renamed:futures-preview,fbsource//third-party/rust:futures-preview,'
```
Reviewed By: k21
Differential Revision: D20213432
fbshipit-source-id: 07ee643d350c5817cda1f43684d55084f8ac68a6
