Patchwork D10629: revlogv2: delay the update of the changelog docket to transaction end

login
register
mail settings
Submitter phabricator
Date May 3, 2021, 12:08 p.m.
Message ID <differential-rev-PHID-DREV-zuvhfqepczhva67zdjhl-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/48947/
State Superseded
Headers show

Comments

phabricator - May 3, 2021, 12:08 p.m.
marmoute created this revision.
Herald added a reviewer: indygreg.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  This prevent external reader to see the transaction content before it is
  commited. However this also prevent the hooks to see the transaction content.
  We will fix this in later changesets.
  
  We have to temporarily suppress the error output of the command ran during the
  transaction as they sometimes get confused about unknown working directory and
  sometimes issue message on std-err in unspecified order.

REPOSITORY
  rHG Mercurial

BRANCH
  default

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

AFFECTED FILES
  mercurial/changelog.py
  mercurial/configitems.py
  mercurial/revlog.py
  tests/test-transaction-safety.t

CHANGE DETAILS




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

Patch

diff --git a/tests/test-transaction-safety.t b/tests/test-transaction-safety.t
--- a/tests/test-transaction-safety.t
+++ b/tests/test-transaction-safety.t
@@ -46,13 +46,13 @@ 
   $ cat << EOF > script/external.sh
   > #!/bin/sh
   > $RUNTESTDIR/testlib/wait-on-file 5 $HG_TEST_FILE_EXT_UNLOCK $HG_TEST_FILE_EXT_WAITING
-  > hg log --rev 'tip' -T 'external: {rev} {desc}\n' > $TESTTMP/output/external.out
+  > hg log --rev 'tip' -T 'external: {rev} {desc}\n' > $TESTTMP/output/external.out 2>/dev/null
   > touch $HG_TEST_FILE_EXT_DONE
   > EOF
   $ chmod +x script/external.sh
   $ cat << EOF > script/internal.sh
   > #!/bin/sh
-  > hg log --rev 'tip' -T 'internal: {rev} {desc}\n' > $TESTTMP/output/internal.out
+  > hg log --rev 'tip' -T 'internal: {rev} {desc}\n' > $TESTTMP/output/internal.out 2>/dev/null
   > $RUNTESTDIR/testlib/wait-on-file 5 $HG_TEST_FILE_EXT_DONE $HG_TEST_FILE_EXT_UNLOCK
   > EOF
   $ chmod +x script/internal.sh
@@ -123,9 +123,9 @@ 
 
   $ make_one_commit first
   pre-commit: -1 
-  external: -1  (revlogv1 !)
-  external: 0 first (revlogv2 known-bad-output !)
-  internal: 0 first
+  external: -1 
+  internal: 0 first (revlogv1 !)
+  internal: -1  (revlogv2 known-bad-output !)
   post-tr:  0 first
 
 #if revlogv1
@@ -149,9 +149,9 @@ 
 
   $ make_one_commit second
   pre-commit: 0 first
-  external: 0 first (revlogv1 !)
-  external: 1 second (revlogv2 known-bad-output !)
-  internal: 1 second
+  external: 0 first
+  internal: 1 second (revlogv1 !)
+  internal: 0 first (revlogv2 known-bad-output !)
   post-tr:  1 second
 
 #if revlogv1
@@ -176,9 +176,9 @@ 
   $ make_one_pull 3
   pre-commit: 1 second
   warning: repository is unrelated
-  external: 1 second (revlogv1 !)
-  external: 5 r3 (revlogv2 known-bad-output !)
-  internal: 5 r3
+  external: 1 second
+  internal: 5 r3 (revlogv1 !)
+  internal: 1 second (revlogv2 known-bad-output !)
   post-tr:  5 r3
 
 #if revlogv1
@@ -202,9 +202,9 @@ 
 
   $ make_one_pull 400
   pre-commit: 5 r3
