Summary:
Like it says in the title. This adds a crate that provides a combinator that
lets us easily find stalls caused by futures that stay in `poll()` for too
long.
The goal is to make this minimal overhead for whoever is using it: all you need
is to import it + give it a logger. It automatically looks up the line where
it's called and gives it back to you in logs. This uses the `track_caller`
functionality to make this work.
Reviewed By: farnz
Differential Revision: D26250068
fbshipit-source-id: a1458e5adebac7eab6c2de458f679c7215147937
Summary: Configure segmented changelog to use caching when caching is requested.
Reviewed By: krallin
Differential Revision: D26121496
fbshipit-source-id: d0711a5939b5178b3a93d081019cfab47996da40
Summary:
krallin noticed that we aren't using warm bookmark cache anymore. Turned out
the reason was in the fact that client uses `listkeyspatterns` call to fetch
bookmarks and not `listkeys`. This diff makes `listkeyspatterns` use warm
bookmark cache as well.
Reviewed By: markbt
Differential Revision: D26124605
fbshipit-source-id: 637db8d66934cabc1793f9f615fefddd07c3af62
Summary:
Lots of generated code in this diff. Only code change was in
`common/rust/cargo_from_buck/lib/cargo_generator.py`.
Path/git-only dependencies (ie `mydep = { path = "../foo/bar" }`) are not
publishable to crates.io. However, we are allowed to specify both a path/git
_and_ a version. When building locally, the path/git is chosen. When publishing,
the version on crates.io is chosen.
See https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#multiple-locations .
Note that I understand that not all autocargo projects are published on crates.io (yet).
The point of this diff is to allow projects to slowly start getting uploaded.
The end goal is autocargo generated `Cargo.toml`s that can be `cargo publish`ed
without further modification.
Reviewed By: lukaspiatkowski
Differential Revision: D26028982
fbshipit-source-id: f7b4c9d4f4dd004727202bd98ab10e201a21e88c
Summary:
When we tried to update to Tokio 0.2.14, we hit lots of hangs. Those were due
to incompatibilities between Tokio 0.2.14 and Futures 1.29. We fixed some of
the bugs (and others had been fixed and were pending a release), and Futures
1.30 have now been released, which unblocks our update.
This diff updates Tokio accordingly (the previous diff in the stack fixes an
incompatibility).
The underlying motivation here is to ease the transition to Tokio 1.0.
Ultimately we'll be pulling in those changes one or way or another, so let's
get started on this incremental first step.
Reviewed By: farnz
Differential Revision: D25952428
fbshipit-source-id: b753195a1ffb404e0b0975eb7002d6d67ba100c2
Summary:
This feature is useful for testing time-dependent stuff (e.g. it
allows you to stop/forward time). It's already included in the buck build.
Reviewed By: SkyterX
Differential Revision: D25946732
fbshipit-source-id: 5e7b69967a45e6deaddaac34ba78b42d2f2ad90e
Summary: Convert `Changsets` trait and all its uses to new type futures
Reviewed By: krallin
Differential Revision: D25638875
fbshipit-source-id: 947423e2ee47a463861678b146641bcc6b899a4a
Summary: Simplify open_blobrepo_given_datasources parameters to pass less arguments, make it so can pass the sql_factory by reference.
Reviewed By: krallin
Differential Revision: D25524254
fbshipit-source-id: c324127f42c53a52f388d303e310014f4fa0d7bb
Summary:
In the next diff I'm going to add Mysql connection object to `MysqlOptions` in order to pass it down from `MononokeAppData` to the code that works with sql.
This change will make MysqlOptions un-copyable.
This diff fixed all issues produced by the change.
Reviewed By: ahornby
Differential Revision: D25590772
fbshipit-source-id: 440ae5cba3d49ee6ccd2ff39a93829bcd14bb3f1
Summary:
Like it says in the title. This is nice to do because we had old futures
wrapping new futures here, so this lets us get rid of a lot of cruft.
Reviewed By: ahornby
Differential Revision: D25502648
fbshipit-source-id: a34973b32880d859b25dcb6dc455c42eec4c2f94
Summary: Convert all BlobRepoHg methods to new type futures
Reviewed By: StanislavGlebik
Differential Revision: D25471540
fbshipit-source-id: c8e99509d39d0e081d082097cbd9dbfca431637e
Summary:
Right now, when we upload a hg commit, we check that we have all the content
the client is referencing.
The only problem is we do this by checking that the filenodes the client
mentions exist, but the way we store filenodes right now is we write them
concurrently with content blobs, so it is in fact possible to have a filenode
that references a piece of content that doesn't actually exist.
That isn't quite what one might call satisfactory when it comes to checking the
content does in fact exist, so this diff updates our content checking
In practice, with the way Mononoke works right now this should be quite free:
the client uploads everything all the time, and then we check later, so this
will just hit in the blobstore cache.
In a future world where clients don't upload stuff they already know we have,
that could be slower, but doing it the way is we do it before is simply not
correct, so that's not much better. The ways to make it faster would be:
- Trust that we'll hit in cache when checking for presence (could work out).
- Have the client prove to us that we have this content, and thread it through.
To do the latter, IMO the code here could actually look at the entries that
were actually uploaded, and not check them for presence again, but right now we
have a few layers of indirection that make this a bit tricky (technically, when
`process_one_entry` gets called, that means "I uploaded this", but nothing in
the signature of any of the functions involved really enforces that).
Reviewed By: StanislavGlebik
Differential Revision: D25422596
fbshipit-source-id: 3cf34d38bd6ed1cd83d93c778f04395c942b26c0
Summary:
Change derived data config to have "enabled" config and "backfilling" config.
The `Mapping` object has the responsibility of encapsulating the configuration options
for the derived data type. Since it is only possible to obtain a `Mapping` from
appropriate configuration, ownership of a `Mapping` means derivation is permitted,
and so the `DeriveMode` enum is removed.
Most callers will use `BonsaiDerived::derive`, or a default `derived_data_utils` implementation
that requires the derived data to be enabled and configured on the repo.
Backfillers can additionally use `derived_data_utils_for_backfill` which will use the
`backfilling` configuration in preference to the default configuration.
Reviewed By: ahornby
Differential Revision: D25246317
fbshipit-source-id: 352fe6509572409bc3338dd43d157f34c73b9eac
Summary:
The `BonsaiDerived` trait is split in two:
* The new `BonsaiDerivable` trait encapsulates the process of deriving the data, either
a single item from its parents, or a batch.
* The `BonsaiDerived` trait is used only as an entry point for deriving with the default
mapping and config.
This split will allow us to use `BonsaiDerivable` in batch backfilling with non-default
config, for example when backfilling a new version of a derived data type.
Reviewed By: krallin
Differential Revision: D25371964
fbshipit-source-id: 5874836bc06c18db306ada947a690658bf89723c
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: 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:
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.
Reviewed By: StanislavGlebik
Differential Revision: D25333450
fbshipit-source-id: 49ad4b1964a4dfd9f3e0108fa421d451ef905256
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:
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: Convert `ChangesetFetcher` to new type futures
Reviewed By: StanislavGlebik
Differential Revision: D25244213
fbshipit-source-id: 4207386d81397a930a566db008019bb8f31bf602
Summary:
Move expected_item_size_byte into CachelibSettings, seems like it should be there.
To enable its use also exposes a parse_and_init_cachelib method for callers that have different defaults to default cachelibe settings.
Reviewed By: krallin
Differential Revision: D24761024
fbshipit-source-id: 440082ab77b5b9f879c99b8f764e71bec68d270e
Summary: convert mercurial_derived_data to new type futures
Reviewed By: ahornby
Differential Revision: D25220329
fbshipit-source-id: c2532a12e915b315fe6eb72f122dbc37822bbb2a
Summary:
This diff prepares the Mononoke codebase for composition-based extendability of
`ScubaSampleBuilder`. Specifically, in the near future I will add:
- new methods for verbose scuba logging
- new data field (`ObservabilityContext`) to check if verbose logging should
be enabled or disabled
The higher-level goal here is to be able to enable/disable verbose Scuba
logging (either overall or for certain slices of logs, like for a certain
session id) in real time, without restarting Mononoke. To do so, I plan to
expose the aforementioned verbose logging methods, which will run a check
against the stored `ObservabilityContext` and make a decision of whether the
logging is enabled or not. `ObservabilityContext` will of course hide
implementation details from the renamed `ScubaSampleBuilderExt`, and just provide a yes/no
answer based on the current config and sample fields.
At the moment this should be a completely harmless change.
Reviewed By: krallin
Differential Revision: D25211089
fbshipit-source-id: ea03dda82fadb7fc91a2433e12e220582ede5fb8
Summary:
- convert save_bonsai_changesets to new type futures
- `blobrepo:blobrepo` is free from old futures deps
Reviewed By: StanislavGlebik
Differential Revision: D25197060
fbshipit-source-id: 910bd3f9674094b56e1133d7799cefea56c84123
Summary: convert `BlobRepo::get_bonsai_bookmark` to new type futures
Reviewed By: StanislavGlebik
Differential Revision: D25188577
fbshipit-source-id: fb6f2b592b9e9f76736bc1af5fa5a08d12744b5f
Summary: Prefix old futures with `Old` so it would be possible to start conversion of BlobRepo to new type futures
Reviewed By: StanislavGlebik
Differential Revision: D25187882
fbshipit-source-id: d66debd2981564b289d50292888f3f6bd3343e94
Summary:
We have `HgBlobChangeset`, `HgFileEnvelope`, `HgManifestEnvelope` ... but we
also have `BlobManifest`. Let's be a little consistent.
Reviewed By: markbt
Differential Revision: D25122288
fbshipit-source-id: 9ae0be49986dbcc31cee9a46bd30093b07795c62
Summary:
We have 4 different ways of awaiting futures in there: sometimes we create a
runtime, sometimes we use async-unit, sometimes we use `fbinit::test` and
sometimes we use `fbinit::compat_test`. Let's be consistent.
While in there, let's also get rid of `should_panic`, since that's not a very
good way of running tests.
Reviewed By: HarveyHunt
Differential Revision: D25186195
fbshipit-source-id: b64bb61935fb2132d2e5d8dff66fd4efdae1bf64
Summary:
HgBlobEntry is kind of a problematic type right now:
- It has no typing to indicate if it's a file or a manifest
- It always has a file type, but we only sometimes care about it
This results in us using `HgBlobEntry` to sometimes represent
`Entry<HgManifestId, (FileType, HgFileNodeId)>`, and some other times to
represent `Entry<HgManifestId, HgFileNodeId>`.
This makes code a) confusing, b) harder to refactor because you might be having
to change unrelated code that cares for the use case you don't care about (i.e.
with or without the FileType), and c) results in us sometimes just throwing in
a `FileType::Normal` because we don't care about the type, which is prone to
breaking stuff.
So, this diff just removes it, and replaces it with the appropriate types.
Reviewed By: farnz
Differential Revision: D25122291
fbshipit-source-id: e9c060c509357321a8059d95daf22399177140f1
Summary: Remove 'static requirement for async methods of Blobstore, propagate this change and fixup low hanging fruits where the code can become 'static free easily.
Reviewed By: ahornby, farnz
Differential Revision: D24839054
fbshipit-source-id: 5d5daa04c23c4c9ae902b669b0a71fe41ee6dee6
Summary:
This isn't actually being consulted anywhere save for a single test, so let's
just remove it (it's not like the test checks anything important — that field
might not as well exist given we never read it).
Reviewed By: farnz
Differential Revision: D25093494
fbshipit-source-id: 5f4a53f8666fc0e8a89ceade44baa96e71fb813f