Patchwork [3,of,3,various] branchcache: stay silent if failing to read cache files

login
register
mail settings
Submitter Mads Kiilerich
Date April 13, 2015, 8:26 p.m.
Message ID <7870d376ee8f4c15b412.1428956761@localhost.localdomain>
Download mbox | patch
Permalink /patch/8643/
State Accepted
Headers show

Comments

Mads Kiilerich - April 13, 2015, 8:26 p.m.
# HG changeset patch
# User Mads Kiilerich <madski@unity3d.com>
# Date 1421194526 -3600
#      Wed Jan 14 01:15:26 2015 +0100
# Node ID 7870d376ee8f4c15b412912600a4e295f1b3fb9b
# Parent  31a330fcad90837e0c0d858db69e67a7f74ec997
branchcache: stay silent if failing to read cache files

The warning has in some cases incorrectly attributed unrelated problems to rbc.

Instead, just do like the branch head cache does and stay quiet when reading
fails. The cache will be missing the first time a repo is used. It is a normal
situation and there is no reason to make a note of that.
Ryan McElroy - April 14, 2015, 3:51 a.m.
On 4/13/2015 1:26 PM, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1421194526 -3600
> #      Wed Jan 14 01:15:26 2015 +0100
> # Node ID 7870d376ee8f4c15b412912600a4e295f1b3fb9b
> # Parent  31a330fcad90837e0c0d858db69e67a7f74ec997
> branchcache: stay silent if failing to read cache files
>
> The warning has in some cases incorrectly attributed unrelated problems to rbc.
>
> Instead, just do like the branch head cache does and stay quiet when reading
> fails. The cache will be missing the first time a repo is used. It is a normal
> situation and there is no reason to make a note of that.
>
> diff --git a/mercurial/branchmap.py b/mercurial/branchmap.py
> --- a/mercurial/branchmap.py
> +++ b/mercurial/branchmap.py
> @@ -335,8 +335,6 @@ class revbranchcache(object):
>               self._rbcsnameslen = len(bndata) # for verification before writing
>               self._names = [encoding.tolocal(bn) for bn in bndata.split('\0')]
>           except (IOError, OSError), inst:
> -            repo.ui.debug("couldn't read revision branch cache names: %s\n" %
> -                          inst)
>               if readonly:
>                   # don't try to use cache - fall back to the slow path
>                   self.branchinfo = self._branchinfo
> diff --git a/tests/test-casefolding.t b/tests/test-casefolding.t
> --- a/tests/test-casefolding.t
> +++ b/tests/test-casefolding.t
> @@ -28,7 +28,6 @@ test case collision on rename (issue750)
>     a
>     committing manifest
>     committing changelog
> -  couldn't read revision branch cache names: * (glob)
>     committed changeset 0:07f4944404050f47db2e5c5071e0e84e7a27bba9
>   
>   Case-changing renames should work:
> diff --git a/tests/test-convert-svn-encoding.t b/tests/test-convert-svn-encoding.t
> --- a/tests/test-convert-svn-encoding.t
> +++ b/tests/test-convert-svn-encoding.t
> @@ -53,7 +53,6 @@ Convert while testing all possible outpu
>     source: svn:afeb9c47-92ff-4c0c-9f72-e1f6eb8ac9af/trunk@1
>     converting: 0/6 revisions (0.00%)
>     committing changelog
> -  couldn't read revision branch cache names: * (glob)
>     4 hello
>     source: svn:afeb9c47-92ff-4c0c-9f72-e1f6eb8ac9af/trunk@2
>     converting: 1/6 revisions (16.67%)
>
I'm not sure I'm onboard with this. You expect --debug to be noisy; this 
would inform you why something might be slow if you see if in 
consecutive invocations. Unless... do we mention in debug mode if we 
can't *write* a cache file? If so, then I'm okay with this change. I 
just want to make sure we don't remove a clue when someone is trying to 
figure out why an operation is slow in a read-only FS.
Mads Kiilerich - April 14, 2015, 1:20 p.m.
On 04/13/2015 11:51 PM, Ryan McElroy wrote:
> On 4/13/2015 1:26 PM, Mads Kiilerich wrote:
>> # HG changeset patch
>> # User Mads Kiilerich <madski@unity3d.com>
>> # Date 1421194526 -3600
>> #      Wed Jan 14 01:15:26 2015 +0100
>> # Node ID 7870d376ee8f4c15b412912600a4e295f1b3fb9b
>> # Parent  31a330fcad90837e0c0d858db69e67a7f74ec997
>> branchcache: stay silent if failing to read cache files
>>
>> The warning has in some cases incorrectly attributed unrelated 
>> problems to rbc.
>>
>> Instead, just do like the branch head cache does and stay quiet when 
>> reading
>> fails. The cache will be missing the first time a repo is used. It is 
>> a normal
>> situation and there is no reason to make a note of that.
>>
>> diff --git a/mercurial/branchmap.py b/mercurial/branchmap.py
>> --- a/mercurial/branchmap.py
>> +++ b/mercurial/branchmap.py
>> @@ -335,8 +335,6 @@ class revbranchcache(object):
>>               self._rbcsnameslen = len(bndata) # for verification 
>> before writing
>>               self._names = [encoding.tolocal(bn) for bn in 
>> bndata.split('\0')]
>>           except (IOError, OSError), inst:
>> -            repo.ui.debug("couldn't read revision branch cache 
>> names: %s\n" %
>> -                          inst)
>>               if readonly:
>>                   # don't try to use cache - fall back to the slow path
>>                   self.branchinfo = self._branchinfo
>> diff --git a/tests/test-casefolding.t b/tests/test-casefolding.t
>> --- a/tests/test-casefolding.t
>> +++ b/tests/test-casefolding.t
>> @@ -28,7 +28,6 @@ test case collision on rename (issue750)
>>     a
>>     committing manifest
>>     committing changelog
>> -  couldn't read revision branch cache names: * (glob)
>>     committed changeset 0:07f4944404050f47db2e5c5071e0e84e7a27bba9
>>     Case-changing renames should work:
>> diff --git a/tests/test-convert-svn-encoding.t 
>> b/tests/test-convert-svn-encoding.t
>> --- a/tests/test-convert-svn-encoding.t
>> +++ b/tests/test-convert-svn-encoding.t
>> @@ -53,7 +53,6 @@ Convert while testing all possible outpu
>>     source: svn:afeb9c47-92ff-4c0c-9f72-e1f6eb8ac9af/trunk@1
>>     converting: 0/6 revisions (0.00%)
>>     committing changelog
>> -  couldn't read revision branch cache names: * (glob)
>>     4 hello
>>     source: svn:afeb9c47-92ff-4c0c-9f72-e1f6eb8ac9af/trunk@2
>>     converting: 1/6 revisions (16.67%)
>>
> I'm not sure I'm onboard with this. You expect --debug to be noisy; 
> this would inform you why something might be slow if you see if in 
> consecutive invocations. 

