Summary:
I'd like to have some socket level logging, and it's good to capture the
metadata there so that we have client identities, session ids, etc.
But I can't do this if I don't have the metadata! This updates our code so that the
metadata is available where this logging will go.
This means we can't rely on loggers in `try_convert_preamble_to_metadata`, but
given this is now only used for hgcli, I don't think that matters, so I just removed it.
Reviewed By: mitrandir77
Differential Revision: D28383121
fbshipit-source-id: 27532f021a9082e74403bba73cad4fc6d0a6d97c
Summary:
I'd like to have some socket level logging, and it's good to capture the
metadata there so that we have client identities, session ids, etc.
To do so I need to not move it into the session. This does that.
Reviewed By: mitrandir77
Differential Revision: D28383123
fbshipit-source-id: 3fd10c3720465824dbcb2528227cbb3521d3068a
Summary: Log the blobstore id as part of sampled pack info. This is allows running the walker pack info logging directly agains a multiplex rather than invoke it for one component at a time.
Reviewed By: farnz
Differential Revision: D28264093
fbshipit-source-id: 0502175200190527b7cc1cf3c48b8154c8b27c90
Summary:
This is going to enable the background update in SegmentedChangelog to log
entries to Scuba.
The scuba sample builder is not fundamentally different than other elements of
the environment. It is used slightly differently to, for example, Logger,
because it has to cloned in all places that want to log rows but otherwise it
has the same characteristics.
Reviewed By: krallin
Differential Revision: D28210008
fbshipit-source-id: 68468868d13f29dddf21095bd7526cb4ff690786
Summary:
Rename `new_with_containers` to `new`. `new` the standard name for
what the fuction does.
Reviewed By: krallin
Differential Revision: D28210005
fbshipit-source-id: 9ad0dad5e8afd89d7538fd32d93de10837e26f9d
Summary:
Refactoring the construction logic for CoreContext.
`test_mock_class` is used in only one place. That usage can be replaced with
`test_mock_session`.
Reviewed By: krallin
Differential Revision: D28210007
fbshipit-source-id: c81ca31cb7255ef640fe641e9b38f239cfb69267
Summary:
Refactoring CoreContext construction to express purpose. We will have
a constructor for request processing, for background processing.
Bulk processing is another category that has it's own constructor
already. Renaming it to make it more prominent.
Reviewed By: krallin
Differential Revision: D28210006
fbshipit-source-id: 2bb74d97e2f3588aa539e58c3d6dd6842f898121
Summary: Upstream crate has landed my PR for zstd 1.4.9 support and made a release, so can remove this patch now.
Reviewed By: ikostia
Differential Revision: D28221163
fbshipit-source-id: b95a6bee4f0c8d11f495dc17b2737c9ac9142b36
Summary:
Quicksand is about 60% of our logging, but we don't really need to log
everything it does because the workload is pretty much the same across all
hosts.
Reviewed By: HarveyHunt
Differential Revision: D28225416
fbshipit-source-id: 68c0f68c4da73e0f7543754c917d7efa4bbf374e
Summary:
Wireproto session multiplexes wireproto commands. Counting them individually is most likely a better metric for QPS even though we wouldn't be able to offload them to a different server/region one by one.
It makes the cost of a query more even across wireproto and edenapi.
Reviewed By: krallin
Differential Revision: D28058054
fbshipit-source-id: 5d606841e07816ec8808a3b9aba4b7c0614b9cb6
Summary:
We used to carry patches for Tokio 0.2 to add support for disabling Tokio coop
(which was necessary to make Mononoke work with it), but this was upstreamed
in Tokio 1.x (as a different implementation), so that's no longer needed. Nobody
else besides Mononoke was using this.
For Hyper we used to carry a patch with a bugfix. This was also fixed in Tokio
1.x-compatible versions of Hyper. There are still users of hyper-02 in fbcode.
However, this is only used for servers and only when accepting websocket
connections, and those users are just using Hyper as a HTTP client.
Reviewed By: farnz
Differential Revision: D28091331
fbshipit-source-id: de13b2452b654be6f3fa829404385e80a85c4420
Summary:
This used to be used by Mononoke, but we're now on Tokio 1.x and on
corresponding versions of Gotham so it's not needed anymore.
Reviewed By: farnz
Differential Revision: D28091091
fbshipit-source-id: a58bcb4ba52f3f5d2eeb77b68ee4055d80fbfce2
Summary:
NOTE: there is one final pre-requisite here, which is that we should default all Mononoke binaries to `--use-mysql-client` because the other SQL client implementations will break once this lands. That said, this is probably the right time to start reviewing.
There's a lot going on here, but Tokio updates being what they are, it has to happen as just one diff (though I did try to minimize churn by modernizing a bunch of stuff in earlier diffs).
Here's a detailed list of what is going on:
- I had to add a number `cargo_toml_dir` for binaries in `eden/mononoke/TARGETS`, because we have to use 2 versions of Bytes concurrently at this time, and the two cannot co-exist in the same Cargo workspace.
- Lots of little Tokio changes:
- Stream abstractions moving to `tokio-stream`
- `tokio::time::delay_for` became `tokio::time::sleep`
- `tokio::sync:⌚:Sender::send` became `tokio::sync:⌚:Sender::broadcast`
- `tokio::sync::Semaphore::acquire` returns a `Result` now.
- `tokio::runtime::Runtime::block_on` no longer takes a `&mut self` (just a `&self`).
- `Notify` grew a few more methods with different semantics. We only use this in tests, I used what seemed logical given the use case.
- Runtime builders have changed quite a bit:
- My `no_coop` patch is gone in Tokio 1.x, but it has a new `tokio::task::unconstrained` wrapper (also from me), which I included on `MononokeApi::new`.
- Tokio now detects your logical CPUs, not physical CPUs, so we no longer need to use `num_cpus::get()` to figure it out.
- Tokio 1.x now uses Bytes 1.x:
- At the edges (i.e. streams returned to Hyper or emitted by RepoClient), we need to return Bytes 1.x. However, internally we still use Bytes 0.5 in some places (notably: Filestore).
- In LFS, this means we make a copy. We used to do that a while ago anyway (in the other direction) and it was never a meaningful CPU cost, so I think this is fine.
- In Mononoke Server it doesn't really matter because that still generates ... Bytes 0.1 anyway so there was a copy before from 0.1 to 0.5 and it's from 0.1 to 1.x.
- In the very few places where we read stuff using Tokio from the outside world (historical import tools for LFS), we copy.
- tokio-tls changed a lot, they removed all the convenience methods around connecting. This resulted in updates to:
- How we listen in Mononoke Server & LFS
- How we connect in hgcli.
- Note: all this stuff has test coverage.
- The child process API changed a little bit. We used to have a ChildWrapper around the hg sync job to make a Tokio 0.2.x child look more like a Tokio 1.x Child, so now we can just remove this.
- Hyper changed their Websocket upgrade mechanism (you now need the whole `Request` to upgrade, whereas before that you needed just the `Body`, so I changed up our code a little bit in Mononoke's HTTP acceptor to defer splitting up the `Request` into parts until after we know whether we plan to upgrade it.
- I removed the MySQL tests that didn't use mysql client, because we're leaving that behind and don't intend to support it on Tokio 1.x.
Reviewed By: mitrandir77
Differential Revision: D26669620
fbshipit-source-id: acb6aff92e7f70a7a43f32cf758f252f330e60c9
Summary:
Currently when we call with_mutated_scuba() we create a new LoggingContainer.
That means that all the data from the previous LoggingContainer like PerfCounters
is lost. I suspect this is the reason we don't log any BlobGets/Puts for
repo_create_commit methods (see
[scuba](https://fburl.com/scuba/mononoke_scs_server/fautos3s)) - we call
[with_mutated_scuba method](https://fburl.com/code/srd1c4xu) right before
calling repo_create_commit(), and I suspect this loses the counters.
Let's instead copy all the Logging fields when calling `with_mutated_scuba`.
Reviewed By: krallin
Differential Revision: D27964719
fbshipit-source-id: 881c11bb5fb1927dbf55d0d625ea8bfbf11be131
Summary:
Update the zstd crates.
This also patches async-compression crate to point at my fork until upstream PR https://github.com/Nemo157/async-compression/pull/117 to update to zstd 1.4.9 can land.
Reviewed By: jsgf, dtolnay
Differential Revision: D27942174
fbshipit-source-id: 26e604d71417e6910a02ec27142c3a16ea516c2b
Summary:
I'd like to be able to track the proportion of traffic coming to bookmarks from
warm bookmarks cache vs. from elsewhere. We don't have a great abstraction to
pass this via the CoreContext at this time, but the SessionClass seems like a
pretty good fit.
Indeed, since it's always available in the CoreContext, and can be freely
mutated without having to rebuild the whole session. Besides, it aligns pretty
well with the existing use cases we have for SessionClass, which is to give you
different level of service depending on who you are.
Reviewed By: StanislavGlebik
Differential Revision: D27938413
fbshipit-source-id: a9dcc5a10c8d1459ee9586324a727c668e2e4e40
Summary:
We didn't log it to perf counters log, and that makes it hard to aggregate,
show distributions etc
Let's start doing that
Reviewed By: krallin
Differential Revision: D27856968
fbshipit-source-id: 82fbba70154ee011073f3122256bd296bbb938ae
Summary:
To prevent bonsai changeset divergence between prod and backup repo by copying
bonsais from prod repo directly during hg sync job push.
See more details about motivation in D27824210
Reviewed By: ikostia
Differential Revision: D27852341
fbshipit-source-id: 93e0b1891008858eb99d5e692e4dd60c2e23f446
Summary:
Like it says in the title. There's no reason for this to be ad ad-hoc "throw in
an arg" when everything else is done by adding arg types.
Reviewed By: HarveyHunt
Differential Revision: D27791333
fbshipit-source-id: 38e5a479800179b249ace5cc599340cb84eb53e2
Summary:
Like it says in the title. Let's remove ad-hoc "add an arg then look the arg"
mechanisms like this one.
Reviewed By: HarveyHunt
Differential Revision: D27791334
fbshipit-source-id: 257cea7763ab5130525ad739fe4ebdda4e8bfeb6
Summary:
There is a very frustrating operation that happens often when working on the
Mononoke code base:
- You want to add a flag
- You want to consume it in the repo somewhere
Unfortunately, when we need to do this, we end up having to thread this from a
million places and parse it out in every single main() we have.
This is a mess, and it results in every single Mononoke binary starting with
heaps of useless boilerplate:
```
let matches = app.get_matches();
let (caching, logger, mut runtime) = matches.init_mononoke(fb)?;
let config_store = args::init_config_store(fb, &logger, &matches)?;
let mysql_options = args::parse_mysql_options(&matches);
let blobstore_options = args::parse_blobstore_options(&matches)?;
let readonly_storage = args::parse_readonly_storage(&matches);
```
So, this diff updates us to just use MononokeEnvironment directly in
RepoFactory, which means none of that has to happen: we can now add a flag,
parse it into MononokeEnvironment, and get going.
While we're at it, we can also remove blobstore options and all that jazz from
MononokeApiEnvironment since now it's there in the underlying RepoFactory.
Reviewed By: HarveyHunt
Differential Revision: D27767700
fbshipit-source-id: e1e359bf403b4d3d7b36e5f670aa1a7dd4f1d209
Summary:
ScrubOptions normally represents options we parsed from the CLI, but right now
we abuse this a little bit to throw a ScrubHandler into them, which we
sometimes mutate before using this config.
In this stack, I'm unifying how we pass configs to RepoFactory, and this little
exception doesn't really fit. So, let's change this up, and make ScrubHandler
something you may give the RepoFactory if you're so inclined.
Reviewed By: HarveyHunt
Differential Revision: D27767699
fbshipit-source-id: fd38bf47eeb723ec7d62f8d34e706d8581a38c43
Summary:
Basically every single Mononoke binary starts with the same preamble:
- Init mononoke
- Init caching
- Init logging
- Init tunables
Some of them forget to do it, some don't, etc. This is a mess.
To make things messier, our initialization consists of a bunch of lazy statics
interacting with each other (init logging & init configerator are kinda
intertwined due to the fact that configerator wants a logger but dynamic
observability wants a logger), and methods you must only call once.
This diff attempts to clean this up by moving all this initialization into the
construction of MononokeMatches. I didn't change all the accessor methods
(though I did update those that would otherwise return things instantiated at
startup).
I'm planning to do a bit more on top of this, as my actual goal here is to make
it easier to thread arguments from MononokeMatches to RepoFactory, and to do so
I'd like to just pass my MononokeEnvironment as an input to RepoFactory.
Reviewed By: HarveyHunt
Differential Revision: D27767698
fbshipit-source-id: 00d66b07b8c69f072b92d3d3919393300dd7a392
Summary:
I want to call something else MononokeEnvironment (the environment the whole
binary is running in), so let's rename this one.
Reviewed By: StanislavGlebik
Differential Revision: D27767696
fbshipit-source-id: bd6f2f282a7fc1bc09926a0286ecb8a5777a0a24
Summary: SQLBlob doesn't benefit from sharing a pool with other MySQL users, but does benefit from more aggressive connection timeouts. Give it its own pool, which we can tweak later.
Reviewed By: krallin
Differential Revision: D27651133
fbshipit-source-id: 8f5216ec0506b217f9365babfe1ebac00f68a9a9
Summary:
Now that the `hg_external_sync` jobs are gone we can delete the code
in Mononoke that behaves differently when a sync job connects.
Reviewed By: StanislavGlebik
Differential Revision: D27500506
fbshipit-source-id: 443fb54577833dbf44ece6ae90a5f25ffed38cd5
Summary:
This import is only used for the `ReadOnlyStorage` type, which is canonically
defined in `blobstore_factory`.
Reviewed By: krallin
Differential Revision: D27363467
fbshipit-source-id: ed1388e661453e1b434c83af63c76da1eea1bce1
Summary:
Switch from `blobrepo_factory` to the new `RepoFactory` to construct `BlobRepo`
in `mononoke_api`.
The factory is part of the `MononokeEnvironment`, and is used to build all of the repos.
Reviewed By: krallin
Differential Revision: D27363473
fbshipit-source-id: 81345969e5899467f01d285c232a510b8edddb17
Summary:
We need to bump SCS counters expressing Mononoke's QPS. They will look something like:
`requests:mononoke:oregon:carolina` for requests coming from proxygen in prn and mononoke in frc.
CSLB expects regions' full names.
We're getting src region from proxygen as a header.
Reviewed By: krallin
Differential Revision: D27082868
fbshipit-source-id: 12accb8a9df5cf6a80c2c281d2f61ac1e68176d1
Summary:
Before I use a new command line argument I need to add it first.
This one will be set to the top level smc tier which is mononoke, mononoke.experimental, etc.
It will be used to bump SCS counters consumed by CSLB.
I don't want this to be optional because I don't want people to accidentally turn off publishing SCS counters representing QPS. On the other hand I don't want people who run it locally to pollute prod counters (in the script that sets this up locally its set to some dommy tier).
Reviewed By: krallin
Differential Revision: D27190020
fbshipit-source-id: 472011f0584012b036efdd015565cda3b3c029e9
Summary:
We use srselect on hg hosts to figure out which Mononoke servers hgcli can connect to.
We were getting BrokenPipe errors most likely from srselect.
`E0318 03:51:02.558708 679 [tk] eden/mononoke/server/repo_listener/src/connection_acceptor.rs:201] connection_acceptor error: Failed to handle connection to [2401:db00:12:90c3:face:0:361:0]:57594: Failed to handle_connection: Failed to handle_http: Failed to serve_connection: error shutting down connection: Broken pipe (os error 32): Broken pipe (os error 32)
`
Tbh it isn't much different than `ErrorKind::NotConnected` because we'are shutting the connection down anyway and it doesn't matter whether client hung up or is already dead.
Reviewed By: krallin
Differential Revision: D27155123
fbshipit-source-id: 96bb2b268f116a20f16605eb04c867c9ad047b1f
Summary:
Unlike the source control service, requests aren't usually cancelled in the
main server. However, if the request doesn't complete within the shutdown
timeout, it does get cancelled.
Add logging for this case.
Reviewed By: krallin
Differential Revision: D27086622
fbshipit-source-id: dbd9dee1a6a84b4cd5570302a0a62fb96d2489aa
Summary:
S3 blobstore is sensitive to the number of open connections and it causes perf
problems. Let's add a way to limit the number of simultaneous open connections.
Also see D26911520
Reviewed By: krallin
Differential Revision: D26911679
fbshipit-source-id: 1e3e4922b26d0a29c94c89b4aa76b9b5928effe6
Summary: Like it says in the title. I also replaced one of our status codes that was wrong.
Reviewed By: johansglock
Differential Revision: D26844865
fbshipit-source-id: b8c1261d0077cf5dc006827e16667e382db7d189
Summary:
It doesn't make a whole lot of sense to user the "tupperware" permission here.
Lets make a distinction in who is allowed to provision mononoke vs. who is allowed to forward identities to it.
I have created a separate action with the same identities allows as in the "tupperware" permission.
https://www.internalfb.com/intern/hipster/acls/view/?type=TIER&consumer=mononoke
Reviewed By: StanislavGlebik
Differential Revision: D26777489
fbshipit-source-id: d3999dc4da7a57ac721572610f65eba664e595e9
Summary:
Atm in tests a separate ConfigStore with file source is created for some configs and then a reference to it is dropped immediately ([see get_config_handle function in mod.rs](https://fburl.com/diffusion/fpkj7ekv)). This is uncomfortable as we may need a reference to e.g. force update configs in tests.
Instead of creating separate stores we can reuse static configerator which already uses local files (in tests).
Reviewed By: krallin
Differential Revision: D26725515
fbshipit-source-id: 24269cd93b7d35216c025807c3f3eb527688b72b
Summary:
This diff rollouts V2 of autocargo in an atomic way so there are quite a few things done here.
Arc lint support:
V1 used to be part of the default fbsource `arc lint` engine, but since V2 calls buck it must live in a separate lint engine. So this diff:
- Adds running `autocargo` as part of `arc lint-rust`
Mergedriver update:
- Mergedriver used in resolving conflicts on commits is now pointing to V2
- It handles files in `public_autocargo/` directories in addition to the ones containig generation preamble
Including regeneration results of running `common/rust/cargo_from_buck/bin/autocargo`. All the differences are accounted for:
- Some sections and attributes are removed as they can be autodiscovered by Cargo (like `lib.path = "src/lib.rs"` or empty [lib] section)
- "readme" attribute is properly defined as relative to Cargo.toml location rather than as hardcoded string
- "unittest = false" on a Buck rule propagates as "test = false; doctest = false" to Cargo
- "rusqlite" is not special-cased anymore, so the "budled" feature will have to be enabled using custom configuration if required by the project (for rust-shed in order to not break windows builds a default feature section was added)
- Files generated from thrift_library rules that do not support "rust" language are removed
- Custom .bzl rules that create rust artifacts (like `rust_python_extension`) are no longer ignored
Others:
- Changed `bin/cargo-autocargo` to be a wrapper for calling V2 via `cargo autocargo`
- Updated following files to use V2:
- `common/rust/tools/reindeer/version-bump`
- `remote_execution/rust/setup.sh`
- Removed few files from V1 that would otherwise interfere with V2 automatic regeneration/linting/testing
Reviewed By: zertosh
Differential Revision: D26728789
fbshipit-source-id: d1454e7ce658a2d3194704f8d77b12d688ec3e64
Summary: We have a number of sleeps in our integration tests. The two main reasons are configs & tunables that need reloading. Currently, we have no way of force-reloading those.
Reviewed By: krallin
Differential Revision: D26615732
fbshipit-source-id: 217c4ae039abd398972b4a9764d08e18d6182493
Summary:
For dependencies V2 puts "version" as the first attribute of dependency or just after "package" if present.
Workspace section is after patch section in V2 and since V2 autoformats patch section then the third-party/rust/Cargo.toml manual entries had to be formatted manually since V1 takes it as it is.
The thrift files are to have "generated by autocargo" and not only "generated" on their first line. This diff also removes some previously generated thrift files that have been incorrectly left when the corresponding Cargo.toml was removed.
Reviewed By: ikostia
Differential Revision: D26618363
fbshipit-source-id: c45d296074f5b0319bba975f3cb0240119729c92
Summary:
Like our other "tokio ecosystem" crates, I'm renaming this one tokio-util-02
(as in tokio-02) since this is the last version it is compatible with. Note that this had a
breaking change as well, so I updated the callsites.
Reviewed By: HarveyHunt
Differential Revision: D26636350
fbshipit-source-id: 30f7da1036c861a97717c8d26648daaae33ecfbd
Summary:
We use a forked version of Gotham in Mononoke. This isn't great, because we
have to maintain this fork. Ideally, we'd upstream our changes, but as is
they're a bit intrusive and not generally useful, which makes this hard.
I've reworked how we do our Gotham changes, and now we only need to make 1 bit
of code public, which might be easier to get upstream. Concretely, Gotham has a
concept of "connected handler" that links a Hyper request and a socket address,
but in our case we want more things. This change lets us instantiate our own
Gotham state, and then add a few more things to it as necessary.
This diff updates our code accordingly to be compatible. This also lets us trim
down on some ceremony we had to do call into Gotham
from Mononoke Server.
Reviewed By: farnz
Differential Revision: D26634653
fbshipit-source-id: 024a48ebc3f323c165ac412ef422755e8cb1c650
Summary:
Like it says in the title, this updates futures_ext to use tokio_shim, which
makes it compatible with Tokio 0.2 and 1.0.
There is one small difference in behavior here, which is that in Tokio 1.0,
sleep isn't Unpin anymore, so callers will need to call `boxed()` or use Tokio's `pin!` macro if they need
Unpin.
I do want to get as close to what upstream is doing in Tokio 1.0, so I think
it's good to keep that behavior.
Reviewed By: farnz
Differential Revision: D26610036
fbshipit-source-id: ff72275da55558fdf8fe3ad009d25cf84e108a5a
Summary:
This was a thing that was only ever used in Mononoke, and we don't think it's
usable and haven't been using it. Let's get rid of it. As-is, it won't even work
for most people due to its (indirect) dependency on Tokio 0.1.
Reviewed By: StanislavGlebik
Differential Revision: D26512243
fbshipit-source-id: faa16683f2adb20dfba43c4768486b982bc02ff9
