This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9275

transaction: drop per-file extra data support
ClosedPublic

Authored by joerg.sonnenberger on Nov 7 2020, 4:31 PM.

Details

Reviewers
indygreg
Group Reviewers
hg-reviewers
Commits
rHG63edc384d3b7: transaction: drop per-file extra data support
rHG203f178a9dc8: transaction: drop per-file extra data support
Summary

At the moment, transactions support an optional extra data argument for
all files to be stored in addition to the original offset. This is used
in core only by the revlog inline to external data migration. It is used
to memoize the number of revisions before the transaction. That number
of can be computed during the walk easily, so drop the requirement.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

indygreg accepted this revision.Nov 7 2020, 5:38 PM

I _think_ this is safe. But I would appreciate an extra set of eyes on the changed code in revlog.py. Essentially what's happening here is we were previously stuffing the number of revisions in the revlog in the in-memory data structure so we could recover the inline revlog if we had to roll back a .i -> .d revlog split that was incurred as part of the transaction.

The key line not changed in this patch is tr.replace(self.indexfile, trindex * self._io.size). And the new code gradually incrementing trindex as long as we're processing entries earlier than the .i offset recorded in the transaction seems correct to me.

This revision is now accepted and ready to land.Nov 7 2020, 5:38 PM
joerg.sonnenberger added a commit: rHG203f178a9dc8: transaction: drop per-file extra data support.Nov 18 2020, 6:07 PM
Closed by commit rHG203f178a9dc8: transaction: drop per-file extra data support (authored by joerg.sonnenberger). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 23425

mercurial/repair.py

cl.strip(striprev, tr) cl.strip(striprev, tr)
stripmanifest(repo, striprev, tr, files) stripmanifest(repo, striprev, tr, files)
for fn in files: for fn in files:
repo.file(fn).strip(striprev, tr) repo.file(fn).strip(striprev, tr)
tr.endgroup() tr.endgroup()
for i in pycompat.xrange(offset, len(tr._entries)): for i in pycompat.xrange(offset, len(tr._entries)):
file, troffset, ignore = tr._entries[i] file, troffset = tr._entries[i]
with repo.svfs(file, b'a', checkambig=True) as fp: with repo.svfs(file, b'a', checkambig=True) as fp:
fp.truncate(troffset) fp.truncate(troffset)
if troffset == 0: if troffset == 0:
repo.store.markremoved(file) repo.store.markremoved(file)
deleteobsmarkers(repo.obsstore, stripobsidx) deleteobsmarkers(repo.obsstore, stripobsidx)
del repo.obsstore del repo.obsstore
repo.invalidatevolatilesets() repo.invalidatevolatilesets()

