Patchwork [1,of,2,sqldirstate] sqldirstate: specify checkambig=True to avoid file stat ambiguity

login
register
mail settings
Submitter Katsunori FUJIWARA
Date Oct. 14, 2016, 7:10 p.m.
Message ID <72486917916f6d47525f.1476472252@feefifofum>
Download mbox | patch
Permalink /patch/17089/
State Accepted
Headers show

Comments

Katsunori FUJIWARA - Oct. 14, 2016, 7:10 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1476456912 -32400
#      Fri Oct 14 23:55:12 2016 +0900
# Node ID 72486917916f6d47525f721cd5b27012091122ed
# Parent  d2c3a2c02eb6c7e5a7331ba0cf15e5bf7c8dc8dc
# Available At https://foozy@bitbucket.org/foozy/hg-experimental
#              hg pull https://foozy@bitbucket.org/foozy/hg-experimental -r 72486917916f
sqldirstate: specify checkambig=True to avoid file stat ambiguity

With sqldirstate extension, size of .hg/dirstate is always fixed: node
ID x 2 + dummy fixed text.

This increases occurrence of file stat ambiguity, and
processes/threads running parallelly might overlook change of
dirstate, because reloading @filecache-ed property repo.dirstate is
avoided in such case. See wiki page below for detail about file stat
ambiguity.

    https://www.mercurial-scm.org/wiki/ExactCacheValidationPlan

This patch specifies checkambig=True at opening .hg/dirstate file for
writing, to avoid file stat ambiguity.

Strictly speaking, to enable sqldirstate with Mercurial earlier than
upcoming 4.0 (or 76f1ea360c7e), it should be examined whether
checkambig optional argument is available for opener or not, because
it has been available since 4.0 (or 76f1ea360c7e).

But current sqldirstate implementation depends on 2ebd507e5ac3 of
Mercurial, which has introduced dirstate._origpl after 76f1ea360c7e.

Therefor, changes in this patch are safe enough, unless removing
dependence on dirstate._origpl from sqldirstate.
Pierre-Yves David - Oct. 14, 2016, 7:31 p.m.
Could we make the flag for this 'sqldirstate-ext' to make it clearer 
that it has an external target?

On 10/14/2016 09:10 PM, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1476456912 -32400
> #      Fri Oct 14 23:55:12 2016 +0900
> # Node ID 72486917916f6d47525f721cd5b27012091122ed
> # Parent  d2c3a2c02eb6c7e5a7331ba0cf15e5bf7c8dc8dc
> # Available At https://foozy@bitbucket.org/foozy/hg-experimental
> #              hg pull https://foozy@bitbucket.org/foozy/hg-experimental -r 72486917916f
> sqldirstate: specify checkambig=True to avoid file stat ambiguity
>
> With sqldirstate extension, size of .hg/dirstate is always fixed: node
> ID x 2 + dummy fixed text.
>
> This increases occurrence of file stat ambiguity, and
> processes/threads running parallelly might overlook change of
> dirstate, because reloading @filecache-ed property repo.dirstate is
> avoided in such case. See wiki page below for detail about file stat
> ambiguity.
>
>     https://www.mercurial-scm.org/wiki/ExactCacheValidationPlan
>
> This patch specifies checkambig=True at opening .hg/dirstate file for
> writing, to avoid file stat ambiguity.
>
> Strictly speaking, to enable sqldirstate with Mercurial earlier than
> upcoming 4.0 (or 76f1ea360c7e), it should be examined whether
> checkambig optional argument is available for opener or not, because
> it has been available since 4.0 (or 76f1ea360c7e).
>
> But current sqldirstate implementation depends on 2ebd507e5ac3 of
> Mercurial, which has introduced dirstate._origpl after 76f1ea360c7e.
>
> Therefor, changes in this patch are safe enough, unless removing
> dependence on dirstate._origpl from sqldirstate.
>
> diff --git a/sqldirstate/sqldirstate.py b/sqldirstate/sqldirstate.py
> --- a/sqldirstate/sqldirstate.py
> +++ b/sqldirstate/sqldirstate.py
> @@ -563,7 +563,7 @@ def makedirstate(cls):
>
>
>  def writefakedirstate(dirstate):
> -    st = dirstate._opener(FAKEDIRSTATE, "w", atomictemp=True)
> +    st = dirstate._opener(FAKEDIRSTATE, "w", atomictemp=True, checkambig=True)
>      st.write("".join(dirstate._pl))
>      st.write("\nThis is fake dirstate put here by the sqldirsate.")
>      st.write("\nIt contains only working copy parents info.")
> @@ -629,7 +629,7 @@ def toflat(sqldirstate):
>      it's not touching anything but the dirstate file
>      """
>      # converts a sqldirstate to a flat one
> -    st = sqldirstate._opener("dirstate", "w", atomictemp=True)
> +    st = sqldirstate._opener("dirstate", "w", atomictemp=True, checkambig=True)
>      newmap = {}
>      for k, v in sqldirstate._map.iteritems():
>          newmap[k] = v
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Katsunori FUJIWARA - Oct. 14, 2016, 7:46 p.m.
At Fri, 14 Oct 2016 21:31:36 +0200,
Pierre-Yves David wrote:
> 
> Could we make the flag for this 'sqldirstate-ext' to make it clearer 
> that it has an external target?

Oops, sorry for lack of "-ext"

> On 10/14/2016 09:10 PM, FUJIWARA Katsunori wrote:
> > # HG changeset patch
> > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> > # Date 1476456912 -32400
> > #      Fri Oct 14 23:55:12 2016 +0900
> > # Node ID 72486917916f6d47525f721cd5b27012091122ed
> > # Parent  d2c3a2c02eb6c7e5a7331ba0cf15e5bf7c8dc8dc
> > # Available At https://foozy@bitbucket.org/foozy/hg-experimental
> > #              hg pull https://foozy@bitbucket.org/foozy/hg-experimental -r 72486917916f
> > sqldirstate: specify checkambig=True to avoid file stat ambiguity
> >
> > With sqldirstate extension, size of .hg/dirstate is always fixed: node
> > ID x 2 + dummy fixed text.
> >
> > This increases occurrence of file stat ambiguity, and
> > processes/threads running parallelly might overlook change of
> > dirstate, because reloading @filecache-ed property repo.dirstate is
> > avoided in such case. See wiki page below for detail about file stat
> > ambiguity.
> >
> >     https://www.mercurial-scm.org/wiki/ExactCacheValidationPlan
> >
> > This patch specifies checkambig=True at opening .hg/dirstate file for
> > writing, to avoid file stat ambiguity.
> >
> > Strictly speaking, to enable sqldirstate with Mercurial earlier than
> > upcoming 4.0 (or 76f1ea360c7e), it should be examined whether
> > checkambig optional argument is available for opener or not, because
> > it has been available since 4.0 (or 76f1ea360c7e).
> >
> > But current sqldirstate implementation depends on 2ebd507e5ac3 of
> > Mercurial, which has introduced dirstate._origpl after 76f1ea360c7e.
> >
> > Therefor, changes in this patch are safe enough, unless removing
> > dependence on dirstate._origpl from sqldirstate.
> >
> > diff --git a/sqldirstate/sqldirstate.py b/sqldirstate/sqldirstate.py
> > --- a/sqldirstate/sqldirstate.py
> > +++ b/sqldirstate/sqldirstate.py
> > @@ -563,7 +563,7 @@ def makedirstate(cls):
> >
> >
> >  def writefakedirstate(dirstate):
> > -    st = dirstate._opener(FAKEDIRSTATE, "w", atomictemp=True)
> > +    st = dirstate._opener(FAKEDIRSTATE, "w", atomictemp=True, checkambig=True)
> >      st.write("".join(dirstate._pl))
> >      st.write("\nThis is fake dirstate put here by the sqldirsate.")
> >      st.write("\nIt contains only working copy parents info.")
> > @@ -629,7 +629,7 @@ def toflat(sqldirstate):
> >      it's not touching anything but the dirstate file
> >      """
> >      # converts a sqldirstate to a flat one
> > -    st = sqldirstate._opener("dirstate", "w", atomictemp=True)
> > +    st = sqldirstate._opener("dirstate", "w", atomictemp=True, checkambig=True)
> >      newmap = {}
> >      for k, v in sqldirstate._map.iteritems():
> >          newmap[k] = v
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel@mercurial-scm.org
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> >
> 
> -- 
> Pierre-Yves David
> 

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
Mateusz Kwapich - Oct. 20, 2016, 3:44 p.m.
Applied those, thanks!

On 10/14/16, 8:46 PM, "Mercurial-devel on behalf of FUJIWARA Katsunori" <mercurial-devel-bounces@mercurial-scm.org on behalf of foozy@lares.dti.ne.jp> wrote:

    At Fri, 14 Oct 2016 21:31:36 +0200,
    Pierre-Yves David wrote:
    > 

    > Could we make the flag for this 'sqldirstate-ext' to make it clearer 

    > that it has an external target?

    
    Oops, sorry for lack of "-ext"
    
    > On 10/14/2016 09:10 PM, FUJIWARA Katsunori wrote:

    > > # HG changeset patch

    > > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>

    > > # Date 1476456912 -32400

    > > #      Fri Oct 14 23:55:12 2016 +0900

    > > # Node ID 72486917916f6d47525f721cd5b27012091122ed

    > > # Parent  d2c3a2c02eb6c7e5a7331ba0cf15e5bf7c8dc8dc

    > > # Available At https://urldefense.proofpoint.com/v2/url?u=https-3A__foozy-40bitbucket.org_foozy_hg-2Dexperimental&d=DQIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=dK7q_6fOymlfdGMBe3wUaA&m=WjVKYkXYCkXvrP_TEhBrNQPfLA8PXEsApyl5XRocisI&s=eAHXuG3WP0Ur7XTANPYcwfk9sh_Xdndj54jo6LCUPDA&e= 

    > > #              hg pull https://urldefense.proofpoint.com/v2/url?u=https-3A__foozy-40bitbucket.org_foozy_hg-2Dexperimental&d=DQIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=dK7q_6fOymlfdGMBe3wUaA&m=WjVKYkXYCkXvrP_TEhBrNQPfLA8PXEsApyl5XRocisI&s=eAHXuG3WP0Ur7XTANPYcwfk9sh_Xdndj54jo6LCUPDA&e=  -r 72486917916f

    > > sqldirstate: specify checkambig=True to avoid file stat ambiguity

    > >

    > > With sqldirstate extension, size of .hg/dirstate is always fixed: node

    > > ID x 2 + dummy fixed text.

    > >

    > > This increases occurrence of file stat ambiguity, and

    > > processes/threads running parallelly might overlook change of

    > > dirstate, because reloading @filecache-ed property repo.dirstate is

    > > avoided in such case. See wiki page below for detail about file stat

    > > ambiguity.

    > >

    > >     https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_wiki_ExactCacheValidationPlan&d=DQIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=dK7q_6fOymlfdGMBe3wUaA&m=WjVKYkXYCkXvrP_TEhBrNQPfLA8PXEsApyl5XRocisI&s=I6XLuxMnHcymVQ9HFIDzagx95_a-A4idFqnWLg3NAeY&e= 

    > >

    > > This patch specifies checkambig=True at opening .hg/dirstate file for

    > > writing, to avoid file stat ambiguity.

    > >

    > > Strictly speaking, to enable sqldirstate with Mercurial earlier than

    > > upcoming 4.0 (or 76f1ea360c7e), it should be examined whether

    > > checkambig optional argument is available for opener or not, because

    > > it has been available since 4.0 (or 76f1ea360c7e).

    > >

    > > But current sqldirstate implementation depends on 2ebd507e5ac3 of

    > > Mercurial, which has introduced dirstate._origpl after 76f1ea360c7e.

    > >

    > > Therefor, changes in this patch are safe enough, unless removing

    > > dependence on dirstate._origpl from sqldirstate.

    > >

    > > diff --git a/sqldirstate/sqldirstate.py b/sqldirstate/sqldirstate.py

    > > --- a/sqldirstate/sqldirstate.py

    > > +++ b/sqldirstate/sqldirstate.py

    > > @@ -563,7 +563,7 @@ def makedirstate(cls):

    > >

    > >

    > >  def writefakedirstate(dirstate):

    > > -    st = dirstate._opener(FAKEDIRSTATE, "w", atomictemp=True)

    > > +    st = dirstate._opener(FAKEDIRSTATE, "w", atomictemp=True, checkambig=True)

    > >      st.write("".join(dirstate._pl))

    > >      st.write("\nThis is fake dirstate put here by the sqldirsate.")

    > >      st.write("\nIt contains only working copy parents info.")

    > > @@ -629,7 +629,7 @@ def toflat(sqldirstate):

    > >      it's not touching anything but the dirstate file

    > >      """

    > >      # converts a sqldirstate to a flat one

    > > -    st = sqldirstate._opener("dirstate", "w", atomictemp=True)

    > > +    st = sqldirstate._opener("dirstate", "w", atomictemp=True, checkambig=True)

    > >      newmap = {}

    > >      for k, v in sqldirstate._map.iteritems():

    > >          newmap[k] = v

    > > _______________________________________________

    > > Mercurial-devel mailing list

    > > Mercurial-devel@mercurial-scm.org

    > > https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel&d=DQIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=dK7q_6fOymlfdGMBe3wUaA&m=WjVKYkXYCkXvrP_TEhBrNQPfLA8PXEsApyl5XRocisI&s=sNg_j-YISgLAN0WFnRMWyI-HEz2uJOUwnYZduxH0hvU&e= 

    > >

    > 

    > -- 

    > Pierre-Yves David

    > 

    
    ----------------------------------------------------------------------
    [FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
    _______________________________________________
    Mercurial-devel mailing list
    Mercurial-devel@mercurial-scm.org
    https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel&d=DQIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=dK7q_6fOymlfdGMBe3wUaA&m=WjVKYkXYCkXvrP_TEhBrNQPfLA8PXEsApyl5XRocisI&s=sNg_j-YISgLAN0WFnRMWyI-HEz2uJOUwnYZduxH0hvU&e=

Patch

diff --git a/sqldirstate/sqldirstate.py b/sqldirstate/sqldirstate.py
--- a/sqldirstate/sqldirstate.py
+++ b/sqldirstate/sqldirstate.py
@@ -563,7 +563,7 @@  def makedirstate(cls):
 
 
 def writefakedirstate(dirstate):
-    st = dirstate._opener(FAKEDIRSTATE, "w", atomictemp=True)
+    st = dirstate._opener(FAKEDIRSTATE, "w", atomictemp=True, checkambig=True)
     st.write("".join(dirstate._pl))
     st.write("\nThis is fake dirstate put here by the sqldirsate.")
     st.write("\nIt contains only working copy parents info.")
@@ -629,7 +629,7 @@  def toflat(sqldirstate):
     it's not touching anything but the dirstate file
     """
     # converts a sqldirstate to a flat one
-    st = sqldirstate._opener("dirstate", "w", atomictemp=True)
+    st = sqldirstate._opener("dirstate", "w", atomictemp=True, checkambig=True)
     newmap = {}
     for k, v in sqldirstate._map.iteritems():
         newmap[k] = v