This is an archive of the discontinued Mercurial Phabricator instance.

Differential D12009

revlog: fix a bug where transaction can be aborted partially
ClosedPublic

Authored by aalekseyev on Jan 20 2022, 9:09 AM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHGccd9cb73125c: revlog: fix a bug where transaction can be aborted partially
Summary

Fix a repo corruption bug caused by a partial transaction rollback.

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

aalekseyev created this revision.Jan 20 2022, 9:09 AM
Alphare accepted this revision.Jan 20 2022, 9:43 AM
Alphare added a subscriber: Alphare.

That's rough, thanks for finding and fixing it.

I think it hasn't affected many users as you said because you need to have a transaction roll back during a split, which would only happen frequently in high intensity.

This revision is now accepted and ready to land.Jan 20 2022, 9:43 AM

Revision Contents
Changeset List

Diff 31709

mercurial/revlog.py

if not self._inline or total_size < _maxinline: if not self._inline or total_size < _maxinline:
return return
troffset = tr.findoffset(self._indexfile) troffset = tr.findoffset(self._indexfile)
if troffset is None: if troffset is None:
raise error.RevlogError( raise error.RevlogError(
_(b"%s not found in the transaction") % self._indexfile _(b"%s not found in the transaction") % self._indexfile
) )
trindex = 0 trindex = None
tr.add(self._datafile, 0) tr.add(self._datafile, 0)
existing_handles = False existing_handles = False
if self._writinghandles is not None: if self._writinghandles is not None:
existing_handles = True existing_handles = True
fp = self._writinghandles[0] fp = self._writinghandles[0]
fp.flush() fp.flush()
fp.close() fp.close()
# We can't use the cached file handle after close(). So prevent # We can't use the cached file handle after close(). So prevent
# its usage. # its usage.
self._writinghandles = None self._writinghandles = None
self._segmentfile.writing_handle = None self._segmentfile.writing_handle = None
# No need to deal with sidedata writing handle as it is only # No need to deal with sidedata writing handle as it is only
# relevant with revlog-v2 which is never inline, not reaching # relevant with revlog-v2 which is never inline, not reaching
# this code # this code
new_dfh = self._datafp(b'w+') new_dfh = self._datafp(b'w+')
new_dfh.truncate(0) # drop any potentially existing data new_dfh.truncate(0) # drop any potentially existing data
try: try:
with self._indexfp() as read_ifh: with self._indexfp() as read_ifh:
for r in self: for r in self:
new_dfh.write(self._getsegmentforrevs(r, r, df=read_ifh)[1]) new_dfh.write(self._getsegmentforrevs(r, r, df=read_ifh)[1])
if troffset <= self.start(r) + r * self.index.entry_size: if (
trindex is None
and troffset
<= self.start(r) + r * self.index.entry_size
):
trindex = r trindex = r
new_dfh.flush() new_dfh.flush()
if trindex is None:
trindex = 0
with self.__index_new_fp() as fp: with self.__index_new_fp() as fp:
self._format_flags &= ~FLAG_INLINE_DATA self._format_flags &= ~FLAG_INLINE_DATA
self._inline = False self._inline = False
for i in self: for i in self:
e = self.index.entry_binary(i) e = self.index.entry_binary(i)
if i == 0 and self._docket is None: if i == 0 and self._docket is None:
header = self._format_flags | self._format_version header = self._format_flags | self._format_version
header = self.index.pack_header(header) header = self.index.pack_header(header)

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

