Patchwork [2,of,2,V2] clone: for local clones, copy over filtered branchcaches as well (issue4286)

login
register
mail settings
Submitter Siddharth Agarwal
Date Aug. 21, 2014, 11:25 p.m.
Message ID <cfc686b2b6d22888aeb8.1408663522@devbig136.prn2.facebook.com>
Download mbox | patch
Permalink /patch/5552/
State Accepted
Headers show

Comments

Siddharth Agarwal - Aug. 21, 2014, 11:25 p.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1408662329 25200
#      Thu Aug 21 16:05:29 2014 -0700
# Node ID cfc686b2b6d22888aeb8621c3e388005001f4caa
# Parent  d5c551bf9d58912c8c084b5abce76b606c870eb6
clone: for local clones, copy over filtered branchcaches as well (issue4286)

Local clones copy/hardlink over files directly, so the branchcaches should all
be valid. There's a slight chance that a read operation would update one of the
branchcaches, but we hold a lock over the source repo so they shouldn't cause
an invalid branchcache to be copied over, just a different, valid one.
Matt Mackall - Aug. 21, 2014, 11:28 p.m.
On Thu, 2014-08-21 at 16:25 -0700, Siddharth Agarwal wrote:
> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1408662329 25200
> #      Thu Aug 21 16:05:29 2014 -0700
> # Node ID cfc686b2b6d22888aeb8621c3e388005001f4caa
> # Parent  d5c551bf9d58912c8c084b5abce76b606c870eb6
> clone: for local clones, copy over filtered branchcaches as well (issue4286)

These are queued for default, thanks.
Augie Fackler - Aug. 23, 2014, 7:18 p.m.
On Thu, Aug 21, 2014 at 04:25:22PM -0700, Siddharth Agarwal wrote:
> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1408662329 25200
> #      Thu Aug 21 16:05:29 2014 -0700
> # Node ID cfc686b2b6d22888aeb8621c3e388005001f4caa
> # Parent  d5c551bf9d58912c8c084b5abce76b606c870eb6
> clone: for local clones, copy over filtered branchcaches as well (issue4286)

er, queued this version instead. Thanks!

>
> Local clones copy/hardlink over files directly, so the branchcaches should all
> be valid. There's a slight chance that a read operation would update one of the
> branchcaches, but we hold a lock over the source repo so they shouldn't cause
> an invalid branchcache to be copied over, just a different, valid one.
>
> diff --git a/mercurial/hg.py b/mercurial/hg.py
> --- a/mercurial/hg.py
> +++ b/mercurial/hg.py
> @@ -11,7 +11,7 @@
>  from node import hex, nullid
>  import localrepo, bundlerepo, unionrepo, httppeer, sshpeer, statichttprepo
>  import bookmarks, lock, util, extensions, error, node, scmutil, phases, url
> -import cmdutil, discovery
> +import cmdutil, discovery, repoview
>  import merge as mergemod
>  import verify as verifymod
>  import errno, os, shutil
> @@ -366,13 +366,20 @@
>
>              # Recomputing branch cache might be slow on big repos,
>              # so just copy it
> +            def copybranchcache(fname):
> +                srcbranchcache = srcrepo.join('cache/%s' % fname)
> +                dstbranchcache = os.path.join(dstcachedir, fname)
> +                if os.path.exists(srcbranchcache):
> +                    if not os.path.exists(dstcachedir):
> +                        os.mkdir(dstcachedir)
> +                    util.copyfile(srcbranchcache, dstbranchcache)
> +
>              dstcachedir = os.path.join(destpath, 'cache')
> -            srcbranchcache = srcrepo.join('cache/branch2')
> -            dstbranchcache = os.path.join(dstcachedir, 'branch2')
> -            if os.path.exists(srcbranchcache):
> -                if not os.path.exists(dstcachedir):
> -                    os.mkdir(dstcachedir)
> -                util.copyfile(srcbranchcache, dstbranchcache)
> +            # In local clones we're copying all nodes, not just served
> +            # ones. Therefore copy all branchcaches over.
> +            copybranchcache('branch2')
> +            for cachename in repoview.filtertable:
> +                copybranchcache('branch2-%s' % cachename)
>
>              # we need to re-init the repo after manually copying the data
>              # into it
> diff --git a/tests/test-clone.t b/tests/test-clone.t
> --- a/tests/test-clone.t
> +++ b/tests/test-clone.t
> @@ -25,12 +25,25 @@
>    .hg/store/data/b.d
>    .hg/store/data/b.i
>
> +Trigger branchcache creation:
> +
> +  $ hg branches
> +  default                       10:a7949464abda
> +  $ ls .hg/cache
> +  branch2-served
> +
>  Default operation:
>
>    $ hg clone . ../b
>    updating to branch default
>    2 files updated, 0 files merged, 0 files removed, 0 files unresolved
>    $ cd ../b
> +
> +Ensure branchcache got copied over:
> +
> +  $ ls .hg/cache
> +  branch2-served
> +
>    $ cat a
>    a
>    $ hg verify
> @@ -58,6 +71,12 @@
>    listing keys for "bookmarks"
>  #endif
>    $ cd ../c
> +
> +Ensure branchcache got copied over:
> +
> +  $ ls .hg/cache
> +  branch2-served
> +
>    $ cat a 2>/dev/null || echo "a not present"
>    a not present
>    $ hg verify
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Augie Fackler - Aug. 23, 2014, 7:19 p.m.
On Aug 23, 2014, at 3:18 PM, Augie Fackler <raf@durin42.com> wrote:

