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:
Checking for our exit condition after pulling a new piece of work from the buffer is good because it means we exit without doing any of the work we buffered (i.e. we'll exit quickly)
However, that approach does means that a piece of work has to go through the stream all the way to the sync step before we decide to drop it and stop the stream.
If there is not work, this means we might take a while to exit! On paper, this is fine because if there is no work, then it's OK to just take our time to exit ... but it might be a little confusing from an operator perspective.
This patch fixes that problem by checking our exit condiiton after every no-op iteration as well.
---
It's worth noting that both checks are indeed required if we want to exit quickly regardless of whether we are very busy or completely idle.
Reviewed By: ikostia
Differential Revision: D15270517
fbshipit-source-id: 06c3b100ccbf69191ac67691a2991086596a15c0
Summary:
This adds support in the hg-sync-job for graceful exits by adding an `--exit-file` parameter. When the file referenced by this parameter exists, the sync job will stop processing new entries.
Note that the sync job will actually exit only once it evaluates the exit condition, i.e. the program will only exit once we get the next entry and decide to stop processing work.
Another way to look at this is that creating the flag file guarantees the sync job will not start any new syncs from that point on, but it doesn't guarantee a timely exit.
That's probably what we want here, but if we also want to guarantee a timely exit, then we can add an additional check for the file existence in `loop_over_log_entries` next to the `loop_forever` check.
---
The idea here is to use `touch $THE_FILE` as our exit command in Tupperware to allow for gracefully exiting the process.
Reviewed By: ikostia
Differential Revision: D15263468
fbshipit-source-id: 0a5b04e662e2a4042de9e0c5207f1b1be46d1807
Summary: It makes pushes faster, especially on non-master regions.
Reviewed By: quark-zju
Differential Revision: D15279259
fbshipit-source-id: c184b68cc8b7509938849cd86bb15ef5d5f33bdd
Summary:
We've hit an issue of slow pushes to Mononoke when a commit modifies a lot of
files (>500 in our case). turned out that the problem was in the fact that
we have only one master write connection open, and each blobstore write
requires a write to mysql because of multiplexed blobstore. Because we have
only one connection open all our mysql writes are serialized, and the push is
taking too much time. It's especially bad in non-master regions.
To mitigate the issue let's add a batching in the blobstore sync queue. When
clients call `blobstore_sync_queue.add(...)` we'll send this new entry via the
channel to a separate task that would send writes in batches. That allows us to
increase throughput significantly.
Reviewed By: jsgf
Differential Revision: D15248288
fbshipit-source-id: 22bab284b0cbe552b4b51bab4027813b4278fd14
Summary:
This change has two goals:
- Put storage configuration that's common to multiple repos in a common place,
rather than replicating it in each server.toml
- Allow tools that only operate on the blobstore level - like blobstore healing
- to be configured directly in terms of the blobstore, rather than indirectly
by using a representative repo config.
This change makes several changes to repo configuration:
1. There's a separate common/storage.toml which defines named storage
configurations (ie, a combination of a blobstore and metadata DB)
2. server.toml files can also define local storage configurations (mostly
useful for testing)
3. server.toml files now reference which storage they're using with
`storage_config = "name"`.
4. Configuration of multiplex blobstores is now explicit. Previously if a
server.toml defined multiple blobstores, it was assumed that it was a
multiplex. Now storage configuration only accepts a single blobstore config,
but that config can be explicitly a multiplexed blobstore, which has the
sub-blobstores defined within it, in the `components` field. (This is
recursive, so it could be nested, but I'm not sure if this has much value in
practice.)
5. Makes configuration parsing more strict - unknown fields will be treated as
an error rather than ignored. This helps flag problems in refactoring/updating
configs.
I've updated all the configs to the new format, both production and in
integration tests. Please review to make sure I haven't broken anything.
Reviewed By: StanislavGlebik
Differential Revision: D15065423
fbshipit-source-id: b7ce58e46e91877f4e15518c014496fb826fe03c
Summary:
Seems redundant to also require callers to open_ssl to also pass a
(mostly) identical string.
Also make open_ssl special-case filenodes with sharding (though filenodes
aren't currently opened through it).
Reviewed By: StanislavGlebik
Differential Revision: D15157834
fbshipit-source-id: 0df45307f17bdb2c021673b3153606031008bee2
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:
We don't need Option<bool> or Option<Vec<T>> - in the former case, the
bool is always treated as having a default value if not present, and in the
latter, None is equivalent to Some(vec![]), so just use an empty vector for
absense.
Reviewed By: lukaspiatkowski
Differential Revision: D15051895
fbshipit-source-id: 0ac6f2e6b13357bf6e30dbfa25c7fdebd208e505
Summary:
Set the `component` to `"commitcloud"` for commit cloud statuses and
messages, rather than using custom highlight functions.
Reviewed By: quark-zju
Differential Revision: D15201944
fbshipit-source-id: 7635942a5ca029209711a2b89c32cc5fd677d22f
Summary:
Reads current replay counter, and where a bookmark would point to after this
bundle is replayed. That can be useful for debugging
Reviewed By: aslpavel
Differential Revision: D15216378
fbshipit-source-id: fd250e27c2a6d7ee407510561a36b820cc5a1d2b
Summary: Fixes the mistakes of D15174538. Apparently this method is used in production utils like the sync job, so it should use myrouter if it is provided.
Reviewed By: ikostia, StanislavGlebik
Differential Revision: D15263742
fbshipit-source-id: a6d4c4a397c627e119b164039a7c00783b63f75f
Summary:
This updates our configuration to allow using a different tier name for sharded filenodes.
One thing I'd like to call out is that we currently use the DB tier name in the keys generated by `CachingFilenodes`. Updating the tier name will therefore result in us dropping all our caches. Is this acceptable? If not, should we just continue using the old tier name.
Reviewed By: jsgf, StanislavGlebik
Differential Revision: D15243112
fbshipit-source-id: 3bfdcefcc823768f2964b4733e570e9cef57cebc
Summary:
In the later diff we'll add batching of BlobstoreSyncQueue writes. It would be
much harder to add the batching if we also have to return this boolean.
And since noboby uses it, let's just remove it
Reviewed By: farnz
Differential Revision: D15248290
fbshipit-source-id: 72c64770c1b023e9de23a5dfccd8b4482302fe96
Summary: Instead of calculating phases of each request, this diff makes sure we are keeping all public phases up to date in database
Reviewed By: StanislavGlebik
Differential Revision: D15045340
fbshipit-source-id: 1ee95eabce4ff517925d5d2b2705e26e68474d92
Summary: This test appears to have broken after two commits landed at the same time that both affected this test file. This fixes that.
Reviewed By: HarveyHunt
Differential Revision: D15244360
fbshipit-source-id: 6b3c595ecc8500190999948a31ae36cc303caa54
Summary:
The mononoke admin integration tests can be flaky when there is logging and an error, because those are respectively sent to stdout and stderr, which means they're not ordered relative to one another.
I attempted to fix this with minimal changes in D15146392, but that didn't solve the issue: StanislavGlebik reported that he still ran into a flaky test.
The reason for this is presumably that even though we write to stderr first then to stdout, there's no guarantee that the `.t` test runner will read whetever we output to stderr before it reads what we output to stdout.
I noted in that earlier diff that a more proper fix would be to write errors to stderr so they are indeed ordered relative to logging. That is what this diff does.
For consistency, I updated other fatal outcomes (bad arguments) to also log to stderr.
Reviewed By: StanislavGlebik
Differential Revision: D15181944
fbshipit-source-id: 3ca48870c39f11a7dcc57f1341f25ce61ccae360
Summary:
Previously it was logged only to stderr, which would make debugging harder.
This diff fixes it
Reviewed By: aslpavel
Differential Revision: D15215522
fbshipit-source-id: ef7e6268bd30aa8a07f307f4c18f3d8f9bf8bee6
Summary:
Note: it usually doesn't matter because delta application usually doesn't need
any fetching from blobstore. But this change is safe and can prevent problems
in future.
Reviewed By: HarveyHunt
Differential Revision: D15241499
fbshipit-source-id: 43fbfd495f0f795b90ef343ac1055d16cdda129c
Summary:
This will help us understand which servers behave poorly and whether there's
any clustering.
Reviewed By: StanislavGlebik
Differential Revision: D15229101
fbshipit-source-id: 1aae9196702ed6fb791b5265c3bdfe90e7e24ae4
Summary: We noticed that it does not work during the rollout. Let's add a test for it.
Reviewed By: StanislavGlebik
Differential Revision: D15226353
fbshipit-source-id: bc97ecd3336561b52919c65ff5625d80e29f9a13
Summary:
1) I don't think anybody uses it
2) Hook tailer has the same functionality
Reviewed By: farnz
Differential Revision: D15216418
fbshipit-source-id: 698fc7d998475fe77ff7bf1ac55068ee75a34acc
Summary:
In the case of mononoke's admin tool it's annoying for users to be required to run myrouter in the background and provide myrouter port to every command.
Thanks to this change it is no longer necessary to run admin commands through myrouter - the tool will simply use a direct connection to XDB using the sql crate.
It is important to note that the raw XDB connection via sql crate doesn't have connection pooling and doesn't handle XDB failover so it is crucial that it is never used for long-lived or request heavy use cases like running mononoke server or blobimport
Reviewed By: jsgf
Differential Revision: D15174538
fbshipit-source-id: 299d3d7941ae6aec31961149f926c2a4965ed970
Summary: Use the existing library function to read a file into a `Vec<u8>`.
Reviewed By: aslpavel
Differential Revision: D15051894
fbshipit-source-id: 853b31450556c0a2e74a09fa06e7814ac68b1052
Summary: This updates Mononoke's repo_read_write_status to fetch the reason from the database. The "Repo is locked in DB" default message is used as a fallback if the reason is NULL.
Reviewed By: HarveyHunt
Differential Revision: D15164791
fbshipit-source-id: f4cb68c28db1db996c7ef1a309b737cb781659d1
Summary:
The config will be used to whitelist connections with certain identities and
blacklist everything else.
Differential Revision: D15150921
fbshipit-source-id: e4090072ea6ba9714575fb8104d9f45e92c6fefb
Summary: Per title, `hg debughttp` now prints out the hostname that the API server reports rather than the hostname in the URL we used to connect to it. The reason for this is that if the API server is behind a VIP, we get the actual hostname rather than just the VIP URL.
Reviewed By: xavierd
Differential Revision: D15170618
fbshipit-source-id: 9af5480f9987d8ea9c914baf3b62a00ad88d1b32
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:
Disallow unknown fields. They're generally the result of a mis-editing
a file and putting the config in the wrong place, or some incomplete refactor.
Reviewed By: StanislavGlebik
Differential Revision: D15168963
fbshipit-source-id: a9c9658378cda4866e44daf6e2c6bfbdfcdb9f84
Summary:
Currently this checks for:
- all referenced files are present
- they're well-formed toml
- all the required config keys are set, and no unknown ones are set
- blob storage and metadata db are both remote or local
- repo ids are not duplicated
Reviewed By: farnz
Differential Revision: D15068649
fbshipit-source-id: ace0e7bc52bf853ac42384c4346c3b73591312e4
Summary: Seems like 15 mins is not enough, let's bump it
Reviewed By: farnz
Differential Revision: D15154597
fbshipit-source-id: 78b8a43bbc95845719245f71ac85fd22336f3ed6
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:
At the moment verify integrity script blocks master commits that do not pass
the check. Non-master commits are not blocked even if they don't pass the check
(for example, if they don't have a valid reviewer), but verify_integrity hook
still logs them to scuba.
Mononoke's verify_integrity was enabled only on master, meaning that the hook
won't run on non-master commits at all, and we won't log non-master commits to scuba.
This diff fixes it.
Reviewed By: farnz
Differential Revision: D15146725
fbshipit-source-id: ab432fb2eccae0fbcc10755f5c8447964c490285
Summary:
This introduces prefetching in the hg sync job, which allows us to separate two logically separate steps, and buffer between them:
- Fetching bundle data from Manifold so it can be replayed in Mercurial (we call this "preparing"). This can happen out of order.
- Submitting the changes to Mercurial. This must happen sequentially.
The upshot is that we can fetch new data from Manifold while we wait for Mercurial to apply it, which should increase our throughput.
---
This required a bit of refactoring for the main loop here. The reason for that is that, currently, we heavily rely on closures to capture the `log_entry` so we can pass it to Scuba, etc.
That doesn't work if we want to have buffering, because there isn't a single scope that represents
Instead, every step of the stream has its own scope, so we need to pass the `log_entry` through the stream itself. We also need to track the log entry in errors as well (e.g. to report them to Scuba).
Reviewed By: ikostia
Differential Revision: D15146682
fbshipit-source-id: f83284571f6ca90cb621c621c5165f7fbafd81b5
Summary:
This separates out the bundle processing into two separate steps: preparing (fetching the data) and syncing (sending it to Mercurial).
The underlying goal here is to extract the 1st part into a buffered stream. Doing so will require a little further rework because we need to attach error handling to the stream (in loop mode) or future (in sync-one mode), which requires further refactoring. I'll do so in a separate diff.
Reviewed By: ikostia
Differential Revision: D15121738
fbshipit-source-id: e88d6d730d413e9c1939eefa72ec18799c8b13be
Summary:
One of the tests for mononoke admin (which I introduced recently) appears to be flaky. Sometimes, output from stdout (the error that terminates the program) and stderr (logs emitted while the program runs) is flipped.
I suspect this is caused by buffering, so this patch flushes all output before writing the error (if there was one).
An alternative approach here might be to write the final error to stderr instead of stdosut (so everything goes to stderr). That feels cleaner, but it does change the interface a little bit, so I didn't take that approach just yet. That said, if nobody objects, I'm happy to pick that approach instead.
Reviewed By: farnz
Differential Revision: D15146392
fbshipit-source-id: 67481afd4802cb48d24d19052988be4a83433efd
Summary:
I introduced several new methods on the Bookmarks trait in:
- D15097549
- D15081759
- D15097935
At the same time, Pavel introduced a new implementation of that trait in CachedBookmarks:
- D14928419
That broke the build, this fixes it.
Reviewed By: farnz
Differential Revision: D15124562
fbshipit-source-id: 61e49261c3dc5bb396883e69c29afa329d5df9ab
Summary: Enabling it was a mistake as we can't enable it on our real hg servers.
Reviewed By: farnz
Differential Revision: D15122167
fbshipit-source-id: 5e28968354b7487bd9c965d8b3f23245f6bd265a
Summary:
This is a tentative implementation of batching for the sync job.
It works by running a peer once per batch and just sending new bundle
metadata on stdin. When the batch capacity fills up, we kill the peer
and run a new one.
Reviewed By: StanislavGlebik
Differential Revision: D15063281
fbshipit-source-id: b681594c023973a165047198d490207e35a354b8
Summary:
We might use it in the sync job to make sync job faster. Merging bundles allows
us to save on sql lock and opening/closing transactions
Reviewed By: ikostia
Differential Revision: D15080348
fbshipit-source-id: bf2e4ca3dd7ff8c399a3baa34ab542f67ea6efa9
Summary: This adds the ability to exclude blobimport entries when querying the count of remaining entries in the HG sync replay log.
Reviewed By: ikostia
Differential Revision: D15097549
fbshipit-source-id: ae1a9a31f51a044924fdebbdd219fff1e2b3d46a
