[PATCH 3 of 3] pull: only prefetch bookmark when using bundle1

[Augie Fackler]

On May 29, 2015, at 6:25 PM, Pierre-Yves David <pierre-yves.david at ens-lyon.org> wrote:

> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david at fb.com>
> # Date 1432727823 25200
> #      Wed May 27 04:57:03 2015 -0700
> # Node ID a9acfde2c3126309056a92d421913599ff3781fe
> # Parent  f9a958f8bc37c17ff8b10eee45ee51379ab90124
> pull: only prefetch bookmark when using bundle1

These were part of my bulk review of marmoute’s series and are now queued. Thanks!

> 
> All bundle2 servers now support the 'listkeys' part(1), so we'll alway be able
> to fetch bookmarks data at the same time than the changeset. This should be
> enough to avoid the one race condition that this bookmark prefetching is trying
> to work around. It even allow future server to make sure everything is generated
> from the same "transaction" if they become capable of such. The current code was
> already overwriting the prefetched value with the one in bundle2 anyway.  Note
> that this is not preventing all races conditions in related to bookmark in 'hg
> pull' it makes nothing better and nothing worse.
> 
> Reducing the number of listkeys calls will reduce the latency on pull.
> 
> The pre-fetch is also moved into a discovery step because it seems to belong
> there.
> 
> (1) Because all servers not speaking 'pushkey' parts are conmpatible with the
> 'HG2X' protocol only.
> 
> diff --git a/mercurial/exchange.py b/mercurial/exchange.py
> --- a/mercurial/exchange.py
> +++ b/mercurial/exchange.py
> @@ -893,11 +893,10 @@ def pull(repo, remote, heads=None, force
>             msg = _("required features are not"
>                     " supported in the destination:"
>                     " %s") % (', '.join(sorted(missing)))
>             raise util.Abort(msg)
> 
> -    pullop.remotebookmarks = remote.listkeys('bookmarks')
>     lock = pullop.repo.lock()
>     try:
>         pullop.trmanager = transactionmanager(repo, 'pull', remote.url())
>         _pulldiscovery(pullop)
>         if _canusebundle2(pullop):
> @@ -941,10 +940,20 @@ def _pulldiscovery(pullop):
>     """Run all discovery steps"""
>     for stepname in pulldiscoveryorder:
>         step = pulldiscoverymapping[stepname]
>         step(pullop)
> 
> + at pulldiscovery('b1:bookmarks')
> +def _pullbookmarkbundle1(pullop):
> +    """fetch bookmark data in bundle1 case
> +
> +    If not using bundle2, we have to fetch bookmark before doing changesets
> +    discovery to reduce the change and impact of race condition."""
> +    if not _canusebundle2(pullop): # all bundle2 server now support listkeys
> +        pullop.remotebookmarks = pullop.remote.listkeys('bookmarks')
> +
> +
> @pulldiscovery('changegroup')
> def _pulldiscoverychangegroup(pullop):
>     """discovery phase for the pull
> 
>     Current handle changeset discovery only, will change handle all discovery
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: Message signed with OpenPGP using GPGMail
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150531/64a02310/attachment.pgp>