[PATCH 1 of 2] tests: update lockdelay.py to handle the `wait` argument

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 1 of 2] tests: update lockdelay.py to handle the `wait` argument

Pulkit Goyal
# HG changeset patch
# User Pulkit Goyal <[hidden email]>
# Date 1599736900 -19800
#      Thu Sep 10 16:51:40 2020 +0530
# Node ID 3095d36efcaf9ce835249c9312e892b0d54aac78
# Parent  64de86fd0984ff1e2306de52849be7ebf7dcfd25
# EXP-Topic tags-fix
tests: update lockdelay.py to handle the `wait` argument

Spotted by a future change.

diff --git a/tests/lockdelay.py b/tests/lockdelay.py
--- a/tests/lockdelay.py
+++ b/tests/lockdelay.py
@@ -10,11 +10,11 @@ import time
 
 def reposetup(ui, repo):
     class delayedlockrepo(repo.__class__):
-        def lock(self):
+        def lock(self, wait=True):
             delay = float(os.environ.get('HGPRELOCKDELAY', '0.0'))
             if delay:
                 time.sleep(delay)
-            res = super(delayedlockrepo, self).lock()
+            res = super(delayedlockrepo, self).lock(wait=wait)
             delay = float(os.environ.get('HGPOSTLOCKDELAY', '0.0'))
             if delay:
                 time.sleep(delay)
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH 2 of 2] tags: take lock while writing `tags2` cache

Pulkit Goyal
# HG changeset patch
# User Pulkit Goyal <[hidden email]>
# Date 1599727895 -19800
#      Thu Sep 10 14:21:35 2020 +0530
# Node ID 3eae3d3a1b44b0fe10e00766e1113c50d48ed086
# Parent  3095d36efcaf9ce835249c9312e892b0d54aac78
# EXP-Topic tags-fix
tags: take lock while writing `tags2` cache

Before this patch, we were not taking lock while writing the cache. This cache
is shared one and hence multiple writes can happen at same time. So let's take a
lock before writing it. We don't wait for lock because outdated cache is still
fine.

diff --git a/mercurial/tags.py b/mercurial/tags.py
--- a/mercurial/tags.py
+++ b/mercurial/tags.py
@@ -514,38 +514,47 @@ def _getfnodes(ui, repo, nodes):
 
 def _writetagcache(ui, repo, valid, cachetags):
     filename = _filename(repo)
+
     try:
-        cachefile = repo.cachevfs(filename, b'w', atomictemp=True)
-    except (OSError, IOError):
+        lock = repo.lock(wait=False)
+    except error.LockError:
+        repo.ui.log(
+            b'tagscache',
+            b'not writing .hg/cache/%s because lock cannot be acquired\n'
+            % filename,
+        )
         return
 
-    ui.log(
-        b'tagscache',
-        b'writing .hg/cache/%s with %d tags\n',
-        filename,
-        len(cachetags),
-    )
-
-    if valid[2]:
-        cachefile.write(
-            b'%d %s %s\n' % (valid[0], hex(valid[1]), hex(valid[2]))
+    try:
+        cachefile = repo.cachevfs(filename, b'w', atomictemp=True)
+        ui.log(
+            b'tagscache',
+            b'writing .hg/cache/%s with %d tags\n',
+            filename,
+            len(cachetags),
         )
-    else:
-        cachefile.write(b'%d %s\n' % (valid[0], hex(valid[1])))
+
+        if valid[2]:
+            cachefile.write(
+                b'%d %s %s\n' % (valid[0], hex(valid[1]), hex(valid[2]))
+            )
+        else:
+            cachefile.write(b'%d %s\n' % (valid[0], hex(valid[1])))
 
-    # Tag names in the cache are in UTF-8 -- which is the whole reason
-    # we keep them in UTF-8 throughout this module.  If we converted
-    # them local encoding on input, we would lose info writing them to
-    # the cache.
-    for (name, (node, hist)) in sorted(pycompat.iteritems(cachetags)):
-        for n in hist:
-            cachefile.write(b"%s %s\n" % (hex(n), name))
-        cachefile.write(b"%s %s\n" % (hex(node), name))
+        # Tag names in the cache are in UTF-8 -- which is the whole reason
+        # we keep them in UTF-8 throughout this module.  If we converted
+        # them local encoding on input, we would lose info writing them to
+        # the cache.
+        for (name, (node, hist)) in sorted(pycompat.iteritems(cachetags)):
+            for n in hist:
+                cachefile.write(b"%s %s\n" % (hex(n), name))
+            cachefile.write(b"%s %s\n" % (hex(node), name))
 
-    try:
         cachefile.close()
     except (OSError, IOError):
         pass
+    finally:
+        lock.release()
 
 
 def tag(repo, names, node, message, local, user, date, editor=False):
diff --git a/tests/test-tags.t b/tests/test-tags.t
--- a/tests/test-tags.t
+++ b/tests/test-tags.t
@@ -163,7 +163,7 @@ Failure to acquire lock results in no wr
   1970/01/01 00:00:00 bob @b9154636be938d3d431e75a7c906504a079bfe07 (5000)> identify
   1970/01/01 00:00:00 bob @b9154636be938d3d431e75a7c906504a079bfe07 (5000)> not writing .hg/cache/hgtagsfnodes1 because lock cannot be acquired
   1970/01/01 00:00:00 bob @b9154636be938d3d431e75a7c906504a079bfe07 (5000)> 0/2 cache hits/lookups in * seconds (glob)
-  1970/01/01 00:00:00 bob @b9154636be938d3d431e75a7c906504a079bfe07 (5000)> writing .hg/cache/tags2-visible with 1 tags
+  1970/01/01 00:00:00 bob @b9154636be938d3d431e75a7c906504a079bfe07 (5000)> not writing .hg/cache/tags2-visible because lock cannot be acquired
   1970/01/01 00:00:00 bob @b9154636be938d3d431e75a7c906504a079bfe07 (5000)> identify exited 0 after * seconds (glob)
   1970/01/01 00:00:00 bob @b9154636be938d3d431e75a7c906504a079bfe07 (5000)> blackbox -l 6
 
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1 of 2] tests: update lockdelay.py to handle the `wait` argument

