This is an archive of the discontinued Mercurial Phabricator instance.

Differential D10726

recover: only apply last journal record per file (issue6423)
ClosedPublic

Authored by joerg.sonnenberger on May 17 2021, 10:51 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG18415fc918a1: recover: only apply last journal record per file (issue6423)
rHGbca9faa0aafc: recover: only apply last journal record per file (issue6423)
rHG672f48cf1d03: recover: only apply last journal record per file (issue6423)
Summary

This got broken in 2019 when the size check was introduced. It is most
noticable when dealing with transactions that involve an inline to
non-inline revlog storage transaction. It wasn't seen as much at the
time because the in-memory journal actually de-duplicated the entry
implicity, but since 63edc384d3b7 the on-disk journal is used for
rollback as well as recover.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

valentin.gatienbaron added a subscriber: valentin.gatienbaron.May 18 2021, 11:14 PM

Does the order of the truncates actually not matter, for concurrent readers? I don't really see how it could matter (linkrevs should make it possible to ignore everything not committed), but before looking at the rollback code, I imagined it truncated in reverse order of writes (and looking it up, it's what's described in the initial revlog paper).

I can't find documentation that dict() guarantees to favor the rightmost key, but I'm assuming it's true anyway.
Modulo the question about order of truncates, this looks good.

valentin.gatienbaron added a comment.May 18 2021, 11:38 PM

Also, can you put (issue6423) in the first line of the commit message, so the bugzilla magic closes the issue?

joerg.sonnenberger retitled this revision from recover: only apply last journal record per file to recover: only apply last journal record per file (issue6423).May 19 2021, 9:54 AM
joerg.sonnenberger added a commit: rHG672f48cf1d03: recover: only apply last journal record per file (issue6423).May 19 2021, 1:58 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG672f48cf1d03: recover: only apply last journal record per file (issue6423) (authored by joerg.sonnenberger). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 28126

mercurial/transaction.py

report, report,
opener, opener,
vfsmap, vfsmap,
entries, entries,
backupentries, backupentries,
unlink=True, unlink=True,
checkambigfiles=None, checkambigfiles=None,
): ):
for f, o in entries: for f, o in sorted(dict(entries).items()):
if o or not unlink: if o or not unlink:
checkambig = checkambigfiles and (f, b'') in checkambigfiles checkambig = checkambigfiles and (f, b'') in checkambigfiles
try: try:
fp = opener(f, b'a', checkambig=checkambig) fp = opener(f, b'a', checkambig=checkambig)
if fp.tell() < o: if fp.tell() < o:
raise error.Abort( raise error.Abort(
_( _(
b"attempted to truncate %s to %d bytes, but it was " b"attempted to truncate %s to %d bytes, but it was "

tests/test-transaction-rollback-on-revlog-split.t

the second entry after the split happened. The sum of the second file.d the second entry after the split happened. The sum of the second file.d
and the second file.i entry should match the first file.i entry. and the second file.i entry should match the first file.i entry.
$ cat .hg/store/journal | tr -s '\000' ' ' | grep data/file $ cat .hg/store/journal | tr -s '\000' ' ' | grep data/file
data/file.i 1174 data/file.i 1174
data/file.d 0 data/file.d 0
data/file.d 1046 data/file.d 1046
data/file.i 128 data/file.i 128
$ hg recover
rolling back interrupted transaction
(verify step skipped, run `hg verify` to check your repository content)
$ f -s .hg/store/data/file*
.hg/store/data/file.d: size=1046
.hg/store/data/file.i: size=128
$ hg tip
changeset: 1:3ce491143aec
tag: tip
user: test
date: Thu Jan 01 00:00:00 1970 +0000
summary: _
$ hg verify
checking changesets
checking manifests
crosschecking files in changesets and manifests
checking files
warning: revlog 'data/file.d' not in fncache!
checked 2 changesets with 2 changes to 1 files
1 warnings encountered!
hint: run "hg debugrebuildfncache" to recover from corrupt fncache
$ cd .. $ cd ..
Now retry the same but intercept the rename of the index and check that
Now retry the procedure but intercept the rename of the index and check that
the journal does not contain the new index size. This demonstrates the edge case the journal does not contain the new index size. This demonstrates the edge case
where the data file is left as garbage. where the data file is left as garbage.
$ hg clone -r 1 troffset-computation troffset-computation-copy2 --config format.revlog-compression=none -q $ hg clone -r 1 troffset-computation troffset-computation-copy2 --config format.revlog-compression=none -q
$ cd troffset-computation-copy2 $ cd troffset-computation-copy2
$ cat > .hg/hgrc <<EOF $ cat > .hg/hgrc <<EOF
> [extensions] > [extensions]
> intercept_rename = $TESTTMP/intercept_rename.py > intercept_rename = $TESTTMP/intercept_rename.py
> [hooks] > [hooks]
> pretxnchangegroup = python:$TESTDIR/helper-killhook.py:killme > pretxnchangegroup = python:$TESTDIR/helper-killhook.py:killme
> EOF > EOF
$ hg pull ../troffset-computation $ hg pull ../troffset-computation
pulling from ../troffset-computation pulling from ../troffset-computation
[80] [80]
$ cat .hg/store/journal | tr -s '\000' ' ' | grep data/file $ cat .hg/store/journal | tr -s '\000' ' ' | grep data/file
data/file.i 1174 data/file.i 1174
data/file.d 0 data/file.d 0
data/file.d 1046 data/file.d 1046
$ hg recover
rolling back interrupted transaction
(verify step skipped, run `hg verify` to check your repository content)
$ f -s .hg/store/data/file*
.hg/store/data/file.d: size=1046
.hg/store/data/file.i: size=1174
$ hg tip
changeset: 1:3ce491143aec
tag: tip
user: test
date: Thu Jan 01 00:00:00 1970 +0000
summary: _
$ hg verify
checking changesets
checking manifests
crosschecking files in changesets and manifests
checking files
checked 2 changesets with 2 changes to 1 files
$ cd ..
Repeat the original test but let hg rollback the transaction.
$ hg clone -r 1 troffset-computation troffset-computation-copy-rb --config format.revlog-compression=none -q
$ cd troffset-computation-copy-rb
$ cat > .hg/hgrc <<EOF
> [hooks]
> pretxnchangegroup = false
> EOF
$ hg pull ../troffset-computation
pulling from ../troffset-computation
searching for changes
adding changesets
adding manifests
adding file changes
transaction abort!
rollback completed
abort: pretxnchangegroup hook exited with status 1
[40]
$ f -s .hg/store/data/file*
.hg/store/data/file.d: size=1046
.hg/store/data/file.i: size=128
$ hg tip
changeset: 1:3ce491143aec
tag: tip
user: test
date: Thu Jan 01 00:00:00 1970 +0000
summary: _
$ hg verify
checking changesets
checking manifests
crosschecking files in changesets and manifests
checking files
warning: revlog 'data/file.d' not in fncache!
checked 2 changesets with 2 changes to 1 files
1 warnings encountered!
hint: run "hg debugrebuildfncache" to recover from corrupt fncache
$ cd .. $ cd ..