Summary: To aid with debugging, return the session UUID to the HTTP client in a header.
Reviewed By: krallin
Differential Revision: D16439125
fbshipit-source-id: fa03b9e5aa4940f0a7023859a9a3d3f38b29565b
Summary:
The API server logs different information for Thrift and HTTP requests, such as logging
future poll time for Thrift requests, but not HTTP. In order to add this logging, it would need to
be duplicated for both HTTP and Thrift logic.
Refactor the logging to perform future stats and parameter collection in the repo actor, so that it
is performed in one common place. Further, reduce the duplicated code in the thrift request handling.
Also, update the logging to log two entries to scuba - one when the core repo actor completes its functionality
and another when the HTTP / Thrift handler completes. This allows for some specific interface information (Such
as HTTP return code or Thrift response size) to be logged.
Reviewed By: fanzeyi
Differential Revision: D16417996
fbshipit-source-id: 61ace1eb3253e7293c126805d4cf5dc100491503
Summary: Makes it easier to use apiserver scuba table
Reviewed By: HarveyHunt
Differential Revision: D16407281
fbshipit-source-id: fe114fa637e5470833fd5bfec8c58609d3d0e272
Summary:
Improve memory usage on mononoke startup and reduce number of small allocations. Done via:
* Pre-size CHashMap used by SkiplistEdgeMapping, working around the 4x multiplier and lack of load factor awareness in CHashMap::with_capacity
* Add a SingleEdge optimization to SkiplistNodeType so as to save vector overhead in the case of one edge ( this is the common case )
* Size the HashMap in rust thrift deserialization with HashMap::with_capacity
Reviewed By: krallin
Differential Revision: D16265993
fbshipit-source-id: 99c3a7149493d824a3c00540bc5557410d0273fc
Summary:
Previously, we were passing a specially created core context that doesn't
use a Scuba logger. Update this to use a CoreContext with a scuba logger in it.
NOTE: The scuba logger in the CoreContext isn't mutable, which means that to add new values to it,
the ScubaSampleBuilder needs to be cloned. However, this means that the contents of the sample will
diverge:
let mut scuba = ctx.scuba().clone();
scuba.add("foo", 1);
// Won't contain foo
ctx.scuba().log();
This is annoying for the API Server as it would be nice to pass CoreContext into the Repo actor and then
log some things from there (Such as params).
Mononoke works around this by cloning the ScubaSampleBuilder after common values have been added
(such as server information) and then logging to scuba at the end of each request. Perhaps we should do this
for the API Server too. i.e. we clone the ScubaSampleBuilder from CoreContext once we enter the Repo actor
code so that we have a central place to record params etc and log once the actor is done, logging another sample
once the Thrift or HTTP code is done too.
Reviewed By: StanislavGlebik
Differential Revision: D16341284
fbshipit-source-id: c28083b04643421d24270e54557552551b938394
Summary:
The response_time middleware was used by both the scuba and slogger middleware.
Now that they're both gone, let's move the response_time into core_context.
Reviewed By: krallin
Differential Revision: D16356804
fbshipit-source-id: e0a5caff8c4fae97d73a7fa6fecd92aa0538ad5f
Summary:
The web part of the API Server has been using fake CoreContexts. Implement a middleware
that creates a new CoreContext for each request and allows the web handles to access it. Further,
move the logic from the ScubaMiddleware and SloggerMiddleware into the CoreContext middleware and remove the code.
Reviewed By: fanzeyi
Differential Revision: D16341282
fbshipit-source-id: dbd9ad2f3ec07314b180ca905df977effdc48130
Summary:
It's used only in a very few places, and most likely that's by accident. We
pass logger via CoreContext now
Reviewed By: krallin
Differential Revision: D16336953
fbshipit-source-id: 36ea4678b3c3df448591c606628b93ff834fae45
Summary:
The API Server uses Mononoke's blobrepo code and therefore the
stats that they implement. However, without starting a task to aggregate the stats,
they will never be collected.
Spawn the required future on the tokio runtime to allow stats to be exported.
Reviewed By: farnz
Differential Revision: D16280673
fbshipit-source-id: b97efe22208b20b175e67da2c5c0c29f407d0e06
Summary:
Move fb_status into open source fb303 and rename it to fb303_status.
Also add a getStatus() call to BaseService.
Reviewed By: wez
Differential Revision: D15800271
fbshipit-source-id: de217fb1245618d757ee853cb3ab19c7aa5dc964
Summary:
It's still clowny that we initialize CoreContext in 3 different places in
Apiserver, that's something we need to cleanup.
Reviewed By: HarveyHunt
Differential Revision: D16180999
fbshipit-source-id: 4f443e3ef25e4797500d04f5ddf6bf59e91ffa86
Summary:
Report to Scuba whenever someone tries to access a blobstore which is blacklisted. Scuba reporting is done for any `get` or `put` method call.
Because of the possible overload - given the high number of requests mononoke receives and that CensoredBlobstore make the verification before we add the caching layer for blobstores - I considered reporting at most one bad request per second. If multiple requests to blacklisted blobstores are made in less than one second, only the first request should be reported. Again, this is not the best approach (to not report all of them), but performance wise is the best solution.
NOTE: I also wrote an implementation using `RwLock` (instead of the current `AtomicI64`), but atomic variables should be faster than using lockers so I gave up on that idea.
Reviewed By: ikostia, StanislavGlebik
Differential Revision: D16108456
fbshipit-source-id: 9e5338c50a1c7d15f823a2b8af177ffdb99e399f
Summary:
Added an option to control for which repositories should censoring be
enabled or disabled. The option is added in `server.toml` as `censoring` and it
is set to true or false. If `censoring` is not specified, then the default
option is set to true ( censoring is enabled).
By disabling `censoring` the verification if the key is blacklisted or not is
omitted, therefor all the files are fetchable.
Reviewed By: ikostia
Differential Revision: D16029509
fbshipit-source-id: e9822c917fbcec3b3683d0e3619d0ef340a44926
Summary: This diff moves content SHA1 computation to use the new `CacheManager` so it can benefit from caching stuff in memcache.
Reviewed By: krallin
Differential Revision: D15978549
fbshipit-source-id: f05f1a1382e291dc08989cf11be752720e158888
Summary:
Add cachelib layer to `CacheManager`.
`CacheManager` behaviors:
| cachelib | memcache | Behavior |
| -- | -- | -- |
| Miss | Miss | Resolve `fill` future, fill both cache layer |
| Miss | Hit | Fetch data from memcache and fill cachelib with the data fetched |
| Hit | Miss | Return data fetched from cachelib, DO NOT fill memcache |
| Hit | Hit | Return data fetched from cachelib |
Reviewed By: StanislavGlebik
Differential Revision: D15929659
fbshipit-source-id: f7914efc7718c614f39a8fd6ad5e6588773fdd78
Summary: When caching is not enabled, initializing a volatile cache pool will return an `Err`. So, when caching is not enabled, we shouldn't try to grab a cache. I also added a little inline documentation to clarify the behavior here.
Reviewed By: farnz
Differential Revision: D15963514
fbshipit-source-id: f3580ce9e4287047435d6f27d099c12306b6b30c
Summary:
Currently, we implicitly expect that caching is enabled if we're dealing with a remote repository, but that means cachelib must be enabled when running with a remote repository, and that is ... slow.
This can be problematic in two cases:
In tests. It makes MySQL tests unbearably slow, and a little more flaky because we end up using so much CPU. With this patch, MySQL tests remain slower than SQLite tests, but by a factor of < 2, which is a pretty substantial improvement.
Running trivial administrative commands (e.g. a `mononoke_admin`), notably using a dev build (which right now unbearably slow). With this patch, such a trivial command is about 6x faster:
```
[torozco@devvm4998.lla1 ~/fbcode] time buck-out/gen/scm/mononoke/admin#binary/admin --repo-id 2102 --mononoke-config-path /home/torozco/local/.mononoke_exec/config/PROD --skip-caching bookmarks list --kind publishing
Jun 21 08:57:36.658 INFO using repo "instagram-server" repoid RepositoryId(2102)
master c96ac4654e4d2da45a9597af859adeac9dba3d7ca964cb42e5c96bc153f185e3 2c5713ad27262b91bf1dfaf21b3cf34fe3926c8d
real 0m5.299s
user 0m5.097s
sys 0m0.699s
[torozco@devvm4998.lla1 ~/fbcode] time buck-out/gen/scm/mononoke/admin#binary/admin --repo-id 2102 --mononoke-config-path /home/torozco/local/.mononoke_exec/config/PROD bookmarks list --kind publishing
I0621 08:57:59.299988 1181997 CacheAllocator-inl.h:3123] Started worker 'PoolRebalancer'
Jun 21 08:57:59.328 INFO using repo "instagram-server" repoid RepositoryId(2102)
master c96ac4654e4d2da45a9597af859adeac9dba3d7ca964cb42e5c96bc153f185e3 2c5713ad27262b91bf1dfaf21b3cf34fe3926c8d
real 0m28.620s
user 0m27.680s
sys 0m2.466s
```
This is also nice because it means the MySQL tests won't talk to Memcache anymore.
---
Note: in this refactor, I made `Caching` an enum so it can't accidentally be swapped with some other boolean.
---
Finally, it also uses up quite a bit less RAM (we no longer need 2GB of RAM to output one line of bookmarks — although we're still using quite a bit!):
```
[torozco@devvm4998.lla1 ~/fbcode] env time buck-out/gen/scm/mononoke/admin#binary/admin --skip-caching --repo-id 2102 --mononoke-config-path /home/torozco/local/.mononoke_exec/config/PROD bookmarks list --kind publishing
Jun 21 09:18:36.074 INFO using repo "instagram-server" repoid RepositoryId(2102)
master abdd2f78dafeaa8d4b96897955a63844b31324f9d89176b3a62088d0e2ae2b22 1702392d14bf7a332bf081518cb1ea3c83a13c39
5.08user 0.68system 0:05.28elapsed 109%CPU (0avgtext+0avgdata 728024maxresident)k
6776inputs+0outputs (8major+115477minor)pagefaults 0swaps
[torozco@devvm4998.lla1 ~/fbcode] env time buck-out/gen/scm/mononoke/admin#binary/admin --repo-id 2102 --mononoke-config-path /home/torozco/local/.mononoke_exec/config/PROD bookmarks list --kind publishing
I0621 09:19:01.385933 1244489 CacheAllocator-inl.h:3123] Started worker 'PoolRebalancer'
Jun 21 09:19:01.412 INFO using repo "instagram-server" repoid RepositoryId(2102)
master abdd2f78dafeaa8d4b96897955a63844b31324f9d89176b3a62088d0e2ae2b22 1702392d14bf7a332bf081518cb1ea3c83a13c39
26.96user 2.27system 0:27.93elapsed 104%CPU (0avgtext+0avgdata 2317716maxresident)k
11416inputs+5384outputs (17major+605118minor)pagefaults 0swaps
```
Reviewed By: farnz
Differential Revision: D15941644
fbshipit-source-id: 0df4a74ccd0220a786ebf0e883e1a9b8aab0a647
Summary: Add type safety to `abomonation_future_cache` by requiring usage of `VolatileLruCachePool`, and make that change for all usages of `LruCachePool`.
Reviewed By: farnz
Differential Revision: D15882275
fbshipit-source-id: 3f192142af254d7b6b8ea7f9cc586c2034c97b93
Summary: We are copying `MPath` a lot, this change should reduce amount of allocations and memory reuse for `MPath`
Reviewed By: farnz
Differential Revision: D15939495
fbshipit-source-id: 8da8f2c38f7b46f27d0df661210c9964aed52101
Summary:
In the next diffs it will be used inside blobrepo to correctly generate hg
filenodes. This diff just moves the functions to a common place.
There are a few cleanups that can be done - see next diff.
Reviewed By: aslpavel
Differential Revision: D15896733
fbshipit-source-id: 19b58e4eb73fd4897d7c8ab28c0dcd8786124f2a
Summary: add derive `Serialization` and `Deserialization` so we could cache these types.
Reviewed By: kulshrax
Differential Revision: D15678816
fbshipit-source-id: ce4a15314b3dba4eb3cd1425ee0e47d4d9ed5771
Summary: This diff adds `CacheManager` to Mononoke API Server that is responsible to manage caches (memcache currently, and cachelib in the future).
Reviewed By: StanislavGlebik
Differential Revision: D15678253
fbshipit-source-id: 3716c11db1be00d8526f43c4f6b4a2434e6c87c3
Summary:
Use `application/cbor` rather than `application/octet-stream` as the
MIME type for streaming responses containing CBOR records. In practice, this
doesn't appear to make any difference on the client side, but it's still nice
to provide the correct MIME type.
Reviewed By: quark-zju
Differential Revision: D15838482
fbshipit-source-id: b92f0d42d4696ac3392468d53f7138055d40eb5b
Summary:
Previously, all of the handlers for Eden API operations would buffer an in-progress response in memory, serialize it, and send it in one go, resulting in high latency for responses. However, the underlying Mononoke APIs used by these handlers are streaming APIs, so it is possible to send the response incrementally as the data for each entry is received.
This diff adds an optional `stream` query parameter to all of the Eden API endpoints. When set to `true`, the API server will stream the response as a series of independently serialized CBOR responses rather than as one big CBOR response. Since this is a breaking format change, the default behavior remains as before to avoid breaking existing clients. Once this is deployed, we'll migrate clients to use the streaming functionality, and then eventually remove the gating.
Reviewed By: xavierd
Differential Revision: D15784634
fbshipit-source-id: c49dfc75498dfb16dbe5fc2b7a669b282a2bfc1b
Summary: The API server's Eden API endpoints (which all return CBOR responses) are usually accessed from behind a VIP, making it difficult to know which actual API server host served the request. To improve debuggability, let's add the (non-standard, but common) "x-served-by" header to CBOR responses containing the hostname of the API server host.
Reviewed By: quark-zju
Differential Revision: D15717893
fbshipit-source-id: 626d752ecb647d4e883e9127a3c549f1e40f355e
Summary: This diff will make API server to log the client certificate every request used to scuba.
Reviewed By: StanislavGlebik
Differential Revision: D15572329
fbshipit-source-id: 695d5fefdae083a36fea27e806aa797c657cba3b
Summary:
This adds support for recording server-side whether a given bookmark is publishing and / or pull-default. This is a change on the way towards supporting Infinite Push in Mononoke. This diff will require schema changes on `xdb.mononoke_production`.
There isn't a whole lot of new user-facing functionality in this particular diff. For starters, nobody can edit this flag on bookmarks, and pushes that create a new bookmark will always result in setting a bookmark as publishing AND pull_default.
What this change does however introduce is the notion of `BookmarkHgKind`, which is represents the behavior of this bookmark as far as Mercurial operations are concerned.
There are 3 such kinds, which are relevant in different parts of the codebase:
- PullDefault - this is useful when we want to respond to listkeys queries.
- Publishing — this is useful when we want to identify commit Phases.
- All - this is useful when we want to respond to listkeyspatterns.
Note that only the first two groups are cached in CachedBookmarks.
---
There are a few things going on in this diff (which logically have to happen together):
- First, I updated the `Bookmarks` trait and its various implementations to expose new methods to select Bookmarks based on their hg kind. There's one method per hg kind, and all the methods use a `Freshness` parameter to determine whether to hit cache or not.
- Second, I refactored the various bookmark-access methods in blobrepo. A few of them were duplicative of each other, and some were unused, which became a bigger problem now that we had more (for publishing, pull default, etc.). We are now down to just one method that doesn't hit cache (which is used only by the blobimport script and some tests — perhaps this could be changed?).
- Third, I updated the call sites for the methods that were udpated in step 2 to use the proper method for their use case.
---
Here's a summary of where each method is used (I'm only listing stuff that isn't unit tests):
- `get_bonsai_heads_maybe_stale`:
- `SqlPhases`'s `get_public`
- `build_skiplist_index`
- `get_bonsai_publishing_bookmarks_most_recent`:
- Blobimport (perhaps we can update this to a `maybe_stale` read?)
- `get_pull_default_bookmarks_maybe_stale`
- `listkeys` implementations
- `get_publishing_bookmarks_maybe_stale`
- API Server's `get_branches`
- `get_bookmarks_by_prefix_maybe_stale`:
- `listkeyspatterns` implementation
---
As an aside, note that a lot of the code changes in this diff are actually in CacheBookmark's tests — I had to update those to minimize sleeps: it was fine to sleep in the earlier tests, but I introduced new quickcheck-based tests, and sleeping in those isn't ideal.
Reviewed By: StanislavGlebik
Differential Revision: D15298598
fbshipit-source-id: 4f0fe80ea98dea8af0c8289db4f6f4e4070af622
Summary: The `/gethgfiles` and `/getfilehistory` endpoints were only used by the Eden API Hyper client, which no longer exists. As such, let's clean up these old endpoints to avoid cluttering the API server code.
Reviewed By: fanzeyi
Differential Revision: D15508314
fbshipit-source-id: f4f76baf5645750a6da9b28897052151f6183e60
Summary: Add an endpoint to the API server that provides functionality similar to the `gettreepack` wire protocol command.
Reviewed By: fanzeyi
Differential Revision: D15492734
fbshipit-source-id: 7d0f113f0d33c68d5bfba5781269a92f0d5a66e8
Summary: Change the API so that the act of getting the data from a data entry (optionally) forces hash validation.
Reviewed By: xavierd
Differential Revision: D15405742
fbshipit-source-id: c127286322c707eb066f89021ac45315e89aa718
Summary:
As part of adding support for infinitepush in Mononoke, we'll include additional server-side metadata on Bookmarks (specifically, whether they are publishing and pull-default).
However, we do use the name `Bookmark` right now to just reference a Bookmark name. This patch updates all reference to `Bookmark` to `BookmarkName` in order to free up `Bookmark`.
Reviewed By: StanislavGlebik
Differential Revision: D15364674
fbshipit-source-id: 126142e24e4361c19d1a6e20daa28bc793fb8686
Summary: Add a new `/repo/eden/trees` endpoint to the API server that serves tree data in Mercurial's tree format. This endpoint uses the same structs used by the file data endpoint for framing the returned tree data.
Reviewed By: xavierd
Differential Revision: D15288534
fbshipit-source-id: 9d31b77d3714ba815b4f3b86fa75b0c5ab1a7469
Summary: Given that the on-the-wire representation of file and tree data is essentially the same, we can reuse our existing structs for file fetching for tree fetching. As such, we should rename these structs to make it clear that they are not just for file data. (This also involves updating some comments/variable names to make sure everything is consistent.)
Reviewed By: singhsrb
Differential Revision: D15286690
fbshipit-source-id: 8c7fa4392057e90a9f19beb17b0bbcbf04b7e8e7
Summary: As we add more functionality to the Eden API, we will have a lot more request structs. These structs are only used by the HTTP data fetching code, and should not be used by actual business logic. As such, while these types need to be public (so that both Mononoke and Mercurial can use them), they should not be re-exported at the top level.
Reviewed By: quark-zju
Differential Revision: D15268439
fbshipit-source-id: e7d1405d2ac234892baedbf7dbf3e133d187cb45
Summary:
This migrates the internal structures representing the repo and storage config,
while retaining the existing config file format.
The `RepoType` type has been replaced by `BlobConfig`, an enum containing all
the config information for all the supported blobstores. In addition there's
the `StorageConfig` type which includes `BlobConfig`, and also
`MetadataDBConfig` for the local or remote SQL database for metadata.
Reviewed By: StanislavGlebik
Differential Revision: D15065421
fbshipit-source-id: 47636074fceb6a7e35524f667376a5bb05bd8612
Summary:
Given that users typically connect to the API server via a VIP, it is
often unclear which actual host we're connected to. As such, API server
responses should contain the actual hostname of the server.
As a first pass at adding this, let's add a simple endpoint that just returns
the hostname, which can be used to see which hosts the VIP is sending requests
to.
Reviewed By: xavierd
Differential Revision: D15168952
fbshipit-source-id: d6fee5716df975fa1588bbf069ad6e970a779b1e
Summary: The SHA1 cache mistakenly caching only by the node hash while it is possible to have multiple files that have the same node hash while different names. This will cause the server incorrectly returning incorrect entry from the cache.
Reviewed By: StanislavGlebik
Differential Revision: D15092963
fbshipit-source-id: 608be9f2b749d0b77687ad5cf2d76d56398a543d
Summary: Now it is possible to configure and enable/disable bookmark cache from configs
Reviewed By: StanislavGlebik
Differential Revision: D14952840
fbshipit-source-id: 3080f7ca4639da00d413a949547705ad480772f7
Summary: Update the Eden API client and server to use the new DataEntry type for file content downloads.
Reviewed By: quark-zju
Differential Revision: D14907958
fbshipit-source-id: 8a7b1cbb54bdc119dda11179ff94d3efdb7e85c9
Summary: Removing this function in favor of using Key.path
Reviewed By: quark-zju
Differential Revision: D14945331
fbshipit-source-id: 6b6bb70375629edf37b2b04a86545f18e15b33b4
Summary:
Previously, the Eden API endpoints on the API server used JSON for encoding requests/responses for debugging purposes. Given that these APIs are mostly used to transfer large amounts of binary data, we should use a binary encoding scheme in production. CBOR fits the bill since it is essentially binary JSON, allowing for more efficient coding of large byte strings.
Although this is a breaking API change, given that nothing depends on these endpoints yet, it should be OK to simply change the format.
Reviewed By: xavierd
Differential Revision: D14907978
fbshipit-source-id: e0aea30d7304f4b727e2ad7fe23379457b6c3e26
Summary:
This function is difficult to justify in the context of the Rust borrow checker.
The primary concern for this pattern is preventing mutation when the object is
passed around.
We can always add the function back if it has to more than just return the
underlying value.
Reviewed By: quark-zju
Differential Revision: D14877545
fbshipit-source-id: acdd796e1bee5445c1bce5ce0ceb41a7334e4966
Summary: Add new API server endpoints for performing batch fetches of data and history. The requests and responses are currently serialized as JSON, though the format can be trivially changed later to something like CBOR (or any other format supported by serde).
Reviewed By: xavierd
Differential Revision: D14573735
fbshipit-source-id: ac46aa8a55db44b695c53f9e32cc4ea2e17504c8
Summary:
getpackv1 needs to return copy metadata together with the content. This diff
fixes it
Reviewed By: kulshrax
Differential Revision: D14668319
fbshipit-source-id: 56a6ea2eb3a116433446773fa4fbe1f5a66c5746
Summary: Implement a backing store that uses thrift protocol to communicate with Mononoke API Server.
Reviewed By: strager
Differential Revision: D13966575
fbshipit-source-id: 66f66dda6b17aecd6c6b4475ab6b004c608f457f