pulling from ../troffset-computation pulling from ../troffset-computation
[255] [255]
#else #else
$ hg pull ../troffset-computation $ hg pull ../troffset-computation
pulling from ../troffset-computation pulling from ../troffset-computation
[80] [80]
#endif #endif
$ cat .hg/store/journal | tr -s '\000' ' ' | grep data/file | tail -1 $ cat .hg/store/journal | tr -s '\000' ' ' | grep data/file | tail -1
data/file.i 192 data/file.i 128
The first file.i entry should match the "Reference size" above. The first file.i entry should match the "Reference size" above.
The first file.d entry is the temporary record during the split, The first file.d entry is the temporary record during the split,
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 1067 data/file.d 1046
data/file.i 192 data/file.i 128
$ hg recover $ hg recover
rolling back interrupted transaction rolling back interrupted transaction
(verify step skipped, run `hg verify` to check your repository content) (verify step skipped, run `hg verify` to check your repository content)
$ f -s .hg/store/data/file* $ f -s .hg/store/data/file*
.hg/store/data/file.d: size=1067 .hg/store/data/file.d: size=1046
.hg/store/data/file.i: size=192 .hg/store/data/file.i: size=128
$ hg tip $ hg tip
changeset: 1:cfa8d6e60429 changeset: 1:cfa8d6e60429
tag: tip tag: tip
user: test user: test
date: Thu Jan 01 00:00:00 1970 +0000 date: Thu Jan 01 00:00:00 1970 +0000
summary: b summary: b
$ hg verify -q $ hg verify -q
warning: revlog 'data/file.d' not in fncache! warning: revlog 'data/file.d' not in fncache!
file@?: rev 2 points to nonexistent changeset 2 1 warnings encountered!
(expected )
file@?: fa1120531cc1 not in manifests
2 warnings encountered!
hint: run "hg debugrebuildfncache" to recover from corrupt fncache hint: run "hg debugrebuildfncache" to recover from corrupt fncache
2 integrity errors encountered!
[1]
$ hg debugrebuildfncache --only-data $ hg debugrebuildfncache --only-data
adding data/file.d adding data/file.d
1 items added, 0 removed from fncache 1 items added, 0 removed from fncache
$ hg verify -q $ hg verify -q
file@?: rev 2 points to nonexistent changeset 2
(expected )
file@?: fa1120531cc1 not in manifests
1 warnings encountered!
2 integrity errors encountered!
[1]
$ cd .. $ cd ..
Now retry the procedure 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
#else #else
$ hg pull ../troffset-computation $ hg pull ../troffset-computation
pulling from ../troffset-computation pulling from ../troffset-computation
[80] [80]
#endif #endif
$ 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 1067 data/file.d 1046
$ hg recover $ hg recover
rolling back interrupted transaction rolling back interrupted transaction
(verify step skipped, run `hg verify` to check your repository content) (verify step skipped, run `hg verify` to check your repository content)
$ f -s .hg/store/data/file* $ f -s .hg/store/data/file*
.hg/store/data/file.d: size=1067 .hg/store/data/file.d: size=1046
.hg/store/data/file.i: size=1174 .hg/store/data/file.i: size=1174
$ hg tip $ hg tip
changeset: 1:cfa8d6e60429 changeset: 1:cfa8d6e60429
tag: tip tag: tip
user: test user: test
date: Thu Jan 01 00:00:00 1970 +0000 date: Thu Jan 01 00:00:00 1970 +0000
summary: b summary: b
adding changesets adding changesets
adding manifests adding manifests
adding file changes adding file changes
transaction abort! transaction abort!
rollback completed rollback completed
abort: pretxnchangegroup hook exited with status 1 abort: pretxnchangegroup hook exited with status 1
[40] [40]
$ f -s .hg/store/data/file* $ f -s .hg/store/data/file*
.hg/store/data/file.d: size=1067 .hg/store/data/file.d: size=1046
.hg/store/data/file.i: size=192 .hg/store/data/file.i: size=128
$ hg tip $ hg tip
changeset: 1:cfa8d6e60429 changeset: 1:cfa8d6e60429
tag: tip tag: tip
user: test user: test
date: Thu Jan 01 00:00:00 1970 +0000 date: Thu Jan 01 00:00:00 1970 +0000
summary: b summary: b
$ hg verify -q $ hg verify -q
warning: revlog 'data/file.d' not in fncache! warning: revlog 'data/file.d' not in fncache!
file@?: rev 2 points to nonexistent changeset 2 1 warnings encountered!
(expected )
file@?: fa1120531cc1 not in manifests
2 warnings encountered!
hint: run "hg debugrebuildfncache" to recover from corrupt fncache hint: run "hg debugrebuildfncache" to recover from corrupt fncache
2 integrity errors encountered!
[1]
$ cd .. $ cd ..