Patchwork D8191: nodemap: make sure on disk change get rolled back with the transaction

login
register
mail settings
Submitter phabricator
Date April 1, 2020, 3:21 p.m.
Message ID <762f7b50fc71a1f967359dca1bed1bb6@localhost.localdomain>
Download mbox | patch
Permalink /patch/45962/
State Not Applicable
Headers show

Comments

phabricator - April 1, 2020, 3:21 p.m.
Closed by commit rHG01b0805534bb: nodemap: make sure on disk change get rolled back with the  transaction (authored by marmoute).
This revision was automatically updated to reflect the committed changes.
This revision was not accepted when it landed; it landed in state "Needs Review".

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D8191?vs=20742&id=20933

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8191/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8191

AFFECTED FILES
  mercurial/configitems.py
  mercurial/revlogutils/nodemap.py
  tests/test-persistent-nodemap.t
  tests/testlib/crash_transaction_late.py

CHANGE DETAILS




To: marmoute, #hg-reviewers
Cc: mercurial-devel

Patch

diff --git a/tests/testlib/crash_transaction_late.py b/tests/testlib/crash_transaction_late.py
new file mode 100644
--- /dev/null
+++ b/tests/testlib/crash_transaction_late.py
@@ -0,0 +1,32 @@ 
+# tiny extension to abort a transaction very late during test
+#
+# Copyright 2020 Pierre-Yves David <pierre-yves.david@octobus.net>
+#
+# This software may be used and distributed according to the terms of the
+# GNU General Public License version 2 or any later version.
+
+from __future__ import absolute_import
+
+from mercurial import (
+    error,
+    transaction,
+)
+
+
+def abort(fp):
+    raise error.Abort(b"This is a late abort")
+
+
+def reposetup(ui, repo):
+
+    transaction.postfinalizegenerators.add(b'late-abort')
+
+    class LateAbortRepo(repo.__class__):
+        def transaction(self, *args, **kwargs):
+            tr = super(LateAbortRepo, self).transaction(*args, **kwargs)
+            tr.addfilegenerator(
+                b'late-abort', [b'late-abort'], abort, order=9999999
+            )
+            return tr
+
+    repo.__class__ = LateAbortRepo
diff --git a/tests/test-persistent-nodemap.t b/tests/test-persistent-nodemap.t
--- a/tests/test-persistent-nodemap.t
+++ b/tests/test-persistent-nodemap.t
@@ -353,3 +353,25 @@ 
 
   $ cat output.txt
 
+Check that a failing transaction will properly revert the data
+
+  $ echo plakfe > a
+  $ f --size --sha256 .hg/store/00changelog-*.nd
+  .hg/store/00changelog-????????????????.nd: size=123584, sha256=8c6cef6fd3d3fac291968793ee19a4be6d0b8375e9508bd5c7d4a8879e8df180 (glob) (pure !)
+  .hg/store/00changelog-????????????????.nd: size=123584, sha256=eb9e9a4bcafdb5e1344bc8a0cbb3288b2106413b8efae6265fb8a7973d7e97f9 (glob) (rust !)
+  .hg/store/00changelog-????????????????.nd: size=123136, sha256=4f504f5a834db3811ced50ab3e9e80bcae3581bb0f9b13a7a9f94b7fc34bcebe (glob) (no-pure no-rust !)
+  $ hg ci -m a3 --config "extensions.abort=$RUNTESTDIR/testlib/crash_transaction_late.py"
+  transaction abort!
+  rollback completed
+  abort: This is a late abort
+  [255]
+  $ hg debugnodemap --metadata
+  uid: ???????????????? (glob)
+  tip-rev: 5005
+  tip-node: bae4d45c759e30f1cb1a40e1382cf0e0414154db
+  data-length: 123584
+  data-unused: 448
+  $ f --size --sha256 .hg/store/00changelog-*.nd
+  .hg/store/00changelog-????????????????.nd: size=123584, sha256=8c6cef6fd3d3fac291968793ee19a4be6d0b8375e9508bd5c7d4a8879e8df180 (glob) (pure !)
+  .hg/store/00changelog-????????????????.nd: size=123584, sha256=eb9e9a4bcafdb5e1344bc8a0cbb3288b2106413b8efae6265fb8a7973d7e97f9 (glob) (rust !)
+  .hg/store/00changelog-????????????????.nd: size=123136, sha256=4f504f5a834db3811ced50ab3e9e80bcae3581bb0f9b13a7a9f94b7fc34bcebe (glob) (no-pure no-rust !)
diff --git a/mercurial/revlogutils/nodemap.py b/mercurial/revlogutils/nodemap.py
--- a/mercurial/revlogutils/nodemap.py
+++ b/mercurial/revlogutils/nodemap.py
@@ -93,6 +93,15 @@ 
     def addpostclose(self, callback_id, callback_func):
         self._postclose[callback_id] = callback_func
 
+    def registertmp(self, *args, **kwargs):
+        pass
+
+    def addbackup(self, *args, **kwargs):
+        pass
+
+    def add(self, *args, **kwargs):
+        pass
+
 
 def update_persistent_nodemap(revlog):
     """update the persistent nodemap right now
@@ -138,6 +147,7 @@ 
             # EXP-TODO: if this is a cache, this should use a cache vfs, not a
             # store vfs
             new_length = target_docket.data_length + len(data)
+            tr.add(datafile, target_docket.data_length)
             with revlog.opener(datafile, b'r+') as fd:
                 fd.seek(target_docket.data_length)
                 fd.write(data)
@@ -161,6 +171,7 @@ 
             data = persistent_data(revlog.index)
         # EXP-TODO: if this is a cache, this should use a cache vfs, not a
         # store vfs
+        tr.add(datafile, 0)
         with revlog.opener(datafile, b'w+') as fd:
             fd.write(data)
             if feed_data:
@@ -177,6 +188,10 @@ 
     file_path = revlog.nodemap_file
     if pending:
         file_path += b'.a'
+        tr.registertmp(file_path)
+    else:
+        tr.addbackup(file_path)
+
     with revlog.opener(file_path, b'w', atomictemp=True) as fp:
         fp.write(target_docket.serialize())
     revlog._nodemap_docket = target_docket
diff --git a/mercurial/configitems.py b/mercurial/configitems.py
--- a/mercurial/configitems.py
+++ b/mercurial/configitems.py
@@ -407,7 +407,6 @@ 
 )
 # TODO before getting `persistent-nodemap` out of experimental
 #
-# * code/tests around aborted transaction
 # * regenerate a new nodemap when the unused/total ration is to high
 # * decide for a "status" of the persistent nodemap and associated location
 #   - part of the store next the revlog itself (new requirements)