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

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

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

valentin.gatienbaron (Valentin Gatien-Baron)
marmoute created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  In case of errors, we need to rollback the change made to the persistent
  nodemap.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

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 <[hidden email]>
+#
+# 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)



To: marmoute, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

valentin.gatienbaron (Valentin Gatien-Baron)
marmoute added a comment.
marmoute updated this revision to Diff 20583.


  eventless rebase past D8255 <https://phab.mercurial-scm.org/D8255>

REPOSITORY
  rHG Mercurial

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

BRANCH
  default

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

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 <[hidden email]>
+#
+# 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)



To: marmoute, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

valentin.gatienbaron (Valentin Gatien-Baron)
In reply to this post by valentin.gatienbaron (Valentin Gatien-Baron)
marmoute added a comment.
marmoute updated this revision to Diff 20686.


  rebase above latest default up to landed-D8182

REPOSITORY
  rHG Mercurial

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

BRANCH
  default

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

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 <[hidden email]>
+#
+# 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)



To: marmoute, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

valentin.gatienbaron (Valentin Gatien-Baron)
In reply to this post by valentin.gatienbaron (Valentin Gatien-Baron)
marmoute updated this revision to Diff 20742.

REPOSITORY
  rHG Mercurial

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

BRANCH
  default

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

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 <[hidden email]>
+#
+# 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)



To: marmoute, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

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

valentin.gatienbaron (Valentin Gatien-Baron)
In reply to this post by valentin.gatienbaron (Valentin Gatien-Baron)
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

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 <[hidden email]>
+#
+# 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)



To: marmoute, #hg-reviewers
Cc: mercurial-devel
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel