Summary: This is useful to get an idea of what the scrub is doing
Reviewed By: farnz
Differential Revision: D28417785
fbshipit-source-id: 1421e0aae13f43371d4c0d066c08aee80b17e9c0
Summary:
During S231236 the LFS servers were using a lot of CPU and were
compressing blobs sent to clients. Thomas hotfixed the servers to disable
compression in order to save some CPU. Instead of having to rebuild and
redeploy the LFS server, update it to be able to disable compression using
configerator.
The `disable_compression` option will disable compression of downloads
globally. The `disable_compression_identities` allows us to disable compression
for some group of identities.
I also refactored some of the shared code from `download` and `download_sha256`
into a new function, `download_inner`.
Reviewed By: krallin
Differential Revision: D28382405
fbshipit-source-id: 792f10a9e3bb32b56ef87aa8e0b2c4b098567579
Summary: Write mostly stores are often in the process of being populated. Add an option to control whether scrub errors are raised for missing values in write mostly stores.
Differential Revision: D28393689
fbshipit-source-id: dfc371dcc3b591beadead82608a747958b53f580
Summary:
Couple of small changes I originally did during next diff split out on their own.
Save a loop over the scrub stores if there are relevant entries on the queue and remove unnecessary repetition of type.
Reviewed By: farnz
Differential Revision: D28411617
fbshipit-source-id: 2015b5b1d68f870b09fbd8929a59c21fe4f57c87
Summary:
When you spawn a task, Tokio puts it on a "LIFO slot" associated with the
current thread. While the task is in the LIFO slot, it is not eligible to be
run by other threads.
If the thread that just spawned `fwd` above goes and does some expensive
synchronous work in `request_handler` (we'd like to avoid that but sometimes
that happens), then that will delay `fwd`.
This means that notably keepalives will not be sent (you can repro by putting a
`std:🧵:sleep` right after we spawn `fwd`). To mitigate this, we spawn
another dummy taks here. This task will take `fwd`'s place in the LIFO slot,
thus pushing `fwd` onto a task queue where other runtime threads can claim it.
This way, even if this thread goes do some expensive CPU-bound work, we won't
delay keepalives.
This doesn't solve the fact that we probably shouldn't be doing such CPU-bound
work to begin with, but at least it might mitigate the issue such that we don't
have I/O delayed even though the runtime isn't visibly overloaded.
Reviewed By: johansglock
Differential Revision: D28412114
fbshipit-source-id: b56c0156ac6cf991cc899a82e3d2a96c63216fda
Summary: Add a write mostly store to the scrub test cases. This in preparation for next diff where the write mostly case has a new scrub option added.
Reviewed By: krallin
Differential Revision: D28393690
fbshipit-source-id: f7878f5e25814a7d327b1a80d4f96c0867944a14
Summary: Log the blobstore stack being used for the scrub
Reviewed By: farnz
Differential Revision: D28408340
fbshipit-source-id: 2299f7f7397f48d70b9a8295f0aa28c89bbf5809
Summary: These will be used in the next diffs, let's add initialization here.
Reviewed By: ikostia
Differential Revision: D28409734
fbshipit-source-id: f656db8259f28559df52562f0590382d89f1f0c0
Summary:
This is an attempt to split D28357210 to make it a bit smaller, so this
new function will be used later.
Reviewed By: krallin
Differential Revision: D28386004
fbshipit-source-id: 42c9ead0668fb78747dcb0fc7c89b6f181f7d9e6
Summary:
We have clients mysteriously timing out despite the fact that we should
normally be sending keepalives. To understand this, add some logging so that if
a client disconnects, we log a lot more detailed information to Scuba.
Reviewed By: mitrandir77
Differential Revision: D28383122
fbshipit-source-id: 0e49262bcc08c75f8e06eae17c882742b58ea51d
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:
This struct is intended to be a single entry-point to the megarepo logic. It is also intended to be owned directly by scs_server without the `Mononoke` struct (from `mononoke_api`) intermediary. In effect, this means that mononoke server won't be affected by `MegarepoApi` at all.
Apart from adding this struct, this diff also adds instantiation of prod `AsyncMethodRequestQueue` and wires it up to the scs_server to enqueue and poll requests.
Reviewed By: StanislavGlebik
Differential Revision: D28356563
fbshipit-source-id: b67ee48387d794fd333d106d3ffd40124086c97e
Summary:
With `MegarepoApi` struct in play, there is a genuine need to have two repo
factories in a single process: this allows the structure to be self-sufficient
and instantiated without any references to `Mononoke` from `monooke_api`.
While this need could be solved by just wrapping a `RepoFactory` in an `Arc`,
it seems like most of it is already clonable, so let's just make it fully
clonable by fixing a few remaining places. (I prefer this over `Arc`, because
there's less refactoring of unrelated code). Given that there will likely be a
single digit of repo factories instantiated in a single process, the difference
between a single arc's clone (`Arc<RepoFactory>`) and ~10 arc clones (what I
did) is negligible.
Differential Revision: D28379860
fbshipit-source-id: fbddbdc913fedcd5846366344bc2f2c1ec4bd91e
Summary: This way implementing MegarepoApi is more convenient.
Reviewed By: krallin
Differential Revision: D28355487
fbshipit-source-id: e7643e854ee46fe6cb9c4a882f6c677bf4e77262
Summary:
Partial getbundle optimization didn't work correctly if one merge parent was an ancestor
of another - it might return a parent commit before a child commit. Say we had a graph like this
```
C
| \
| B
| /
A
```
Previously partial getbundle optimization could have visited A before it
visited B, and returned commits in the wrong order, which in turn would lead to
"hg pull" failures. The reason for that is that we don't order commits by generation number.
This diff fixes it by using UniqueHeap to sort commits by generation number
before returning them. Also note that as the name suggests UniqueHeap stores
only unique values in the heap, so we don't need to keep separate `visited`
hashset
Reviewed By: krallin
Differential Revision: D28378145
fbshipit-source-id: 9467fb7cfa8386e9e2725b80f386240b522ff3ee
Summary:
Just as with rewrite_commit function that I moved a to commit_transformation
not so long ago (D28259214 (df340221a0)), let's also move copy_file_contents. The motivation
is because we are going to use it in the next diff for sync_changeset method.
Reviewed By: ikostia
Differential Revision: D28352402
fbshipit-source-id: 12288a51540c9793d988e4063735bcbc1c3b1a7f
Summary:
Those blobs are designed to fit in cachelib, so we shouldn't attempt to
zstd-encoded them (not to mention that they don't usually compress very well
since many of those blobs come from binary files, though that's not true of
all).
However, we weren't actually doing this right now. There were a few reasons:
- Our threshold didn't allow enough headroom. I don't know when exactly this
got introduced (of indeed if that has worked since we introduced cachelib
compression).
- We serialize a bunch of extra metadata that we really don't need as it's a
bit meaningless once it's gone through the cache (we notably don't serialize
this in Memcache). This diff updates us to just store bytes here.
Differential Revision: D28350387
fbshipit-source-id: 4d684ab58cea137044e20951ec4cbb21240b8dfc
Summary: See the next diff for motivation: this makes it easier to implement.
Differential Revision: D28350388
fbshipit-source-id: 026605cf8296a945d6cc81b7f36d9198325bf13c
Summary:
Test is flaky: https://www.internalfb.com/intern/test/281474993296146?ref_report_id=0
I suppose this happens due to mechanics how we measure qps - with low volume, some averaging or bucketing might not work as precisely as with a lot of qps we have in normal, prod scenarios.
Lowering the threshold by 1 should fix this.
Reviewed By: ahornby
Differential Revision: D28350150
fbshipit-source-id: 694bfb8cce1935704e35b27f7d4455439d4bfffe
Summary:
I should've made them structs from the beginning, but of course I thought that
I know better and these tokens can definitely not be richer than just strings.
Well, it turns out we need them to be richer. Specific reason is that in theory
a single Mononoke (or scs_server) instance can run with multiple storage
configs. For us this means that one target's requests may be stored in one
db, while another target's requests - in another one. For blobstores this is
even used in practice, while for xdb it's just a theoretical thing, but we need
to support it nevertheless.
To do so, let's add the ability to query the target (and, correspondingly, the
Mononoke repo) from any king of params our async methods receive: ThriftParams
or Token implementors.
In addition, this diff really implements `AddScubaParams` and `AddScubaResponse` for more things than before, so there's that.
Finally, apart from making tokens structured, this diff also changes an interface in two more ways:
- it adds optional `message` fields to several params structs
- it adds `changesets_to_merge` to `MegarepoChangeTargetConfigParams`
Reviewed By: StanislavGlebik
Differential Revision: D28333999
fbshipit-source-id: 99bd19b040b59ee788ef661dda3171cc56254d33
Summary: This is going to be used in the next diffs.
Reviewed By: StanislavGlebik
Differential Revision: D28333977
fbshipit-source-id: ad52d307e13ae9bd662209ef7ec6afdcf0ee24c7
Summary:
This has been here for a little while, but it's worth changing. Currently, we
entirely discard logs coming via a CoreContext in EdenAPI.
We don't typically log many of those anywhere in Mononoke, but when we do they
tend to be for error conditions, warnings, or aggregated reporting, and can be
quite meaningful as a result.
So, let's update to not discard them. To make it easy to differentiate those
logs from EdenAPI request-level logs (even though they'll both have `service` =
`edenapi`), let's give the latter a Log Tag (which is consistent with what
we do in repo client).
Differential Revision: D28350733
fbshipit-source-id: 3b12a4b56f28435460186e1f7578163ca7bdaebc
Summary:
Previously it was possible to write configs only, now it's possible to read
them as well.
Reviewed By: ikostia
Differential Revision: D28326571
fbshipit-source-id: d946201a384cc3998d1c197b7eabb77b9f35129d
Summary:
Adding mappng to keep track of two things:
1) keep track of the latest source commit that was synced into a given target - this will be used during sync_changeset() method to validate if a parent changeset of a given changeset was already synced
2) which source commit maps to what target commit
Reviewed By: ikostia
Differential Revision: D28319908
fbshipit-source-id: f776d294d779695e99d644bf5f0a5a331272cc14
Summary:
This is going to be use to rewrite (or transform) commits from source to
target. This diff does a few tihngs:
1) adds a MultiMover type and a function that produces a mover given a config. This is similar to Mover type we used for fbsource<-> ovrsource megarepo sync, though this time it can produce a few target paths for a given source path.
2) Moves `rewrite_commit` function from cross_repo_sync to megarepo_api, and make it work with MultiMover.
Reviewed By: ikostia
Differential Revision: D28259214
fbshipit-source-id: 16ba106dc0c65cb606df10c1a210578621c62367
Summary:
This crate is a foundation for the async requests support in megarepo service.
The idea is to be able to store serialized parameters in the blobstore upon
request arrival, and to be able to query request results from the blobstore
while polling.
This diff manipulates the following classes of types:
- param types for async methods: self-explanatory
- response types: these contain only a resulting value of a completed successful execution
- stored result types: these contain a result value of a completed execution. It may either be successful or failed. These types exist for the purpose of preserving execution result in the blobstore.
- poll-response types: these contain and option of a response. If the optional value is empty, this means that the request is not yet ready
- polling tokens: these are used by the client to ask about the processing status for a submitted request
Apart from that, some of these types have both Rust and Thrift counterparts, mainly for the purposes of us being able to implement traits for Rust types.
Relationships between these types are encoded in various traits and their associated types.
The lifecycle of an async request is as follows therefore:
1. the request is submitted by the client, and enqueued
1. params are serialized and saved into a blobstore
1. an entry is created in the SQL table
1. the key from that table is used to create a polling token
1. some external system processes a request [completely absent form this diff]
1. it notices a new entry in the queue
1. it reads request's params from the blobstore
1. it processes the request
1. it preserves either a success of a failure of the request into the blobstore
1. it updates the SQL table to mention that the request is now ready to be polled
1. the client polls the request
1. queue struct receives a polling token
1. out of that token it constructs DB keys
1. it looks up the request row and checks if it is in the ready state
1. if that is the case, it reads the result_blobstore_key value and fetches serialized result object
1. now it has to turn this serialized result into a poll response:
1. if the result is absent, poll response is a success with an empty payload
1. if the result is present and successful, poll response is a success with the result's successful variant as a payload
1. if the result is present and is a failure, the polling call throws a thrift exception with that failure
Note: Why is there yet another .thrift file introduced in this diff? I felt like these types aren't a part of the scs interface, so they don't belong in `source_control.thrift`. On the other hand, they wrap things defined in `source_control.thrift,` so I needed to include it.
Reviewed By: StanislavGlebik
Differential Revision: D27964822
fbshipit-source-id: fc1a33a799d01c908bbe18a5394eba034b780174
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:
When sampling multiplex stores its interesting to know which component of the store one is sampling.
This adds a new SamplingBlobstorePutOps struct with implements the BlobstorePutOps that multiplex blobstore requires. Its connected up to blobstore factory in the next diff.
Reviewed By: farnz
Differential Revision: D28264444
fbshipit-source-id: 560de455854b6a6794b969d02046d67d372efd37
Summary: What we're trying to do here is all explained in the inline comments.
Reviewed By: farnz
Differential Revision: D28287486
fbshipit-source-id: 605c5272118b9d0b76f6284f4e81febe4b6f652e
Summary:
These are undermaintained, and need an update for oncall support. Start by moving to CXX, which makes maintenance easier.
In the process, I've fixed a couple of oddities in the API that were either due to the age of the code, or due to misunderstandings propagating through bindgen that CXX blocks, and fixed up the users of those APIs.
Reviewed By: dtolnay
Differential Revision: D28264737
fbshipit-source-id: d18c3fc5bfce280bd69ea2a5205242607ef23f28
Summary:
Because cachelib is not initialised at this point, it returns `None` unconditionally.
I'm refactoring the cachelib bindings so that this returns an error - take it out completely for now, leaving room to add it back in if caching is useful here
Reviewed By: sfilipco
Differential Revision: D28286986
fbshipit-source-id: cd9f43425a9ae8f0eef6fd32b8cd0615db9af5f6
Summary: This wants to use Scuba so it needs this.
Reviewed By: StanislavGlebik
Differential Revision: D28282511
fbshipit-source-id: 6d3a2b6316084f7e16f5a2f92cfae1d101a9c2d3
Summary:
In blobstore factory we can end up with duplicate layers of wrapper blobstores like ReadOnlyBlobstore.
For the multiplex, its inner stores get throttling, readonly etc wrappers, and it itself only writes to its queue if an inner store succeeds, which it can't when inner store has ReadOnlyBlobstore wrapper.
Differential Revision: D28250832
fbshipit-source-id: 5a3f85584b9cce17ca7ce4b83cdb2117644850db
Summary: Add support for logging the inner parts of a multiplex blobstore stack. This helps understand what wrappers have been applied.
Differential Revision: D28230927
fbshipit-source-id: 873ee30ec00fdc2dfc79b47e5831231c51e2ce0d
Summary:
Fixing the bulkops fetch size to MAX_FETCH_STEP means we can use the chunk size option to control how many changesets are walked together without affecting query performance.
This will allow more accurate first referencing commit time lookup for files and manifests, as all commits in a chunk could possibly discover them, with smaller chunks the discovered mtime becomes less approximate, at the possible cost of some walk rate performance if one was to run with very small chunk sizes.
Differential Revision: D28120030
fbshipit-source-id: 0010d0672288c6cc4e19f5e51fd8b543a087a74a
Summary: Knowing the numeric changeset id is useful in next diff when chunking in walker is loading from bulkops in large chunks, but then walking commits in smaller chunks.
Differential Revision: D28127581
fbshipit-source-id: c5b3e6c2a94e33833d701540428e1ff4f8898225
Summary: Found this useful while debugging the pack sampling
Differential Revision: D28118243
fbshipit-source-id: d94b0b87125a9863f56f72029c484909a3696329
Summary: This update makes it so that we don't log versions to scuba from tests.
Reviewed By: krallin
Differential Revision: D27449808
fbshipit-source-id: 9c79e83fbfdf3d9a02c2cfc8b6a8255edb4241fe
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:
This is where async requests are logged to be processed, and from where they
are polled later.
It will acquire more functionality when the actual request processing business
logic is implemented.
Reviewed By: StanislavGlebik
Differential Revision: D28092910
fbshipit-source-id: 00e45229aa2db73fa0ae5a1cf99b8f2d3a162006
Summary:
Log the sizing metadata about keys that scrub has seen to the pack info logs.
This uses the sampling blobstore to see all blobstore gets and captures info from them.
Also updates relatedness_key fieldname to mtime as that way its less easily confused with similarity_key
Differential Revision: D28115620
fbshipit-source-id: 666a444c2d91b0ca5bb225cea971f9b183e6a48d
Summary:
Pass BlobstoreGetData to the sampler so that it has a chance to sample the BlobstoreMetadata as well as the BlobstoreBytes.
This is used in the next diff for sampling the sizing information.
Reviewed By: markbt
Differential Revision: D28115619
fbshipit-source-id: 7a79d482c9ba1ed8b08afab5f1c1b8fe7c4f257a
Summary: When reading from packblob we'd like to see metadata about sizes so that can log it for the packer later in this stack.
Reviewed By: markbt
Differential Revision: D28101971
fbshipit-source-id: 96dd0d5497c2bb5c27231709dbd19d00168e1a77
