Summary:
Current `test_mock` creates logger with default_drain and output isn't printed in unit-tests. There is a method `logger_that_can_work_in_tests()` which overcome it, let's use it for constructing test CoreContext.
While testing I've also spotted some leftover from migration to new BonsaiDerivable. Lets fix it too
Reviewed By: krallin
Differential Revision: D31430162
fbshipit-source-id: a086be521f0ceaeb3267e87f24980fb11587a6e7
Summary:
Using modulo on arbitrary integers to get random numbers [isn't correct](https://www.internalfb.com/diff/D31305392 (da13975a4f)?dst_version_fbid=311037904117090&transaction_fbid=550270779610744), as the distribution between numbers isn't fair (unless the size is a power of two).
This was raised on D31305392 (da13975a4f), but we decided to land that quickly to unblock builds before doing these changes.
I'm applying the changes suggested on D31305392 (da13975a4f). This is what this diff does:
- For all cases where we generate small numbers (up to 5), replace with call to `Gen::choose`, so `u32::arbitrary(g) % 3` becomes `g.choose(&[0, 1, 2]).unwrap()`.
- For generating numbers in range 0..=1, I instead replaced with generating a boolean, which gets rid of the `unreachable!` calls.
- I removed the code to generate numbers in range 0..=0.
- For generating larger numbers, I used `u64::arbitrary` instead, which should make things "less wrong".
Some things I assumed, but am happy to change before landing, just let me know:
- Theoretically we don't *need* to change the code for `% 2` and `% 4`, as the math checks out there. I changed it for consistency there, but am happy to change it back.
- Using boolean also wasn't suggested initially, I'm happy to change back.
Reviewed By: krallin
Differential Revision: D31379381
fbshipit-source-id: a0bac26ebabd32a6c65f717512de998ef5dc37c8
Summary:
`commit_lookup_pushrebase_history` is an endpoint that tries to traverse Pushrebase and Commit Sync mappings to find the commit's pushed version for a landed commit. But currently it traverses Commit Sync mapping blindly because it doesn't know the sync direction. This can (very rare) lead to an inaccurate result.
Let's use the `source_repo` column I've introduced in this diff stack and don't traverse in the wrong direction if we know it is wrong.
Reviewed By: StanislavGlebik
Differential Revision: D30975759
fbshipit-source-id: 9c5ecf059dcdebf0c91f0c5545f0c6e95610c2ec
Summary:
The backfiller should use the background session class so that we ensure data
is written to all blobstores in a multiplex.
Reviewed By: yancouto
Differential Revision: D31429168
fbshipit-source-id: 32c767dbef291771565f73cedf3cd01c1a3cce40
Summary:
When rederiving batches of commits, the batch derivation process must mark
these commits as derived so that rederivation will continue with the next
batch.
Reviewed By: yancouto
Differential Revision: D31429169
fbshipit-source-id: e9f6a84a0391ee8d72a0007f39e755410bfac724
Summary: Since we migrated all derived data types to use manager, we can now delete a bunch of code.
Reviewed By: krallin
Differential Revision: D31344999
fbshipit-source-id: db864bdc3ba0f95cb34be6e554d629d254f09608
Summary: The test output was previously incorrect due to wrong rounding from jq tool
Reviewed By: ahornby
Differential Revision: D31429221
fbshipit-source-id: 2979e393c6f6c1b52e41d732f155275166062bff
Summary:
fix the values in the test, the test was broken due to changes in jq
jq used to provide incorrect rounding.
Reviewed By: ahornby
Differential Revision: D31390426
fbshipit-source-id: ab4d7014109d23aa5b4fb95db4f485cea70e5b05
Summary: I'm going to reuse that derived type for unit-test in 2DService. And for that I need to make it library.
Reviewed By: HarveyHunt
Differential Revision: D31340518
fbshipit-source-id: 3960c0d3ae9a72e1fa6dc9afb170c0c708b3cdf8
Summary: Same as D30974102 (91c4748c5b) but for mercurial filenodes.
Reviewed By: markbt
Differential Revision: D31170597
fbshipit-source-id: fda62e251f9eb0e1b6b4aa950d93560b1ff81f67
Summary:
This fixes these tests (which are blocking the deployment):
```
FAILED eden/mononoke/mononoke_types:mononoke_types-unittest - content_metadata::test::content_metadata_blob_roundtrip
FAILED eden/mononoke/mononoke_types:mononoke_types-unittest - content_metadata::test::content_metadata_thrift_roundtrip
FAILED eden/mononoke/mononoke_types:mononoke_types-unittest - file_contents::test::file_contents_blob_roundtrip
FAILED eden/mononoke/mononoke_types:mononoke_types-unittest - file_contents::test::file_contents_thrift_roundtrip
```
They were broken since the update of quickcheck, as it now checks with big values for ints, which broke these tests as the code didn't do a good job of converting between u64/i64. This diff fixes that.
Usually, we just want to use `as` conversion everywhere. Thrift only stores i64. but it's fine to store a negative value that will correctly overflow back to the correct positive value of u64. In practice though, this shouldn't really happen, as these are sizes, and we're never getting anywhere near the u64 limit.
Using `TryInto` doesn't cause these overflows, but for just storage, it makes things work in less cases.
Reviewed By: krallin
Differential Revision: D31379001
fbshipit-source-id: feeb87a62148f97b3bd467e8c2ef2156c8e3329a
Summary:
If a file is replaced by a move or copy from another file, we should ignore
this copy info when computing the renames in a diff. If we do not, then we
will fail to include the deletion of the copy source in the case of a move, and
the copy information doesn't add anything anyway as the destination file will
just show as modified.
Reviewed By: mitrandir77
Differential Revision: D31180151
fbshipit-source-id: c89a8ae26a516fd958406bb967a587b3b6c36a48
Summary: Switch derivation of Git trees to use the `DerivedDataManager`.
Reviewed By: yancouto
Differential Revision: D31303798
fbshipit-source-id: 193f5d373a56a0d1099f49db76758227d15c3762
Summary: Add overrides for changesets, filenodes and bonsai_hg_mapping in the derived data manager.
Reviewed By: yancouto
Differential Revision: D31378456
fbshipit-source-id: b1faa543ca65fa041d2d0ddc908ea5fb950d023a
Summary:
Vendoring this patch: https://github.com/curl/curl/pull/7737 to curl-sys rust crate. On windows the hg client is using curl that's bundled with sys-curl. I need this patch to have unix domain sockets working in hg client on windows.
I had to manually vendor https://raw.githubusercontent.com/mzr/curl/57e7ec4dbe4dd2831de51f2644879387d2ea7b44/docs/INSTALL because reindeer didn't do it. IDK why.
oss-eden-{darwin,linux,windows}-getdeps fail with:
```
FAILED: eden/scm/lib/backingstore/CMakeFiles/rust_backingstore.cargo eden/scm/lib/backingstore/debug/libbackingstore.a eden/scm/lib/backingstore/release/libbackingstore.a
cd /data/sandcastle/temp/fbcode_builder_getdeps/shipit/eden/eden/scm/lib/backingstore && /data/sandcastle/temp/fbcode_builder_getdeps/installed/cmake-hQhVzQT-WzFKTeqXjLxo5lLi8IG4_MjX2-YRqptCUVs/bin/cmake -E remove -f /data/sandcastle/temp/fbcode_builder_getdeps/shipit/eden/eden/scm/lib/backingstore/Cargo.lock && /data/sandcastle/temp/fbcode_builder_getdeps/installed/cmake-hQhVzQT-WzFKTeqXjLxo5lLi8IG4_MjX2-YRqptCUVs/bin/cmake -E env CARGO_TARGET_DIR=/data/sandcastle/temp/fbcode_builder_getdeps/build/eden/eden/scm/lib/backingstore CARGO_HOME=/data/sandcastle/temp/fbcode_builder_getdeps/build/eden/_cargo_home cargo build --release -p backingstore --features fb
Blocking waiting for file lock on package cache
Blocking waiting for file lock on package cache
error: failed to calculate checksum of: /data/sandcastle/boxes/eden-trunk-hg-fbcode-fbsource/third-party/rust/vendor/curl-sys-0.4.45+curl-7.78.0/curl/docs/INSTALL
Caused by:
failed to open file `/data/sandcastle/boxes/eden-trunk-hg-fbcode-fbsource/third-party/rust/vendor/curl-sys-0.4.45+curl-7.78.0/curl/docs/INSTALL`
Caused by:
No such file or directory (os error 2)
```
Not idea how to fix this. Seems related to the fact that reindeer didn't vendor docs/INSTALL.
# EDIT:
# It might been caused by some bug in hg. now it's fine
# The failure in oss-eden-linux-getdeps looks unrelated (something with rocksdb)
Reviewed By: krallin
Differential Revision: D31370778
fbshipit-source-id: a1245f8cb6b58f5765e34c95dfd78325a8e6e457
Summary:
This diff adds snapshot tests for most eden api wire types, while at the same time making the testing code much smaller, including tests for "wire" and "serialize" roundtrips.
## Context:
The previous diff had added an easy way to add snapshot tests. This stack aims to simplify the wire protocol code needed to create/modify an endpoint. A good thing to do before that is to add snapshot tests to all wire types, so that if we change them in a refactor, we're confident they still work exactly the same. This will also be useful when a type is changed in the future.
## How this makes tests easier
- In order to create snapshot tests, we need example objects to test with. Luckily we already use a framework for generating example objects (quickcheck::Arbitrary), so the idea here is to use that to make snapshot tests as automatic as possible.
- At the same time, the "wire" and "serialize" roundtrip tests (which also used Arbitrary), can also be made more automatic.
Now, using a simple helper, `auto_wire_tests!(WireObjectName)`, it is possible to derive all three types of tests automatically. This makes the current code smaller, and safer as we now have the additional safety provided by snapshot tests.
## Observations
- Not all wire types had tests implemented for them (I assume because it was too much work doing so, and might have done that myself in the past), I only moved the ones that already had. I'll do another pass and add remaining objects on a following diff.
- There are a couple actual non-refactor changes. I'll add comments explaining those.
- quickcheck crate is using quite an old version. I tried updating but it snowballed into something much more complicated, so I kept using the old version. We'll need to get to it at some point, though.
Reviewed By: markbt
Differential Revision: D31019233
fbshipit-source-id: 30c4a90848d0a5dcaffb89b9a0cd1cebfe4ace55
Summary:
https://pxl.cl/1Qh3j
This is the most called edenapi endpoint by far. If we sample logging of it, we can increase the retention of the scuba table.
if we wish, it's possible to not change retention for some "non-trivial" requests, but I haven't done that.
Reviewed By: liubov-dmitrieva
Differential Revision: D31277391
fbshipit-source-id: ee19e9daa4cd39c5d3eac1063e82aa40fc108bc7
Summary:
This is used to uniquely identify requests in gotham. It's logged to output, on errors, and on Scuba.
Problem: On scuba, this packs very badly, as it is a large string (36 chars), unique for all requests.
Solution: Let's get a prefix of it, it should reduce size used on scuba. Got a prefix of size 5.
This affects both LFS and EdenApi.
Pros:
- Reduces size
- Very easy fix
Cons:
- More chance of conflict. The space of this id is 16^5 = 10^6. There will surely be conflicts, but maybe that's not a huge deal?
Alternative: Using 8 digits, that's about 4bi ids, which will reduce conflicts significantly in exchange for more space.
Why not use an int id (example: u64), or using other characters in id (not only hex): This would reduce the size of data significantly, but has drawbacks:
- For int, would require a big refactoring, as everything assumes the id to be string. Specially since this goes through client-server, might be complicated.
- Not just getting a prefix means more processing on each request, and means we need to recalculate it everytime.
- Size reduction might not be that big, as scuba already packs stuff pretty well.
Reviewed By: krallin
Differential Revision: D31305547
fbshipit-source-id: 23f6b6cb7de5b7a090864db414d4d71cd68c4946
Summary: D31115820 (ae87b82eaf) updated quickcheck, but there's some stuff we need to fix forward. This diff fixes the remaining failures I could find.
Reviewed By: farnz
Differential Revision: D31305392
fbshipit-source-id: a6684d47833bc0fd933751c13cdd71392cb1833b
Summary:
Somehow before this fix I've seen us runing out of semaphores and deadlocking
because not freeing semaphores immediately after finishing running the
function requiring git repo.
Reviewed By: mojsarn
Differential Revision: D31310626
fbshipit-source-id: ba12b2d4918ecc30ca0aa6ff011176f7634badf9
Summary: Need this for cargo check/rust-analyzer to work.
Reviewed By: guswynn
Differential Revision: D31319911
fbshipit-source-id: ebd3fa72d8fc3667391a2067f95cab9e5f53301f
Summary: I'm parsing some deeply-nested JSON, and it's running into the limit. This feature enables a potential footgun, but even with the feature enabled you have to add code to reach for said footgun.
Reviewed By: jsgf
Differential Revision: D31284743
fbshipit-source-id: 00ea5d7d7db8bdeb878d48fe390831f39e007409
Summary:
It was added on the client side in D30686450 (7eb11cb392) to handle octopus merges correctly,
let's add it on mononoke as well, otherwise new_streaming_clone fails to parse
a revlog.
Reviewed By: mitrandir77
Differential Revision: D31305651
fbshipit-source-id: 976d7fdb8775f859e4732fd8a68f9b28f04ce4f9
Summary:
We want to reduce duplicated work. Since requests will be consistently hashed, each instance of service will receive some set of requests from multiple clients. By storing requests together with shared future derivation, any client can get the state of derivation.
In addition upon receiving requests we can clean up the map to remove already completed futures so the map will not grow indefinitely.
Reviewed By: StanislavGlebik
Differential Revision: D30776322
fbshipit-source-id: 961055f8b3328378451edd677506d7e716a9afd2
Summary:
This updates the following crates to their latest versions:
- `rand`: 0.7 => 0.8
- `quickcheck`: 0.9 => 1.0
Both crates introduced some breaking changes, so affected clients had to be fixed accordingly. Most changes are rather mechanical and shouldn't change the existing logic. In addition, a few buggy property tests were uncovered, presumably due to `quicktest` becoming smarter with its choice of inputs in the newer version, and the fixes are included in this diff.
Reviewed By: yancouto
Differential Revision: D31115820
fbshipit-source-id: 60a61dfac3236fd93cd4f03b86506654d81d330f
Summary:
The lookup of content ids was not working as expected.
Reasons:
- If content id was provided, we never checked it was actually on the blobstore, and failed when building the metadata for it. This was happening since D30016963 (f64520a312)
This diff fixes that by explicitly checking it exists. I also added some comments to clarify.
Reviewed By: liubov-dmitrieva, StanislavGlebik
Differential Revision: D31268102
fbshipit-source-id: 9801a7f4ce1536e68f44ebe114087e53cf094d7a
Summary:
One way to mitigate the skiplist inefficiencies is to just use segmented
changelog if we can.
Currently we can do it only for commits on master bookmarks for most repos but
upcoming defrag work from farnz would allow us to include release branches
there as well. That will cover most of the is_ancestor queries.
NOTE: This is not the end of diffs switching us to use segmented changelog. I'm planning to also do it for other places where we do ancestry checks and lower common ancestor operations.
Reviewed By: StanislavGlebik
Differential Revision: D31169338
fbshipit-source-id: 9d4b27d3fb22016b0239c52d71a9b2d9ae9a103b
Summary: This would allow us to benefit from segmented changelog server-side
Reviewed By: StanislavGlebik
Differential Revision: D31169337
fbshipit-source-id: 3c648ed2f144cee57de7c319692a37b04adf5705
Summary:
this admin command in D29734333 (3f8de3336a) started depending on innerRepo because it
needed access to ephemeral blobstore. It didn't need other parts of inner repo
so there's no need for that dependency.
Reviewed By: krallin
Differential Revision: D31210293
fbshipit-source-id: 004fb95d17e7e1d3095db0258f3c55dadaf5524c
Summary:
This mode rederives commits and compares that what was rederived is the same to
what has already been derived. It's useful to test any changes to derive data
logic and make sure these changes don't have any bugs
Reviewed By: markbt
Differential Revision: D31143741
fbshipit-source-id: 618dbf12ab444b5686d50f83a590314adc6c5dda
Summary: Remove some more path clones by changing within_restrictions to take Option<&MPath>
Reviewed By: StanislavGlebik
Differential Revision: D31175004
fbshipit-source-id: 92f0b4b594c4b3e30258acd019e7f42d9b3bc5fb
Summary: Remove a couple of clones of path by moving up ChangesetPathContentContext::new_with_fsnode_entry
Reviewed By: StanislavGlebik
Differential Revision: D31175005
fbshipit-source-id: fa686f69087e317877c2c9a9c0cffe05a6006775
Summary:
I believe this is the reason for -
https://fb.workplace.com/groups/238845853462687/posts/845939069420026. We used
default config that doesn't do any chunking and puts large files as a single
blobs.
Let's not do that
Reviewed By: farnz
Differential Revision: D31209331
fbshipit-source-id: 43c2d2ab7caac110a1474856da09c119a5e72429
Summary:
EdenApiUploads: eliminate extra lookup if no stacks
In EdenApiUploads we filter heads first and then we filter the commits belonging to these stacks.
However, in some usecases users don't use stacks. If there is no any single stack, the second lookup would be redundant and it would be nice to avoid it completely.
We can pass a flag to the upload code saying that extra filtering is not needed.
For example, in configerator repo users usually don't do stacks.
Reviewed By: markbt
Differential Revision: D31203489
fbshipit-source-id: 0921a01198bfc377afc3af3f7319fd0c5fec04d7
Summary: Same as D30974102 (91c4748c5b) but for mercurial cs.
Reviewed By: StanislavGlebik
Differential Revision: D31145642
fbshipit-source-id: c1be7b49bf0cbe70b844f1a31de706215a51d1ae
Summary: Same as D30974102 (91c4748c5b) but for fastlog.
Reviewed By: ahornby
Differential Revision: D31142066
fbshipit-source-id: 44a79e8a9db180736324db734b018344a77c070a
Summary:
Same as D30974102 (91c4748c5b) but for deleted manifest.
Needed some changes regarding using `DerivationContext` instead of `BlobRepo`.
Reviewed By: StanislavGlebik
Differential Revision: D31121260
fbshipit-source-id: f37daac320173b0896f12c83bdd8a723d22ec876
Summary:
Same as D30974102 (91c4748c5b) but for fsnodes.
Needed some changes regarding using `DerivationContext` instead of `BlobRepo`.
Reviewed By: StanislavGlebik
Differential Revision: D31113044
fbshipit-source-id: 6e996135f59f26e76e52b0b24ea61917216d1e53
Summary:
Same as D30974102 (91c4748c5b) but for skeleton manifest.
Needed some changes regarding using `DerivationContext` instead of `DerivedDataManager`.
Reviewed By: StanislavGlebik
Differential Revision: D31111484
fbshipit-source-id: eacc1d3247dffac4537745ec2a2071ef0abcbd43
Summary:
Same as D30974102 (91c4748c5b) but for changeset info.
This turned out quite simple, as we already have the bonsai changeset, so there's no need to do any async stuff.
Reviewed By: StanislavGlebik
Differential Revision: D31110319
fbshipit-source-id: 952686ae5583b858361b7a2a67fe914bfe5239d6
Summary:
We might have a somewhat weird case - a file was replaced with a directory and
then in the next the same file was deleted again (even though this file doesn't exist
anymore). In that case we need to make sure these two commits are in two
different stacks of commits, however previously we weren't doing that. This
diff fixes it.
Reviewed By: markbt
Differential Revision: D31168174
fbshipit-source-id: 4b9986e615ec98b6452ff81b113124d14f236382
Summary: .drain() retains the drained container and its heap allocation for reuse, but as we're not reusing the container, moving the contents into_iter() makes the intent clearer
Reviewed By: StanislavGlebik
Differential Revision: D31149817
fbshipit-source-id: 07cc8b7cabc9b1d522daee8b13cfa6eeb96e2d30
Summary: .drain() retains the drained container and its heap allocation for reuse, but as we're not reusing the container, moving the contents into_iter() by for..in makes the intent clearer.
Reviewed By: StanislavGlebik
Differential Revision: D31149816
fbshipit-source-id: 63c7bba8a457e62a37944aecd8ec8c42dac8deaa