Yes, --debug will be noisy but it should only emit helpful noise and not 
be more noisy than necessary. Emitting a debug message in this case was 
kind of an arbitrary choice and now I have a slight preference for not 
emitting it.

> Unless... do we mention in debug mode if we can't *write* a cache 
> file? If so, then I'm okay with this change. I just want to make sure 
> we don't remove a clue when someone is trying to figure out why an 
> operation is slow in a read-only FS.

Yes, writing will still emit a debug message.

/Mads
Pierre-Yves David - April 14, 2015, 8:50 p.m.
On 04/14/2015 09:20 AM, Mads Kiilerich wrote:
> On 04/13/2015 11:51 PM, Ryan McElroy wrote:
>> On 4/13/2015 1:26 PM, Mads Kiilerich wrote:
>>> # HG changeset patch
>>> # User Mads Kiilerich <madski@unity3d.com>
>>> # Date 1421194526 -3600
>>> #      Wed Jan 14 01:15:26 2015 +0100
>>> # Node ID 7870d376ee8f4c15b412912600a4e295f1b3fb9b
>>> # Parent  31a330fcad90837e0c0d858db69e67a7f74ec997

This one is pushed to the clowncopter, Thanks (expecting a V2 for patch 2)

Patch

diff --git a/mercurial/branchmap.py b/mercurial/branchmap.py
--- a/mercurial/branchmap.py
+++ b/mercurial/branchmap.py
@@ -335,8 +335,6 @@  class revbranchcache(object):
             self._rbcsnameslen = len(bndata) # for verification before writing
             self._names = [encoding.tolocal(bn) for bn in bndata.split('\0')]
         except (IOError, OSError), inst:
-            repo.ui.debug("couldn't read revision branch cache names: %s\n" %
-                          inst)
             if readonly:
                 # don't try to use cache - fall back to the slow path
                 self.branchinfo = self._branchinfo
diff --git a/tests/test-casefolding.t b/tests/test-casefolding.t
--- a/tests/test-casefolding.t
+++ b/tests/test-casefolding.t
@@ -28,7 +28,6 @@  test case collision on rename (issue750)
   a
   committing manifest
   committing changelog
-  couldn't read revision branch cache names: * (glob)
   committed changeset 0:07f4944404050f47db2e5c5071e0e84e7a27bba9
 
 Case-changing renames should work:
diff --git a/tests/test-convert-svn-encoding.t b/tests/test-convert-svn-encoding.t
--- a/tests/test-convert-svn-encoding.t
+++ b/tests/test-convert-svn-encoding.t
@@ -53,7 +53,6 @@  Convert while testing all possible outpu
   source: svn:afeb9c47-92ff-4c0c-9f72-e1f6eb8ac9af/trunk@1
   converting: 0/6 revisions (0.00%)
   committing changelog
-  couldn't read revision branch cache names: * (glob)
   4 hello
   source: svn:afeb9c47-92ff-4c0c-9f72-e1f6eb8ac9af/trunk@2
   converting: 1/6 revisions (16.67%)