> On Thu, Aug 21, 2014 at 04:25:22PM -0700, Siddharth Agarwal wrote:
>> # HG changeset patch
>> # User Siddharth Agarwal <sid0@fb.com>
>> # Date 1408662329 25200
>> #      Thu Aug 21 16:05:29 2014 -0700
>> # Node ID cfc686b2b6d22888aeb8621c3e388005001f4caa
>> # Parent  d5c551bf9d58912c8c084b5abce76b606c870eb6
>> clone: for local clones, copy over filtered branchcaches as well (issue4286)
> 
> er, queued this version instead. Thanks!

Whoops, mpm already queued. Dropping my copy.

> 
>> 
>> Local clones copy/hardlink over files directly, so the branchcaches should all
>> be valid. There's a slight chance that a read operation would update one of the
>> branchcaches, but we hold a lock over the source repo so they shouldn't cause
>> an invalid branchcache to be copied over, just a different, valid one.
>> 
>> diff --git a/mercurial/hg.py b/mercurial/hg.py
>> --- a/mercurial/hg.py
>> +++ b/mercurial/hg.py
>> @@ -11,7 +11,7 @@
>> from node import hex, nullid
>> import localrepo, bundlerepo, unionrepo, httppeer, sshpeer, statichttprepo
>> import bookmarks, lock, util, extensions, error, node, scmutil, phases, url
>> -import cmdutil, discovery
>> +import cmdutil, discovery, repoview
>> import merge as mergemod
>> import verify as verifymod
>> import errno, os, shutil
>> @@ -366,13 +366,20 @@
>> 
>>             # Recomputing branch cache might be slow on big repos,
>>             # so just copy it
>> +            def copybranchcache(fname):
>> +                srcbranchcache = srcrepo.join('cache/%s' % fname)
>> +                dstbranchcache = os.path.join(dstcachedir, fname)
>> +                if os.path.exists(srcbranchcache):
>> +                    if not os.path.exists(dstcachedir):
>> +                        os.mkdir(dstcachedir)
>> +                    util.copyfile(srcbranchcache, dstbranchcache)
>> +
>>             dstcachedir = os.path.join(destpath, 'cache')
>> -            srcbranchcache = srcrepo.join('cache/branch2')
>> -            dstbranchcache = os.path.join(dstcachedir, 'branch2')
>> -            if os.path.exists(srcbranchcache):
>> -                if not os.path.exists(dstcachedir):
>> -                    os.mkdir(dstcachedir)
>> -                util.copyfile(srcbranchcache, dstbranchcache)
>> +            # In local clones we're copying all nodes, not just served
>> +            # ones. Therefore copy all branchcaches over.
>> +            copybranchcache('branch2')
>> +            for cachename in repoview.filtertable:
>> +                copybranchcache('branch2-%s' % cachename)
>> 
>>             # we need to re-init the repo after manually copying the data
>>             # into it
>> diff --git a/tests/test-clone.t b/tests/test-clone.t
>> --- a/tests/test-clone.t
>> +++ b/tests/test-clone.t
>> @@ -25,12 +25,25 @@
>>   .hg/store/data/b.d
>>   .hg/store/data/b.i
>> 
>> +Trigger branchcache creation:
>> +
>> +  $ hg branches
>> +  default                       10:a7949464abda
>> +  $ ls .hg/cache
>> +  branch2-served
>> +
>> Default operation:
>> 
>>   $ hg clone . ../b
>>   updating to branch default
>>   2 files updated, 0 files merged, 0 files removed, 0 files unresolved
>>   $ cd ../b
>> +
>> +Ensure branchcache got copied over:
>> +
>> +  $ ls .hg/cache
>> +  branch2-served
>> +
>>   $ cat a
>>   a
>>   $ hg verify
>> @@ -58,6 +71,12 @@
>>   listing keys for "bookmarks"
>> #endif
>>   $ cd ../c
>> +
>> +Ensure branchcache got copied over:
>> +
>> +  $ ls .hg/cache
>> +  branch2-served
>> +
>>   $ cat a 2>/dev/null || echo "a not present"
>>   a not present
>>   $ hg verify
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@selenic.com
>> http://selenic.com/mailman/listinfo/mercurial-devel
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Pierre-Yves David - Aug. 23, 2014, 8:54 p.m.
On 08/21/2014 04:25 PM, Siddharth Agarwal wrote:
> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1408662329 25200
> #      Thu Aug 21 16:05:29 2014 -0700
> # Node ID cfc686b2b6d22888aeb8621c3e388005001f4caa
> # Parent  d5c551bf9d58912c8c084b5abce76b606c870eb6
> clone: for local clones, copy over filtered branchcaches as well (issue4286)
>
> Local clones copy/hardlink over files directly, so the branchcaches should all
> be valid. There's a slight chance that a read operation would update one of the
> branchcaches, but we hold a lock over the source repo so they shouldn't cause
> an invalid branchcache to be copied over, just a different, valid one.