Yuya Nishihara
In reply to this post by Pulkit Goyal
On Fri, 11 Sep 2020 12:34:00 +0530, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <[hidden email]>
> # Date 1599736900 -19800
> #      Thu Sep 10 16:51:40 2020 +0530
> # Node ID 3095d36efcaf9ce835249c9312e892b0d54aac78
> # Parent  64de86fd0984ff1e2306de52849be7ebf7dcfd25
> # EXP-Topic tags-fix
> tests: update lockdelay.py to handle the `wait` argument

Queued this, thanks.
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2 of 2] tags: take lock while writing `tags2` cache

Yuya Nishihara
In reply to this post by Pulkit Goyal
On Fri, 11 Sep 2020 12:34:01 +0530, Pulkit Goyal wrote:

> # HG changeset patch
> # User Pulkit Goyal <[hidden email]>
> # Date 1599727895 -19800
> #      Thu Sep 10 14:21:35 2020 +0530
> # Node ID 3eae3d3a1b44b0fe10e00766e1113c50d48ed086
> # Parent  3095d36efcaf9ce835249c9312e892b0d54aac78
> # EXP-Topic tags-fix
> tags: take lock while writing `tags2` cache
>
> Before this patch, we were not taking lock while writing the cache. This cache
> is shared one and hence multiple writes can happen at same time. So let's take a
> lock before writing it. We don't wait for lock because outdated cache is still
> fine.
>
> diff --git a/mercurial/tags.py b/mercurial/tags.py
> --- a/mercurial/tags.py
> +++ b/mercurial/tags.py
> @@ -514,38 +514,47 @@ def _getfnodes(ui, repo, nodes):
>  
>  def _writetagcache(ui, repo, valid, cachetags):
>      filename = _filename(repo)
> +
>      try:
> -        cachefile = repo.cachevfs(filename, b'w', atomictemp=True)

Write race should be dealt with atomictemp. This wouldn't need a lock.
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2 of 2] tags: take lock while writing `tags2` cache

Pulkit Goyal
On Sat, Sep 12, 2020 at 8:31 AM Yuya Nishihara <[hidden email]> wrote:

>
> On Fri, 11 Sep 2020 12:34:01 +0530, Pulkit Goyal wrote:
> > # HG changeset patch
> > # User Pulkit Goyal <[hidden email]>
> > # Date 1599727895 -19800
> > #      Thu Sep 10 14:21:35 2020 +0530
> > # Node ID 3eae3d3a1b44b0fe10e00766e1113c50d48ed086
> > # Parent  3095d36efcaf9ce835249c9312e892b0d54aac78
> > # EXP-Topic tags-fix
> > tags: take lock while writing `tags2` cache
> >
> > Before this patch, we were not taking lock while writing the cache. This cache
> > is shared one and hence multiple writes can happen at same time. So let's take a
> > lock before writing it. We don't wait for lock because outdated cache is still
> > fine.
> >
> > diff --git a/mercurial/tags.py b/mercurial/tags.py
> > --- a/mercurial/tags.py
> > +++ b/mercurial/tags.py
> > @@ -514,38 +514,47 @@ def _getfnodes(ui, repo, nodes):
> >
> >  def _writetagcache(ui, repo, valid, cachetags):
> >      filename = _filename(repo)
> > +
> >      try:
> > -        cachefile = repo.cachevfs(filename, b'w', atomictemp=True)
>
> Write race should be dealt with atomictemp. This wouldn't need a lock.

Missed it thanks.
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel