Bug 3873 - Pulls can race with bookmark updates on the server, causing bookmarks not to be updated at all
Summary: Pulls can race with bookmark updates on the server, causing bookmarks not to ...
Status: RESOLVED FIXED
Alias: None
Product: Mercurial
Classification: Unclassified
Component: Mercurial (show other bugs)
Version: unspecified
Hardware: All All
: normal bug
Assignee: Siddharth Agarwal
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-29 17:34 UTC by Siddharth Agarwal
Modified: 2013-07-23 17:21 UTC (History)
3 users (show)

See Also:
Python Version: ---


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Siddharth Agarwal 2013-03-29 17:34 UTC
$ hg init master
$ cd master; touch a; hg ci -Am "a"; hg book book1; cd ..
$ hg clone master slave
$ echo "[hooks]\nchangegroup = echo Press Enter to finish pulling; read -r DUMMY" >> slave/.hg/hgrc
$ cd master; touch b; hg ci -Am "b"; cd ..
$ cd slave
$ hg pull

This will pull in b, then pause until you press Enter. In a different terminal (or using job control):

$ cd ../master; touch c; hg ci -Am "c"

Now resume the slave pull. Note that the bookmark didn't get moved at all on the slave, not even to b.

We're hitting this at FB with large pulls, where the master bookmark on the server gets updated in the middle of the pull. The master bookmark on the client doesn't get updated at all.

Seems like a fix would be to either do the bookmark determination simultaneously with the changeset determination, or to do it before pulling in changesets if possible.
Comment 1 Matt Mackall 2013-03-29 18:04 UTC
FYI, you can actually do the racing commit in the hook. We use similar tricks in various places in the test suite to test similar races.
Comment 2 Matt Mackall 2013-03-29 18:05 UTC
Not sure what we can do about this short of making this happen:

http://mercurial.selenic.com/wiki/BundleFormat2
Comment 3 Siddharth Agarwal 2013-03-29 18:10 UTC
What about figuring out bookmarks before pulling in changesets? If the changeset corresponding to that bookmark does get pulled in, we move the bookmark forward. That would leave a much smaller race window.
Comment 4 Matt Mackall 2013-03-29 18:30 UTC
That's probably feasible.
Comment 5 HG Bot 2013-04-02 02:15 UTC
Fixed by http://selenic.com/repo/hg/rev/a60963c02f92
Siddharth Agarwal <sid0@fb.com>
pull: list bookmarks before pulling changesets (issue3873)

Consider a bookmark B that exists both locally and remotely. If B is updated
remotely, and then a pull is performed where the pull set contains the new
location of B, the bookmark is updated locally. However, if remote B is
updated in the middle of a pull to a location not in the pull set, the
bookmark won't be updated locally at all.

To fix this, list bookmarks before pulling in changesets, not after. This
still leaves a race open if B gets moved in between listing bookmarks and
pulling in changesets, but the race window is much smaller. Fixing the race
properly would require a bundle format upgrade.

test-hook.t's output changes because we no longer do two listkeys calls during
pull, just one.

test-pull-http.t's output changes because we now search for bookmarks before
searching for changes.

(please test the fix)