mononoke: switch the order of inserts of bonsai_hg_mapping and changesets

Summary:
Turned out that some of our commit cloud commits dont have an entry in changesets table,
but they do have an entry in bonsai_hg_mapping. It happens because of transient mysql errors.
Because of the mess we have in how we determine if a given hg changeset is present in mononoke or not,
this broken commit is considered backed up by commit cloud, while in reality its impossible to pull this commit.

This diff fixes it by first inserting changeset entry, and then bonsai_hg_mapping.
This way if an insertion of a changeset entry fails then the commit is not
considered to exist at all.
If an insertion of bonsai hg mapping fails, then commit is considered to be inserted but the whole
operation (it can be "hg cloud sync" for example) is considered failed. In this case hg client will
try to reupload this commit later.

But there's one caveat.
Hg changesets and bonsai changesets are not always round-trippable i.e.
generating from bonsai to hg and hg to bonsai might not always
produce the same results. So it's possible to push a commit which inserts changeset entry but not hg bonsai entry,
then we accidentally derive hg changeset from this bonsai and it's different from what user has pushed to mononoke.
This could lead to problems later (i.e. re-pushing the same commit again later would fail).

Another solution (inserting bonsai hg mapping and changeset entry in the same transaction) could make this issue
less likely, but it won't eliminate it completely. We discussed it, and I think it probably make sense to go
with the simpler fix first, since this problem should happen rarely in practice
(and if it happens often then we should prioritize fixing it).

Reviewed By: krallin

Differential Revision: D26845779

fbshipit-source-id: 61f8b0d835dc4aa0130e6be632c43307ff4a771f
This commit is contained in:
Stanislau Hlebik 2021-03-05 08:00:25 -08:00 committed by Facebook GitHub Bot
parent 7e8332c9a5
commit 1d7ad86d52

View File

@ -267,6 +267,17 @@ impl CreateChangeset {
let changeset_complete_fut = async move {
let ((hg_cs, bonsai_cs), _) = future::try_join(changeset, parents_complete).await?;
// update changeset mapping
let completion_record = ChangesetInsert {
repo_id: repo.get_repoid(),
cs_id: bonsai_cs.get_changeset_id(),
parents: bonsai_cs.parents().into_iter().collect(),
};
complete_changesets
.add(ctx.clone(), completion_record)
.await
.context("While inserting into changeset table")?;
// update bonsai mapping
let bcs_id = bonsai_cs.get_changeset_id();
let bonsai_hg_entry = BonsaiHgMappingEntry {
@ -279,17 +290,6 @@ impl CreateChangeset {
.await
.context("While inserting mapping")?;
// update changeset mapping
let completion_record = ChangesetInsert {
repo_id: repo.get_repoid(),
cs_id: bonsai_cs.get_changeset_id(),
parents: bonsai_cs.parents().into_iter().collect(),
};
complete_changesets
.add(ctx.clone(), completion_record)
.await
.context("While inserting into changeset table")?;
Ok::<_, Error>((bonsai_cs, hg_cs))
}
.try_timed()