mercurial/revlog.py

): ):
return return
trinfo = tr.find(self.indexfile) trinfo = tr.find(self.indexfile)
if trinfo is None: if trinfo 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
) )
troffset = trinfo[1]
trindex = trinfo[2] trindex = 0
if trindex is not None: tr.add(self.datafile, 0)
dataoff = self.start(trindex)
else:
# revlog was stripped at start of transaction, use all leftover data
trindex = len(self) - 1
dataoff = self.end(tiprev)
tr.add(self.datafile, dataoff)
if fp: if fp:
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
with self._indexfp(b'r') as ifh, self._datafp(b'w') as dfh: with self._indexfp(b'r') as ifh, self._datafp(b'w') as dfh:
for r in self: for r in self:
dfh.write(self._getsegmentforrevs(r, r, df=ifh)[1]) dfh.write(self._getsegmentforrevs(r, r, df=ifh)[1])
if troffset <= self.start(r):
trindex = r
with self._indexfp(b'w') as fp: with self._indexfp(b'w') as fp:
self.version &= ~FLAG_INLINE_DATA self.version &= ~FLAG_INLINE_DATA
self._inline = False self._inline = False
io = self._io io = self._io
for i in self: for i in self:
e = io.packentry(self.index[i], self.node, self.version, i) e = io.packentry(self.index[i], self.node, self.version, i)
fp.write(e) fp.write(e)
transaction.add(self.datafile, offset) transaction.add(self.datafile, offset)
transaction.add(self.indexfile, curr * len(entry)) transaction.add(self.indexfile, curr * len(entry))
if data[0]: if data[0]:
dfh.write(data[0]) dfh.write(data[0])
dfh.write(data[1]) dfh.write(data[1])
ifh.write(entry) ifh.write(entry)
else: else:
offset += curr * self._io.size offset += curr * self._io.size
transaction.add(self.indexfile, offset, curr) transaction.add(self.indexfile, offset)
ifh.write(entry) ifh.write(entry)
ifh.write(data[0]) ifh.write(data[0])
ifh.write(data[1]) ifh.write(data[1])
self._enforceinlinesize(transaction, ifh) self._enforceinlinesize(transaction, ifh)
nodemaputil.setup_persistent_nodemap(transaction, self) nodemaputil.setup_persistent_nodemap(transaction, self)
def addgroup( def addgroup(
self, self,
r = len(self) r = len(self)
end = 0 end = 0
if r: if r:
end = self.end(r - 1) end = self.end(r - 1)
ifh = self._indexfp(b"a+") ifh = self._indexfp(b"a+")
isize = r * self._io.size isize = r * self._io.size
if self._inline: if self._inline:
transaction.add(self.indexfile, end + isize, r) transaction.add(self.indexfile, end + isize)
dfh = None dfh = None
else: else:
transaction.add(self.indexfile, isize, r) transaction.add(self.indexfile, isize)
transaction.add(self.datafile, end) transaction.add(self.datafile, end)
dfh = self._datafp(b"a+") dfh = self._datafp(b"a+")
def flush(): def flush():
if dfh: if dfh:
dfh.flush() dfh.flush()
ifh.flush() ifh.flush()

mercurial/transaction.py

report, report,
opener, opener,
vfsmap, vfsmap,
entries, entries,
backupentries, backupentries,
unlink=True, unlink=True,
checkambigfiles=None, checkambigfiles=None,
): ):
for f, o, _ignore in entries: for f, o in entries:
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 "
@active @active
def endgroup(self): def endgroup(self):
"""apply delayed registration of file entry. """apply delayed registration of file entry.
This is used by strip to delay vision of strip offset. The transaction This is used by strip to delay vision of strip offset. The transaction
sees either none or all of the strip actions to be done.""" sees either none or all of the strip actions to be done."""
q = self._queue.pop() q = self._queue.pop()
for f, o, data in q: for f, o in q:
self._addentry(f, o, data) self._addentry(f, o)
@active @active
def add(self, file, offset, data=None): def add(self, file, offset):
"""record the state of an append-only file before update""" """record the state of an append-only file before update"""
if file in self._map or file in self._backupmap: if file in self._map or file in self._backupmap:
return return
if self._queue: if self._queue:
self._queue[-1].append((file, offset, data)) self._queue[-1].append((file, offset))
return return
self._addentry(file, offset, data) self._addentry(file, offset)
def _addentry(self, file, offset, data): def _addentry(self, file, offset):
"""add a append-only entry to memory and on-disk state""" """add a append-only entry to memory and on-disk state"""
if file in self._map or file in self._backupmap: if file in self._map or file in self._backupmap:
return return
self._entries.append((file, offset, data)) self._entries.append((file, offset))
self._map[file] = len(self._entries) - 1 self._map[file] = len(self._entries) - 1
# add enough data to the journal to do the truncate # add enough data to the journal to do the truncate
self._file.write(b"%s\0%d\n" % (file, offset)) self._file.write(b"%s\0%d\n" % (file, offset))
self._file.flush() self._file.flush()
@active @active
def addbackup(self, file, hardlink=True, location=b''): def addbackup(self, file, hardlink=True, location=b''):
"""Adds a backup of the file to the transaction """Adds a backup of the file to the transaction
def find(self, file): def find(self, file):
if file in self._map: if file in self._map:
return self._entries[self._map[file]] return self._entries[self._map[file]]
if file in self._backupmap: if file in self._backupmap:
return self._backupentries[self._backupmap[file]] return self._backupentries[self._backupmap[file]]
return None return None
@active @active
def replace(self, file, offset, data=None): def replace(self, file, offset):
''' '''
replace can only replace already committed entries replace can only replace already committed entries
that are not pending in the queue that are not pending in the queue
''' '''
if file not in self._map: if file not in self._map:
raise KeyError(file) raise KeyError(file)
index = self._map[file] index = self._map[file]
self._entries[index] = (file, offset, data) self._entries[index] = (file, offset)
self._file.write(b"%s\0%d\n" % (file, offset)) self._file.write(b"%s\0%d\n" % (file, offset))
self._file.flush() self._file.flush()
@active @active
def nest(self, name='<unnamed>'): def nest(self, name='<unnamed>'):
self._count += 1 self._count += 1
self._usages += 1 self._usages += 1
self._names.append(name) self._names.append(name)
backupentries = [] backupentries = []
fp = opener.open(file) fp = opener.open(file)
lines = fp.readlines() lines = fp.readlines()
fp.close() fp.close()
for l in lines: for l in lines:
try: try:
f, o = l.split(b'\0') f, o = l.split(b'\0')
entries.append((f, int(o), None)) entries.append((f, int(o)))
except ValueError: except ValueError:
report( report(
_(b"couldn't read journal entry %r!\n") % pycompat.bytestr(l) _(b"couldn't read journal entry %r!\n") % pycompat.bytestr(l)
) )
backupjournal = b"%s.backupfiles" % file backupjournal = b"%s.backupfiles" % file
if opener.exists(backupjournal): if opener.exists(backupjournal):
fp = opener.open(backupjournal) fp = opener.open(backupjournal)

tests/test-mq-qpush-fail.t

> def wrapplayback(orig, > def wrapplayback(orig,
> journal, report, opener, vfsmap, entries, backupentries, > journal, report, opener, vfsmap, entries, backupentries,
> unlink=True, checkambigfiles=None): > unlink=True, checkambigfiles=None):
> orig(journal, report, opener, vfsmap, entries, backupentries, unlink, > orig(journal, report, opener, vfsmap, entries, backupentries, unlink,
> checkambigfiles) > checkambigfiles)
> # Touching files truncated at "transaction.abort" causes > # Touching files truncated at "transaction.abort" causes
> # forcible re-loading invalidated filecache properties > # forcible re-loading invalidated filecache properties
> # (including repo.changelog) > # (including repo.changelog)
> for f, o, _ignore in entries: > for f, o in entries:
> if o or not unlink: > if o or not unlink:
> os.utime(opener.join(f), (0.0, 0.0)) > os.utime(opener.join(f), (0.0, 0.0))
> def extsetup(ui): > def extsetup(ui):
> extensions.wrapfunction(transaction, '_playback', wrapplayback) > extensions.wrapfunction(transaction, '_playback', wrapplayback)
> EOF > EOF
$ hg qpush -a --config extensions.wrapplayback=$TESTTMP/wrapplayback.py && echo 'qpush succeeded?!' $ hg qpush -a --config extensions.wrapplayback=$TESTTMP/wrapplayback.py && echo 'qpush succeeded?!'
applying patch1 applying patch1
applying patch2 applying patch2