Patchwork [1,of,4,STABLE] dirstate: delay writing out to ensure timestamp of each entries explicitly

login
register
mail settings
Submitter Katsunori FUJIWARA
Date July 22, 2014, 3:15 p.m.
Message ID <89b809fa6cefb56d10ac.1406042124@feefifofum>
Download mbox | patch
Permalink /patch/5190/
State Accepted
Commit 89b809fa6cefb56d10ac0ce68dfdddadc896af20
Headers show

Comments

Katsunori FUJIWARA - July 22, 2014, 3:15 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1406041170 -32400
#      Tue Jul 22 23:59:30 2014 +0900
# Branch stable
# Node ID 89b809fa6cefb56d10ac0ce68dfdddadc896af20
# Parent  a5168eb9b2bc488f42809872d22321a9825bc738
dirstate: delay writing out to ensure timestamp of each entries explicitly

Even though "dirstate.write()" is invoked explicitly after "normal"
invocations, timestamp field of entries may be still "unset" in the
"dirstate" file itself , because "pack_dirstate" drops it when it is
equal to the timestamp of "dirstate" file itself.

This can avoid overlooking modification of files, which are updated at
same time in the second. But on the other hand, this may hide timing
critical problems.

For example, incorrect "normal"-ing (or lack of "normallookup"-ing on
the already "normal"-ed entry) is visible only when:

  - the target file is modified in the working directory at T1, and
  - "dirstate" file is written out at T2 (!= T1)

Otherwise, T1 is dropped by "pack_dirstate" in "dirstate.write()"
invocation, and "unset" is stored into "dirstate" file.

It often fails to reproduce problems from incorrect "normal"-ing by
Mercurial testset, because automated actions in the small repository
almost always causes that T1 and T2 are same.

This patch adds the debug feature to delay writing out to ensure
timestamp of each entries explicitly.

This feature is used to make timing critical "dirstate" problems
reproducable in subsequent patches.
Katsunori FUJIWARA - July 22, 2014, 3:21 p.m.
At Wed, 23 Jul 2014 00:15:24 +0900,
FUJIWARA Katsunori wrote:
> 
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1406041170 -32400
> #      Tue Jul 22 23:59:30 2014 +0900
> # Branch stable
> # Node ID 89b809fa6cefb56d10ac0ce68dfdddadc896af20
> # Parent  a5168eb9b2bc488f42809872d22321a9825bc738
> dirstate: delay writing out to ensure timestamp of each entries explicitly

This series can fix timing critical issues like below:

    http://www.selenic.com/pipermail/mercurial-devel/2013-May/051225.html

> Even though "dirstate.write()" is invoked explicitly after "normal"
> invocations, timestamp field of entries may be still "unset" in the
> "dirstate" file itself , because "pack_dirstate" drops it when it is
> equal to the timestamp of "dirstate" file itself.
> 
> This can avoid overlooking modification of files, which are updated at
> same time in the second. But on the other hand, this may hide timing
> critical problems.
> 
> For example, incorrect "normal"-ing (or lack of "normallookup"-ing on
> the already "normal"-ed entry) is visible only when:
> 
>   - the target file is modified in the working directory at T1, and
>   - "dirstate" file is written out at T2 (!= T1)
> 
> Otherwise, T1 is dropped by "pack_dirstate" in "dirstate.write()"
> invocation, and "unset" is stored into "dirstate" file.
> 
> It often fails to reproduce problems from incorrect "normal"-ing by
> Mercurial testset, because automated actions in the small repository
> almost always causes that T1 and T2 are same.
> 
> This patch adds the debug feature to delay writing out to ensure
> timestamp of each entries explicitly.
> 
> This feature is used to make timing critical "dirstate" problems
> reproducable in subsequent patches.
> 
> diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
> --- a/mercurial/dirstate.py
> +++ b/mercurial/dirstate.py
> @@ -504,6 +504,14 @@
>      def write(self):
>          if not self._dirty:
>              return
> +
> +        # enough 'delaywrite' prevents 'pack_dirstate' from dropping
> +        # timestamp of each entries in dirstate, because of 'now > mtime'
> +        delaywrite = self._ui.configint('debug', 'dirstate.delaywrite', 0)
> +        if delaywrite:
> +            import time # to avoid useless import
> +            time.sleep(delaywrite)
> +
>          st = self._opener("dirstate", "w", atomictemp=True)
>          # use the modification time of the newly created temporary file as the
>          # filesystem's notion of 'now'
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
> 

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
Matt Mackall - July 22, 2014, 6:28 p.m.
On Wed, 2014-07-23 at 00:21 +0900, FUJIWARA Katsunori wrote:
> At Wed, 23 Jul 2014 00:15:24 +0900,
> FUJIWARA Katsunori wrote:
> > 
> > # HG changeset patch
> > # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> > # Date 1406041170 -32400
> > #      Tue Jul 22 23:59:30 2014 +0900
> > # Branch stable
> > # Node ID 89b809fa6cefb56d10ac0ce68dfdddadc896af20
> > # Parent  a5168eb9b2bc488f42809872d22321a9825bc738
> > dirstate: delay writing out to ensure timestamp of each entries explicitly
> 
> This series can fix timing critical issues like below:
> 
>     http://www.selenic.com/pipermail/mercurial-devel/2013-May/051225.html

These are queued for stable, thanks. But I'll probably reject the next
set of patches you send with this sort of 0-of-N ban workaround. Put
this sort of thing _in the commit description for future reference_ and
_keep my inbox tidy_.

Patch

diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -504,6 +504,14 @@ 
     def write(self):
         if not self._dirty:
             return
+
+        # enough 'delaywrite' prevents 'pack_dirstate' from dropping
+        # timestamp of each entries in dirstate, because of 'now > mtime'
+        delaywrite = self._ui.configint('debug', 'dirstate.delaywrite', 0)
+        if delaywrite:
+            import time # to avoid useless import
+            time.sleep(delaywrite)
+
         st = self._opener("dirstate", "w", atomictemp=True)
         # use the modification time of the newly created temporary file as the
         # filesystem's notion of 'now'