Copying an invalid branch cache (instead of non cache) is not a huge 
deal as the cache would be detected as invalid and recomputed.

Patch

diff --git a/mercurial/hg.py b/mercurial/hg.py
--- a/mercurial/hg.py
+++ b/mercurial/hg.py
@@ -11,7 +11,7 @@ 
 from node import hex, nullid
 import localrepo, bundlerepo, unionrepo, httppeer, sshpeer, statichttprepo
 import bookmarks, lock, util, extensions, error, node, scmutil, phases, url
-import cmdutil, discovery
+import cmdutil, discovery, repoview
 import merge as mergemod
 import verify as verifymod
 import errno, os, shutil
@@ -366,13 +366,20 @@ 
 
             # Recomputing branch cache might be slow on big repos,
             # so just copy it
+            def copybranchcache(fname):
+                srcbranchcache = srcrepo.join('cache/%s' % fname)
+                dstbranchcache = os.path.join(dstcachedir, fname)
+                if os.path.exists(srcbranchcache):
+                    if not os.path.exists(dstcachedir):
+                        os.mkdir(dstcachedir)
+                    util.copyfile(srcbranchcache, dstbranchcache)
+
             dstcachedir = os.path.join(destpath, 'cache')
-            srcbranchcache = srcrepo.join('cache/branch2')
-            dstbranchcache = os.path.join(dstcachedir, 'branch2')
-            if os.path.exists(srcbranchcache):
-                if not os.path.exists(dstcachedir):
-                    os.mkdir(dstcachedir)
-                util.copyfile(srcbranchcache, dstbranchcache)
+            # In local clones we're copying all nodes, not just served
+            # ones. Therefore copy all branchcaches over.
+            copybranchcache('branch2')
+            for cachename in repoview.filtertable:
+                copybranchcache('branch2-%s' % cachename)
 
             # we need to re-init the repo after manually copying the data
             # into it
diff --git a/tests/test-clone.t b/tests/test-clone.t
--- a/tests/test-clone.t
+++ b/tests/test-clone.t
@@ -25,12 +25,25 @@ 
   .hg/store/data/b.d
   .hg/store/data/b.i
 
+Trigger branchcache creation:
+
+  $ hg branches
+  default                       10:a7949464abda
+  $ ls .hg/cache
+  branch2-served
+
 Default operation:
 
   $ hg clone . ../b
   updating to branch default
   2 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ cd ../b
+
+Ensure branchcache got copied over:
+
+  $ ls .hg/cache
+  branch2-served
+
   $ cat a
   a
   $ hg verify
@@ -58,6 +71,12 @@ 
   listing keys for "bookmarks"
 #endif
   $ cd ../c
+
+Ensure branchcache got copied over:
+
+  $ ls .hg/cache
+  branch2-served
+
   $ cat a 2>/dev/null || echo "a not present"
   a not present
   $ hg verify