[PATCH 2 of 2 SPRINT] changelog: avoid redundant truncation of 00changelog.i at failure

Yuya Nishihara yuya at tcha.org
Sun Nov 8 07:32:29 CST 2015


On Wed, 04 Nov 2015 03:13:55 +0900, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy at lares.dti.ne.jp>
> # Date 1446574170 -32400
> #      Wed Nov 04 03:09:30 2015 +0900
> # Node ID 79fae5df4947f0665eb0adf762a4c95d823901c1
> # Parent  9716bacc8e464e9ffaab609a74b9d31c9f93aa24
> changelog: avoid redundant truncation of 00changelog.i at failure
> 
> Before this patch, '00changelog.i' is always truncated at transaction
> failure, even though actual changes are written not into it but into
> '00changelog.i.a' (aka "pending file").
> 
> This truncation changes behavior depending on timestamp of the file
> (e.g. filecache). and may cause issues, which are difficult to
> be certainly reproduced, (e.g. issue4876).
> 
> On the other hand, if once changes are written into '00changelog.i' by
> '_finalize()', it should be truncated at failure.
> 
> To avoid such redundant truncation of '00changelog.i' at failure
> correctly, this patch schedules invoking own '_avoidrollback()' at
> failure by 'transaction.addabort()' in 'delayupdate()'.
> 
> It implies 'transaction._avoidrollback()' to avoid redundant
> truncation of '00changelog.i' at failure
> 
> This patch introduces new status field '_finalized' to detect whether
> truncation is needed or not, because combination of current inetrnal
> status fields '_divert' and '_delayed' can't distinguish cases below:
> 
>   - transaction is aborted before any change (truncation not needed)
>   - changelog is finalized successfully (truncation needed)
> 
> If changelog object in 'repo._filecache' isn't discarded as expected
> at failure, this patch can reproduce problems like issue4876 more
> frequently.
> 
> diff --git a/mercurial/changelog.py b/mercurial/changelog.py
> --- a/mercurial/changelog.py
> +++ b/mercurial/changelog.py
> @@ -147,6 +147,7 @@ class changelog(revlog.revlog):
>          self._delayed = False
>          self._delaybuf = None
>          self._divert = False
> +        self._finalized = False
>          self.filteredrevs = frozenset()
>  
>      def tip(self):
> @@ -252,6 +253,7 @@ class changelog(revlog.revlog):
>          self._delayed = True
>          tr.addpending('cl-%i' % id(self), self._writepending)
>          tr.addfinalize('cl-%i' % id(self), self._finalize)
> +        tr.addabort('cl-%i' % id(self), self._avoidrollback)
>  
>      def _finalize(self, tr):
>          "finalize index updates"
> @@ -270,6 +272,7 @@ class changelog(revlog.revlog):
>              fp.close()
>              self._delaybuf = None
>          self._divert = False
> +        self._finalized = True
>          # split when we're done
>          self.checkinlinesize(tr)

I can't tell if this is right or not, but it seems that _finalized should
be initialized to False per transaction.


More information about the Mercurial-devel mailing list