Page MenuHomePhabricator

rust-nodemap: falling back to C impl as mitigation

Authored by gracinet on Aug 2 2021, 6:22 AM.



This is a mitigation for

We see sometimes almost all data except the most recent revisions
removed from the persistent nodemap, but we don't know how to
reproduce yet.

This has sadly repercussions beyond just needing to reconstruct the
persistent nodemap: for instance, this automatically filters out
all bookmarks pointing to revisions that the nodemap cannot resolve.
If such filtering happens in a transaction, the update of the
bookmarks file that happens at the end of transaction loses all
bookmarks that have been affected. There may be similar consequences
for other data.

So this is a data loss, something that we have to prevent as soon as

As a mitigation measure, we will now fallback to the C implementation
in case nodemap lookups failed. This will add some latency, e.g., in
discovery, yet less than disabling the persistent nodemap entirely.

We considered implementing the fallback directly on the Python
side, but revlog.get_rev() is not systematically used, there are
also several direct calls to the index method (self.index.rev() for
a revlog instance). It is therefore more direct to implement the
mitigation in the rust-cpython wrapper.

Diff Detail

rHG Mercurial
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

gracinet created this revision.Aug 2 2021, 6:22 AM

This is for the stable branch (mitigation for a bug labeled as blocker)

Alphare accepted this revision as: Alphare.Aug 2 2021, 9:49 AM
Alphare added a subscriber: Alphare.

I'll give other reviewers a little time to review as well, but this looks fine as a workaround. I hope we'll be able to figure out the underlying cause soon.


nit: I'd have added a small comment to reference issue6554 to remove the fallback once it's fixed.

gracinet updated this revision to Diff 29775.Aug 2 2021, 3:00 PM
gracinet marked an inline comment as done.Aug 2 2021, 3:01 PM
gracinet added inline comments.

Good point, thanks.

pulkit accepted this revision.Aug 4 2021, 4:36 AM
This revision is now accepted and ready to land.Aug 4 2021, 4:36 AM
gracinet marked an inline comment as done.Aug 4 2021, 4:43 PM
This revision was automatically updated to reflect the committed changes.