Summary:
Let's add a command that validates that the created catchup commit is correct.
For now it validates that unodes are the same between catchup commit and commit
that we are merging in.
Later we can add more invariants that we want to check.
Reviewed By: krallin
Differential Revision: D23782369
fbshipit-source-id: 61d19aa73777d5fbb3e1b127bdcf39f5e6309b52
Summary: Add error context to file content scrub so that we can tell if an Error has propagated via the scrub stream loading.
Reviewed By: StanislavGlebik
Differential Revision: D23838144
fbshipit-source-id: 40a8a090510959cab1020182c19076b8a3317b1b
Summary:
Implemented S3 blobstore
Isilon implements S3 as 1:1 mapping into filesystem, and it limits the maximum number of blobs in the single directory. To overcome it lets shard the keys using base64 encoding and making 2 level dir structure with 2 chars dir names.
Reviewed By: krallin
Differential Revision: D23562541
fbshipit-source-id: c87aca2410381a07babb191cbd8cf28233556e03
Summary:
When running the repo import tool, it's possible that we need to do additional setup steps before being able to run the tool, which otherwise would only come up when we run it.
Firstly, if the repo we import into doesn't have a callsign (e.g. FBS, WWW...), but we want to check Phabricator, our tool would hang when checking Phabricator, because we need the callsign for checking. Therefore, we need to inform the user to set the callsign for the repo.
Secondly, in case the repo push-redirects to a larger repo, we generate a bookmark for the commits imported into the large. However, we need to inform the Phabricator team to include the large repo's bookmark before we can import the commits, because this bookmark publishes the imported commits on Phabricator.
This diff adds a subcommand to check these additional steps, so we wouldn't find these out during the actual import run.
Reviewed By: StanislavGlebik
Differential Revision: D23783462
fbshipit-source-id: 3cdf4035548213d8cee9717fb985c22741a6749b
Summary:
In the later diffs we are going to change how CommitSyncer is initialized. In
order to make it simpler let's refactor cross_repo_sync_test to move
CommitSyncer creation in a single function.
There are a few tests that have very peculiar initialization - for example they
have movers that fail. For those tests I combined the new function for creation
of CommitSyncer with manual initialization of CommitSyncRepos struct.
Reviewed By: krallin
Differential Revision: D23811507
fbshipit-source-id: 682ab30aa09c9189fcd02850a19f1ddf021c0329
Summary:
Pull Request resolved: https://github.com/facebookexperimental/eden/pull/60
For the tests that output different data to stdout in OSS vs FB create helpers that remove the differences.
Reviewed By: farnz
Differential Revision: D23814134
fbshipit-source-id: c6656528021c9a90b98e3c89a9bbe8c5178c6919
Summary:
Add `scsc land-stack` to facilitate testing of stack landing via the source control service.
Use this to test that landing of stacks works.
Reviewed By: aslpavel
Differential Revision: D23813366
fbshipit-source-id: 1f7b682fa5e33a232cb1da5c702a703223658942
Summary:
Update the conversion of `BookmarkMovementError` to `MononokeError` to reflect
that most movement errors are caused by invalid requests.
Reviewed By: aslpavel
Differential Revision: D23814794
fbshipit-source-id: 48503353aaae7b3cd03e5221a8ad014eef2e9414
Summary:
Implement the `repo_land_stack` method by working out which commits are in the
stack to be landed, and then pushrebasing them onto the target bookmark.
Reviewed By: aslpavel
Differential Revision: D23813370
fbshipit-source-id: babe34f0e9f1db055adb2e5d1debefd8ebcf6f86
Summary:
Sometimes the `AsyncIntoResponse` trait needs additional data (e.g. the set of commit
identity schemes the client is interested in) to convert the item into the response
type.
Currently we use a tuple of `(item, &additional_data)` to work around this, however
this will become less readable as we add new items with more additional data.
Split this use-case out into a new trait: `AsyncIntoResponseWith`. This defines
an associated type which is the type of the additional data needed, and provides a
new method `into_response_with`, where a reference to the additional data can be
provided.
Note that conversions for tuple types that are logical `(name, value)` or `(id,
value)` pairs are still ok. It is specifically the case where we have `(item,
&additional_data)` that we are converting here (i.e. the additional data merely
informs the conversion, it is not part of the resulting response value).
Reviewed By: aslpavel
Differential Revision: D23813371
fbshipit-source-id: c0dcfe826288ad53ad572ae4dd956540605998f5
Summary: Make it clear which error is which, and what the number of expected and actual items are.
Reviewed By: StanislavGlebik
Differential Revision: D23813369
fbshipit-source-id: 5b94c5a67438c475235876669ec2be3fd1866700
Summary: Intern ids to reduce space used in the walk state. This is significant on large repos.
Reviewed By: farnz
Differential Revision: D23691524
fbshipit-source-id: b42f926d88083d06ffc44508db44747f9a14e0a5
Summary:
Passing option is not necessary since live_commit_sync_config is always
available.
Reviewed By: ahornby
Differential Revision: D23811021
fbshipit-source-id: ee11f88d57814d9abac8650e52febd9e431770da
Summary:
I've re-backfilled some of blame values for configerator. But old values might
still be in memcache. To make sure that's not the case let's bump the memcache
key.
Reviewed By: krallin
Differential Revision: D23810971
fbshipit-source-id: c333a51ffb2babf7da808b276f9cfa31baaa105c
Summary: Small change to make it more readable and reduce likelihood of allocation (although the collect might be optimized away anyway)
Reviewed By: farnz
Differential Revision: D23760762
fbshipit-source-id: 5c47352386de128b65052d63b3f3ff1081a462e3
Summary: Make `StreamBody` accept a `Stream` of `Bytes` instead of a `TryStream` of `Bytes`. This means that applications returning streaming responses will be forced to deal with errors prior to returning the response.
Reviewed By: krallin
Differential Revision: D23780216
fbshipit-source-id: dbad61947ef23bbfc4edf3d286ad0218c1859d87
Summary:
Using the `EndOnErr` combinator introduced in the previous diff, log any errors that occur during a streaming response to stderr.
Note that **the intent of this diff is to implement the most basic possible example of doing something with these errors**, with the goal of allowing us to modify `StreamBody` to only accept infallible `Stream`s.
I'd imagine that in all likelihood we'd want to do something smarter with the errors than just print them, but I figure that can be added in later diffs since it seems like doing something else (like logging the error to Scuba or adding to the RequestContext) might require additional changes that are beyond the scope of this diff.
At the very least, this seems like an improvement from before, where these errors would just get passed straight through to Hyper.
Reviewed By: krallin
Differential Revision: D23780217
fbshipit-source-id: 2f885f9fdc6af3dd28d95be1daa1d82c732453fa
Summary: Add a new `EndOnErr` combinator for `TryStream`s (exposed via the `GothamTryStreamExt::end_on_err` method) which fuses the underlying `TryStream` upon hitting an error, and passes the error to the user-provided callback. This is useful for contexts like the LFS server, where mid-stream errors are considered unrecoverable and must result in termination of the response.
Reviewed By: krallin
Differential Revision: D23778490
fbshipit-source-id: 05caa52ca62d085cc63cc8feb4619188fe0fac61
Summary: Use the new `ForwardErr` stream combinator to log errors that occur during a streaming response. Right now, they are just printed to stderr, but in the future we should also do other things such as logging them to Scuba. This diff supersedes the approach from D22720957.
Reviewed By: krallin
Differential Revision: D23780215
fbshipit-source-id: 8d2267f1166e665a62a167a6d95bb0b1797b5767
Summary: Implement `ContentMeta` for streams wrapped with the `ForwardErr` combinator, so that they may be used as input to `StreamBody`. (Presently, this won't actually work since `StreamBody` expects a `TryStream`, but this will change later in the stack.)
Reviewed By: krallin
Differential Revision: D23777842
fbshipit-source-id: 234bcdf104afbf2c9832fbe54d85744bfb470363
Summary:
This diff adds a new `ForwardErr` combinator that allows redirecting the errors from a `TryStream` into a channel, allowing them to be processed asynchronously without disrupting the stream itself. This effectively splits the `TryStream` into two `Stream`s containing the successful items and errors respectively.
To make it easy to use the combinator, I've added a new `GothamTryStreamExt` extension trait (in the vein of the old `futures_ext` crate) that allows the user to simply call `forward_err` on any `TryStream`. The trait name is a bit of a misnomer (suggestions welcome!) in that there isn't anything Gotham-specific about it, but the name `TryStreamExt` is taken and I didn't want to set up a successor to `futures_ext` just for the sake of one combinator. (Though we will likely expand the trait in the future.)
Reviewed By: krallin
Differential Revision: D23777501
fbshipit-source-id: 8628cdc2e104cd9b972afda745858f9cb9e85245
Summary:
Compressed responses from LFS are slower than they should right now. Normally,
we'd expect something along the lines of normal response time + compression
time, but right now it's a lot more than this.
The reason for this is that our compressed streams are eager, i.e. they will
consume and compress as much of the underlying stream as possible before
sending off the data. This is problematic for LFS, because we try very hard to
serve everything out of RAM directly (and very often succeed), so that means
we compress the whole stream before sending it off.
This means we might spend e.g. 500ms compressing (this is how long it takes
zstd to compress the object I was testing on, which is a ~80MiB binary that
compresses down to 33% of that), and _then_ we'll spend some time transferring
the compressed data, when we could have started transferring immediately while
we were compressing.
To achieve this, let's simply tell our compressed stream to stop waiting for
more data once in a while (every 4 MiB, which seems very frequent but actually
really isn't).
Reviewed By: StanislavGlebik
Differential Revision: D23782756
fbshipit-source-id: a0d523d84f92e215eb366f551063383fc835fdd6
Summary:
I saw this throw the LFS server into an infinite loop when I tested it. We're
not using this right now, so I'm not investing time into root-causing the
issue, and instead let's just take this out.
Reviewed By: StanislavGlebik
Differential Revision: D23782757
fbshipit-source-id: f320fc72c3ff279042c2fe9fcb9c4904e9e1bfdf
Summary:
Pull Request resolved: https://github.com/facebookexperimental/eden/pull/51
This diff extends capabilities of CargoBuilder in getdeps so that individual manifests can be build even without workspaces. Thanks to that a build for edenapi/tools can be made and its artifacts can be used in mononoke integration tests.
Reviewed By: StanislavGlebik
Differential Revision: D23574887
fbshipit-source-id: 8a974a6b5235d36a44fe082aad55cd380d84dd09
Summary:
This makes deletion commits a bit less confusing, but it also have another
benefit.
Without the sort some directories might have been changed multiple times in
deletion commits e.g. if a directory had 5 files, and these files were deleted
in 5 different deletion commits then the directory would be changed 5 times.
This was not good, because it made some data derivation slower (in particular,
fastlog), because it had to regenerate the same data over and over again.
Reviewed By: ikostia
Differential Revision: D23780066
fbshipit-source-id: d5c52b13f58dcaf2012d9c12bf77398561cf10ef
Summary:
Spotted a TODO in fsnode get_fsnode_id. There was only one user of the function, which didn't really need to call it as it had the blob already.
As well as being a bit tidier this also saves a clone of the fsnode.
Reviewed By: StanislavGlebik
Differential Revision: D23758689
fbshipit-source-id: e0a8c124c929fda3af4c96a76d441a79e5bfbd5b
Summary:
Save memory in walker state tracking by not memoing hash values. For large repos this is significant.
I was expecting a small slowdown from this, but so far looks pretty much the same walk rate. Speculation: this may be due to the num cpus lock sharding fix in dashmap 3.11.10 which means there are many more shards than when the memo was tested with 3.11.9, so saving time inside locks is less significant.
Reviewed By: StanislavGlebik
Differential Revision: D23680550
fbshipit-source-id: 351b5ec39885fc30996207c7dccc22c749e30321
Summary:
The `gotham_ext::response` module was getting a bit large, so this diff moves `ContentMeta`, `ContentStream`, and `CompressedContentStream` into a new submodule, alongside the contents of the old `content_encoding` module. This way, the `response` module remains entirely centered around the `TryIntoResponse` trait (and the various body structs that implement that trait).
Later diffs in this stack will be adding an additional layer between the content streams and the body structs, at which point it probably doesn't make sense to have these right next to each other. Splitting them out now will allow for better code organization going forward.
Reviewed By: krallin
Differential Revision: D23777492
fbshipit-source-id: 86e598dcb37578d3b22217a2a65f1bde84d72215
Summary: Previously, we used the call sign of the repo we import when checking any of the if the commits are parsed by Phabricator. However, we also use this callsign for other repos when checking Phabricator, which is an incorrect implementation. E.g. if fbsource back-syncs to ovsource, we would have used FBS callsign when checking Phabricator for both fbsource and ovrsource, but we should use OVRSOURCE callsign for repo ovrsource. This diff corrects this implementation by saving the callsigns of the small repos in their SmallRepoBackSyncVars.
Reviewed By: StanislavGlebik
Differential Revision: D23758355
fbshipit-source-id: b322acb2ec589eabed5362bfd6b963e2dd1d6ea9
Summary:
Pull Request resolved: https://github.com/facebookexperimental/eden/pull/58
This makes the test-bookmarks-filler.t pass. Additionally remove few tests from exclusion lists as they started to pass.
Reviewed By: ikostia
Differential Revision: D23757401
fbshipit-source-id: eddcda5fd1806d77d0046b6ced3695df6b3d775d
Summary:
We are running out of space on integration tests runs on Linux. In order to avoid that this change is adding some cleanups.
1. Adding `docker rmi $(docker image ls -aq)` frees up 4 GB.
2. Cleaning up `eden_scm` build directory frees up 3 GB.
3. Cleaning up `mononoke` build directory frees up 1 GB.
This diff also includes a fix for run_tests_getdeps.py where we run all the "PASSING" tests when --rerun flag is passed instead of only the failed ones.
Pull Request resolved: https://github.com/facebookexperimental/eden/pull/57
Reviewed By: krallin
Differential Revision: D23742159
Pulled By: lukaspiatkowski
fbshipit-source-id: 3b5e89ad29c753d585c1a6f01a9a1d6c1e616fbf
Summary: fixes build and test errors on OSS introduced by D23596262 (deb57a25ed)
Reviewed By: ikostia
Differential Revision: D23757086
fbshipit-source-id: 7973ce36b3589cbe21590bd7e19a9828be72128f
Summary: Since repo_import tool is automated, we need a way to recover the process when the tool break without restarting the whole process. To do this, I defined a new struct (RecoveryFields) that allows us to keep track of the state. The most important fields are the import_stage (ImportStage) we need for keeping track of the process and to indicate the first stage of recovery, and the cs_ids we use throughout the process. For each stage in importing, we save the state after we have finished that part. This way we can also recover from interrupts. To do process recovery, we only need to use `recover-process <file_path>` subcommand, where file_path stores the saved state of importing. For normal run we will use `import [<args>]` subcommand.
Reviewed By: krallin
Differential Revision: D23678367
fbshipit-source-id: c0e0b270ea2ccc499368e54f37550cfa58c03970
Summary:
This change allows us to use warm bookmark cache for all clients except
for external sync job (i.e. the job we use to keep configerator-hg in sync with
configerator-git).
This is useful we'd like to use warm bookmark cache for configerator but it
doesn't work with external sync job. We'd like to use it because warm bookmark
cache doesn't advance a bookmark until this revision showed up in
configerator-hg - this proved to be useful when rolling out configerator for
devservers since there were tools that talked to hg, and they were failing if
hg was behind.
Currently hg external sync job doesn't work with warm bookmark cache because it
tries to incorrect move a master. What I mean by that is that hg external sync
job sends unbundle request which contains a pushkey part which says "move
master from commit A to commit B". If commit A is outdated because of warm
bookmark cache then this update will just fail, because master bookmark
actually points to commit C.
Let's just not ever use warm bookmark cache for external sync job
Reviewed By: aslpavel
Differential Revision: D23754603
fbshipit-source-id: c8eec54bca2224688d4a829ded372c6fc4d7930f
Summary:
Pass the elements to the hasher to avoid needing to alloc a vec from them.
This saves building the vec inside MPathElement, and when used on top of smallvec based MPathElement also saves allocation of a Vec from the SmallVec for each element.
Reviewed By: aslpavel
Differential Revision: D23703342
fbshipit-source-id: dd81c6d69b90f128d697ba847dde34058ad1ea6e
Summary:
Use smallvec for the internal storage of MPathElement.
The previous Bytes had a stack size of 32 bytes plus the text it pointed to.
Using SmallVec we can store up to 24 bytes without allocation keeping the same space as the previous Bytes object.
Given most path elements are directory names and directory names are usually short it is expected that this will save both space and allocations.
Reviewed By: farnz
Differential Revision: D23703344
fbshipit-source-id: 39ffc3bd3bb765bd1dbb757b4b1a7782382db909
Summary:
When sending trees and files we try to avoid sending trees that are
available from the main server. To do so, we currently check to see if the
tree/file is from the local store (i.e. .hg/store instead of $HGCACHE).
In a future diff we'll be moving trees to use the Rust store, which doesn't
expose the difference between shared and local stores. So we need to stop
depending on logic to test the local store.
Instead we can test if the commit is public or not, and only send the tree/file
is the commit is not public. This is technically a revert of the 2018 D7992502 (5e95b0e32e)
diff, which stopped depending on phases because we'd receive public commits from
svn there were not public on the server yet. Since svn is gone, I think it's
safe go back to that way.
This code was usually to help when the client was further ahead than another
client and in some commit cloud edge cases, but 1) we don't do much/any p2p
exchange anymore, and 2) we did some work this year to ensure clients have more
up-to-date remote bookmarks during exchange (as a way of making phases and
discovery more reliable), so hopefully we can rely on phases more now.
Reviewed By: quark-zju
Differential Revision: D23639017
fbshipit-source-id: 34c13aa2b5ef728ea53ffe692081ef443e7e57b8
Summary:
There was a bug. If an entry was skipped then we haven't updated the counter.
That means we might skip the same entry over and over again.
Let's fix it
Reviewed By: ikostia
Differential Revision: D23728790
fbshipit-source-id: f323d14c4deba5736ceb8ada7cb7ee48a69c1272
Summary:
Previously we were able to add a backpressure to the x-repo-sync job that waits
until backsync queue gets empty. However it's also useful to wait until hg sync
queue drains for the large repo. This diff makes it possible to do so.
Reviewed By: aslpavel
Differential Revision: D23728201
fbshipit-source-id: 6b198c8d9c35179169a46f2b804f83838edeff1e
Summary: For fsnodes output the filecontent child nodes first as they can be drained without expanding to more nodes.
Reviewed By: farnz
Differential Revision: D23702268
fbshipit-source-id: 26aeca20d47030dbb9145d406db885fe0fce932c
Summary: Use sorted_vector_map when parsing fsnodes, as inputs are stored sorted, which can result in high cost of BTree insertion when traversing large repos.
Reviewed By: aslpavel
Differential Revision: D23691500
fbshipit-source-id: 1f7a5faf2ef3cb4a72a635d0d8e89037bf4d96b3
Summary:
We are currently logging only the outermost underlying error or context, not any of the lower level causes. This makes mononoke_blobstore_trace less useful!
This changes to use anyhow's alternate format that includes causes
Reviewed By: krallin
Differential Revision: D23708577
fbshipit-source-id: fa2e71734841e2b75d824c456dccf61c1fb13fd2
Summary:
Instead of using force_set and force_delete let's use create() update() and
delete() calls.
Reviewed By: ikostia
Differential Revision: D23704245
fbshipit-source-id: 40bcfd906c4f61a860e5ec8312cddc0d80ea94ae
Summary:
This is the function that was used in repo_import tool to wait until hg sync
has processed all of the entries in the queue. Let's move it to the hg sync
helper lib so that it can be used in other places. E.g. I'd like to use it in
the next diffs in mononoke_x_repo_sync_job.
Reviewed By: krallin
Differential Revision: D23708280
fbshipit-source-id: ea846081d89b55b0d2f5407c971e13869cedfd8b
Summary:
In preparation of moving away from SSH as an intermediate entry point for
Mononoke, let Mononoke work with newly introduced Metadata. This removes any
assumptions we now make about how certain data is presented to us, making the
current "ssh preamble" no longer central.
Metadata is primarily based around identities and provides some
backwards-compatible entry points to make sure we can satisfy downstream
consumers of commits like hooks and logs.
Simarly we now do our own reverse DNS resolving instead of relying on what's
been provided by the client. This is done in an async matter and we don't rely
on the result, so Mononoke can keep functioning in case DNS is offline.
Reviewed By: farnz
Differential Revision: D23596262
fbshipit-source-id: 3a4e97a429b13bae76ae1cdf428de0246e684a27
Summary: Support for multiple heads in `BonsaiDerived::find_all_underived_ancestors`. This change will be needed to remove manual step of fetching of all changesets in `backfill_derived_data` utilty.
Reviewed By: StanislavGlebik
Differential Revision: D23705295
fbshipit-source-id: 32aa97a77f0a4461cbe4bf1864477e3e121e1879
Summary:
As it says in the title, this adds support for receiving compressed responses
in the revisionstore LFS client. This is controlled by a flag, which I'll
roll out through dynamicconfig.
The hope is that this should greatly improve our throughput to corp, where
our bandwidth is fairly scarce.
Reviewed By: StanislavGlebik
Differential Revision: D23652306
fbshipit-source-id: 53bf86d194657564bc3bd532e1a62208d39666df