-  external: 5 r3 (revlogv1 !)
-  external: 402 r400 (revlogv2 known-bad-output !)
-  internal: 402 r400
+  external: 5 r3
+  internal: 402 r400 (revlogv1 !)
+  internal: 5 r3 (revlogv2 known-bad-output !)
   post-tr:  402 r400
 
 #if revlogv1
@@ -228,9 +228,9 @@ 
 
   $ make_one_commit third
   pre-commit: 402 r400
-  external: 402 r400 (revlogv1 !)
-  external: 403 third (revlogv2 known-bad-output !)
-  internal: 403 third
+  external: 402 r400
+  internal: 403 third (revlogv1 !)
+  internal: 402 r400 (revlogv2 known-bad-output !)
   post-tr:  403 third
 
 #if revlogv1
@@ -255,9 +255,9 @@ 
 
   $ make_one_pull tip
   pre-commit: 403 third
-  external: 403 third (revlogv1 !)
-  external: 503 r500 (revlogv2 known-bad-output !)
-  internal: 503 r500
+  external: 403 third
+  internal: 503 r500 (revlogv1 !)
+  internal: 403 third (revlogv2 known-bad-output !)
   post-tr:  503 r500
 
 #if revlogv1
diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -2096,7 +2096,7 @@ 
                     try:
                         yield
                         if self._docket is not None:
-                            self._docket.write(transaction)
+                            self._write_docket(transaction)
                     finally:
                         self._writinghandles = None
                 finally:
@@ -2105,6 +2105,15 @@ 
                 if dfh is not None:
                     dfh.close()
 
+    def _write_docket(self, transaction):
+        """write the current docket on disk
+
+        Exist as a method to help changelog to implement transaction logic
+
+        We could also imagine using the same transaction logic for all revlog
+        since docket are cheap."""
+        self._docket.write(transaction)
+
     def addrevision(
         self,
         text,
@@ -3187,18 +3196,6 @@ 
             # Nothing to generate or remove
             return
 
-        # changelog implement some "delayed" writing mechanism that assume that
-        # all index data is writen in append mode and is therefor incompatible
-        # with the seeked write done in this method. The use of such "delayed"
-        # writing will soon be removed for revlog version that support side
-        # data, so for now, we only keep this simple assert to highlight the
-        # situation.
-        delayed = getattr(self, '_delayed', False)
-        diverted = getattr(self, '_divert', False)
-        if delayed and not diverted:
-            msg = "cannot rewrite_sidedata of a delayed revlog"
-            raise error.ProgrammingError(msg)
-
         new_entries = []
         # append the new sidedata
         with self._writing(transaction):
diff --git a/mercurial/configitems.py b/mercurial/configitems.py
--- a/mercurial/configitems.py
+++ b/mercurial/configitems.py
@@ -1145,7 +1145,6 @@ 
 )
 # "out of experimental" todo list.
 #
-# * properly hide uncommitted content to other process
 # * expose transaction content hooks during pre-commit validation
 # * include management of a persistent nodemap in the main docket
 # * enforce a "no-truncate" policy for mmap safety
diff --git a/mercurial/changelog.py b/mercurial/changelog.py
--- a/mercurial/changelog.py
+++ b/mercurial/changelog.py
@@ -443,12 +443,13 @@ 
         self._filteredrevs = val
         self._filteredrevs_hashcache = {}
 
+    def _write_docket(self, tr):
+        if not self._delayed:
+            super(changelog, self)._write_docket(tr)
+
     def delayupdate(self, tr):
         """delay visibility of index updates to other readers"""
-        if self._docket is not None:
-            return
-
-        if not self._delayed:
+        if self._docket is None and not self._delayed:
             if len(self) == 0:
                 self._divert = True
                 if self._realopener.exists(self._indexfile + b'.a'):
@@ -468,7 +469,9 @@ 
         self._delayed = False
         self.opener = self._realopener
         # move redirected index data back into place
-        if self._divert:
+        if self._docket is not None:
+            self._write_docket(tr)
+        elif self._divert:
             assert not self._delaybuf
             tmpname = self._indexfile + b".a"
             nfile = self.opener.open(tmpname)