Patchwork [20,of,22] obsstore: make markerindex support serialization

login
register
mail settings
Submitter Jun Wu
Date June 4, 2017, 11:59 p.m.
Message ID <edae716e97c441aff526.1496620772@x1c>
Download mbox | patch
Permalink /patch/21200/
State Accepted
Headers show

Comments

Jun Wu - June 4, 2017, 11:59 p.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1496615642 25200
#      Sun Jun 04 15:34:02 2017 -0700
# Node ID edae716e97c441aff52688f3be4750b34b199901
# Parent  160567b62f74e8e02fa2e9a7b2cb4076b105e528
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r edae716e97c4
obsstore: make markerindex support serialization

This allows us to load and save index state to disk.

Note this is the first user of .hg/store/cache. The indexes are purely
depending on .hg/store/obsstore and .hg/store/cache looks reasonable - if
store is shared, the indexes won't need to be duplicated.
Augie Fackler - June 5, 2017, 6:37 p.m.
On Sun, Jun 04, 2017 at 04:59:32PM -0700, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1496615642 25200
> #      Sun Jun 04 15:34:02 2017 -0700
> # Node ID edae716e97c441aff52688f3be4750b34b199901
> # Parent  160567b62f74e8e02fa2e9a7b2cb4076b105e528
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r edae716e97c4
> obsstore: make markerindex support serialization
>
> This allows us to load and save index state to disk.
>
> Note this is the first user of .hg/store/cache. The indexes are purely
> depending on .hg/store/obsstore and .hg/store/cache looks reasonable - if
> store is shared, the indexes won't need to be duplicated.

I'm kind of sort of -0 on adding another cache dir, but I don't feel
strongly enough to want a change. Hold for opinions from other
reviewers though.

>
> diff --git a/mercurial/obsolete.py b/mercurial/obsolete.py
> --- a/mercurial/obsolete.py
> +++ b/mercurial/obsolete.py
> @@ -532,9 +532,11 @@ class markerindex(radixlink.radixlink):
>      def __init__(self, obsstore, name, keyfunc):
>          """keyfunc: rawmarker -> [key]"""
> -        super(markerindex, self).__init__()
> +        path = 'cache/obsindex-%s' % name
> +        data = obsstore.svfs.tryread(path)
> +        super(markerindex, self).__init__(data)
> +        self._path = path
>          self._keyfunc = keyfunc
>          self._obsstore = weakref.proxy(obsstore)
>          self.name = name
> -        self.sourceoftruthsize = 0
>          self._readmarker = formats[obsstore._version][0]
>          self.update()
> @@ -567,4 +569,18 @@ class markerindex(radixlink.radixlink):
>              return default
>
> +    def flush(self):
> +        """write index state to disk"""

Nit: this method should be called save() or write(), not flush() - the
latter makes it sound like the file was already staged and we're just
committing the write, and we're doing a fair amount more than that.

> +        svfs = self._obsstore.svfs
> +        # fast path: no need to write if file size matches
> +        try:
> +            if svfs.stat(self._path).st_size == len(self):
> +                return
> +        except OSError as inst:
> +            if inst.errno != errno.ENOENT:
> +                raise
> +        svfs.makedirs('cache')
> +        with svfs(self._path, 'w', atomictemp=True) as f:
> +            f.write(self.data)
> +
>  class markerreader(object):
>      """read a range of markers with proper caching"""
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Pierre-Yves David - June 7, 2017, 2:12 p.m.
On 06/05/2017 07:37 PM, Augie Fackler wrote:
> On Sun, Jun 04, 2017 at 04:59:32PM -0700, Jun Wu wrote:
>> # HG changeset patch
>> # User Jun Wu <quark@fb.com>
>> # Date 1496615642 25200
>> #      Sun Jun 04 15:34:02 2017 -0700
>> # Node ID edae716e97c441aff52688f3be4750b34b199901
>> # Parent  160567b62f74e8e02fa2e9a7b2cb4076b105e528
>> # Available At https://bitbucket.org/quark-zju/hg-draft
>> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r edae716e97c4
>> obsstore: make markerindex support serialization
>>
>> This allows us to load and save index state to disk.
>>
>> Note this is the first user of .hg/store/cache. The indexes are purely
>> depending on .hg/store/obsstore and .hg/store/cache looks reasonable - if
>> store is shared, the indexes won't need to be duplicated.
>
> I'm kind of sort of -0 on adding another cache dir, but I don't feel
> strongly enough to want a change. Hold for opinions from other
> reviewers though.

All other caches depending on store content still lives on the cache 
directory. So I don't see a reason to make an exception here.

This patch in my other series makes available the proper vfs to access 
the 'cache/' to the obsstore logic.

Cheers,
Jun Wu - June 7, 2017, 3:15 p.m.
Excerpts from Pierre-Yves David's message of 2017-06-07 15:12:46 +0100:
> [...] 
> All other caches depending on store content still lives on the cache 
> directory. So I don't see a reason to make an exception here.

I think I have mentioned "shared repo". Why don't you think that's a reason?
 
> This patch in my other series makes available the proper vfs to access 
> the 'cache/' to the obsstore logic.
> 
> Cheers,
>
Yuya Nishihara - June 7, 2017, 3:36 p.m.
On Wed, 7 Jun 2017 08:15:34 -0700, Jun Wu wrote:
> Excerpts from Pierre-Yves David's message of 2017-06-07 15:12:46 +0100:
> > [...] 
> > All other caches depending on store content still lives on the cache 
> > directory. So I don't see a reason to make an exception here.
> 
> I think I have mentioned "shared repo". Why don't you think that's a reason?

I agree with Pierre-Yves here. Well, there's a reason to make it shareable,
but no reason to handle it specially. How about starting off with the plain-old
.hg/cache directory and later thinking about the proper way to share caches
across repositories?

Patch

diff --git a/mercurial/obsolete.py b/mercurial/obsolete.py
--- a/mercurial/obsolete.py
+++ b/mercurial/obsolete.py
@@ -532,9 +532,11 @@  class markerindex(radixlink.radixlink):
     def __init__(self, obsstore, name, keyfunc):
         """keyfunc: rawmarker -> [key]"""
-        super(markerindex, self).__init__()
+        path = 'cache/obsindex-%s' % name
+        data = obsstore.svfs.tryread(path)
+        super(markerindex, self).__init__(data)
+        self._path = path
         self._keyfunc = keyfunc
         self._obsstore = weakref.proxy(obsstore)
         self.name = name
-        self.sourceoftruthsize = 0
         self._readmarker = formats[obsstore._version][0]
         self.update()
@@ -567,4 +569,18 @@  class markerindex(radixlink.radixlink):
             return default
 
+    def flush(self):
+        """write index state to disk"""
+        svfs = self._obsstore.svfs
+        # fast path: no need to write if file size matches
+        try:
+            if svfs.stat(self._path).st_size == len(self):
+                return
+        except OSError as inst:
+            if inst.errno != errno.ENOENT:
+                raise
+        svfs.makedirs('cache')
+        with svfs(self._path, 'w', atomictemp=True) as f:
+            f.write(self.data)
+
 class markerreader(object):
     """read a range of markers with proper caching"""