This is an archive of the discontinued Mercurial Phabricator instance.

undo: move undolog to vfs and mark as lock-safe
ClosedPublic

Authored by quark on Jul 17 2017, 4:56 PM.

Download Raw Diff

Details

Reviewers

durham
ryanmce

Group Reviewers

Restricted Project

Commits

rFBHGX910ef98c3557: undo: move undolog to vfs and mark as lock-safe

Summary

The upstream has added some warning around repo.svfs/vfs write operations
without taking locks. For undo, we don't want to take the repo lock (since
that means read-only operations like hg log will be blocked by another
hg commit). So undo has its own locking and own transaction (for lower
overhead). Therefore move all files we may write (including lock and
transaction files) to .hg/undolog and whitelist the directory at
localrepository._wlockfreeprefix to bypass the checks.

Diff Detail

Repository

rFBHGX Facebook Mercurial Extensions

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

quark created this revision.Jul 17 2017, 4:56 PM

Herald added a reviewer: Restricted Project. · View Herald TranscriptJul 17 2017, 4:56 PM

quark added a reviewer: durham.Jul 17 2017, 9:25 PM

Why not just use the normal vfs? Then you don't need your own vfs object. The undo log can be written to even if only the working copy or bookmarks change (so no store lock was ever taken), so it seems like it would live at the .hg level instead of the store level?

In D112#1789, @durham wrote:

Why not just use the normal vfs? Then you don't need your own vfs object. The undo log can be written to even if only the working copy or bookmarks change (so no store lock was ever taken), so it seems like it would live at the .hg level instead of the store level?

Good point. IIRC the choice of svfs was because repo.transaction somehow hardcodes it to track revlogs. Since we have our own transaction now I'll try to use vfs.

quark edited the summary of this revision. (Show Details)Jul 18 2017, 1:28 PM

quark retitled this revision from undo: bypass svfs audit check to undo: move undolog to vfs and mark as lock-safe.

quark updated this revision to Diff 272.

quark added a subscriber: felixmerk.Jul 18 2017, 1:30 PM

If this fixes the test, I'm totes in favor!

This revision is now accepted and ready to land.Jul 18 2017, 1:41 PM

Closed by commit rFBHGX910ef98c3557: undo: move undolog to vfs and mark as lock-safe (authored by quark). · Explain WhyJul 18 2017, 7:10 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			hgext3rd/undo.py (17 lines)
M			tests/test-undo.t (28 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	219		Jul 17 2017, 4:56 PM	★	★
Diff 2	272		Jul 18 2017, 1:28 PM	★	★
Diff 3	291	rFBHGX910ef98c3557a718cabb0171b5ddbc1073cb38e9	Jul 18 2017, 2:30 PM	★	★

Diff 291

hgext3rd/undo.py

	# undo.py: records data in revlog for future undo functionality			# undo.py: records data in revlog for future undo functionality
	#			#
	# Copyright 2017 Facebook, Inc.			# Copyright 2017 Facebook, Inc.
	#			#
	# This software may be used and distributed according to the terms of the			# This software may be used and distributed according to the terms of the
	# GNU General Public License version 2 or any later version.			# GNU General Public License version 2 or any later version.

	from __future__ import absolute_import			from __future__ import absolute_import

	from mercurial.i18n import _			from mercurial.i18n import _

	from mercurial import (			from mercurial import (
	dispatch,			dispatch,
	error,			error,
	extensions,			extensions,
				localrepo,
	lock as lockmod,			lock as lockmod,
	registrar,			registrar,
	revlog,			revlog,
	revset,			revset,
	revsetlang,			revsetlang,
	smartset,			smartset,
	transaction,			transaction,
	util,			util,
	)			)

	from mercurial.node import (			from mercurial.node import (
	bin,			bin,
	hex,			hex,
	nullid,			nullid,
	)			)

	cmdtable = {}			cmdtable = {}
	command = registrar.command(cmdtable)			command = registrar.command(cmdtable)

	# Setup			# Setup

	def extsetup(ui):			def extsetup(ui):
	extensions.wrapfunction(dispatch, 'runcommand', _runcommandwrapper)			extensions.wrapfunction(dispatch, 'runcommand', _runcommandwrapper)

				# undo has its own locking, whitelist itself to bypass repo lock audit
				localrepo.localrepository._wlockfreeprefix.add('undolog/')

	# Wrappers			# Wrappers

	def _runcommandwrapper(orig, lui, repo, cmd, fullargs, *args):			def _runcommandwrapper(orig, lui, repo, cmd, fullargs, *args):
	# This wrapper executes whenever a command is run.			# This wrapper executes whenever a command is run.
	# Some commands (eg hg sl) don't actually modify anything			# Some commands (eg hg sl) don't actually modify anything
	# ie can't be undone, but the command doesn't know this.			# ie can't be undone, but the command doesn't know this.
	command = fullargs			command = fullargs

	True if changes have been recorded, False otherwise			True if changes have been recorded, False otherwise
	'''			'''
	changes = False			changes = False
	if repo is not None:# some hg commands don't require repo			if repo is not None:# some hg commands don't require repo
	# undolog specific lock			# undolog specific lock
	# allows running command during other commands when			# allows running command during other commands when
	# otherwise legal. Could cause weird undolog states,			# otherwise legal. Could cause weird undolog states,
	# which gap handling generally covers.			# which gap handling generally covers.
	with lockmod.lock(repo.vfs, "undologlock",			repo.vfs.makedirs('undolog')
	desc="undolog"):			with lockmod.lock(repo.vfs, "undolog/lock", desc="undolog"):
	# developer config: undo._duringundologlock			# developer config: undo._duringundologlock
	if repo.ui.configbool('undo', '_duringundologlock'):			if repo.ui.configbool('undo', '_duringundologlock'):
	repo.hook("duringundologlock")			repo.hook("duringundologlock")
	tr = lighttransaction(repo)			tr = lighttransaction(repo)
	with tr:			with tr:
	changes = log(repo.filtered('visible'), command, tr)			changes = log(repo.filtered('visible'), command, tr)
	return changes			return changes

	def lighttransaction(repo):			def lighttransaction(repo):
	# full fledged transactions have two serious issues:			# full fledged transactions have two serious issues:
	# 1. they may cause infite loops through hooks			# 1. they may cause infite loops through hooks
	# that run commands			# that run commands
	# 2. they are really expensive performance wise			# 2. they are really expensive performance wise
	#			#
	# ligtthransaction avoids certain hooks from being			# ligtthransaction avoids certain hooks from being
	# executed, doesn't check repo locks, doesn't check			# executed, doesn't check repo locks, doesn't check
	# abandoned tr's (since we only record info) and doesn't			# abandoned tr's (since we only record info) and doesn't
	# do any tag handling			# do any tag handling
	vfsmap = {'plain': repo.vfs}			vfsmap = {'plain': repo.vfs}
	tr = transaction.transaction(repo.ui.warn, repo.svfs, vfsmap,			tr = transaction.transaction(repo.ui.warn, repo.vfs, vfsmap,
	"_undojournal", "_undolog")			"undolog/tr.journal", "undolog/tr.undo")
	return tr			return tr

	def log(repo, command, tr):			def log(repo, command, tr):
	'''logs data neccesary for undo if repo state has changed			'''logs data neccesary for undo if repo state has changed

	input:			input:
	repo: mercurial.localrepo			repo: mercurial.localrepo
	command: los, first is command to be recorded as run			command: los, first is command to be recorded as run
	# changes have been recorded			# changes have been recorded
	return True			return True

	# Write: Logs			# Write: Logs

	def writelog(repo, tr, name, revstring):			def writelog(repo, tr, name, revstring):
	if tr is None:			if tr is None:
	raise error.ProgrammingError			raise error.ProgrammingError
	# the transaction code doesn't work with vfs
	# specifically, repo.recover() assumes svfs?
	repo.svfs.makedirs('undolog')
	rlog = _getrevlog(repo, name)			rlog = _getrevlog(repo, name)
	node = rlog.addrevision(revstring, tr, 1, nullid, nullid)			node = rlog.addrevision(revstring, tr, 1, nullid, nullid)
	return hex(node)			return hex(node)

	def _logdate(repo, tr):			def _logdate(repo, tr):
	revstring = " ".join(str(x) for x in util.makedate())			revstring = " ".join(str(x) for x in util.makedate())
	return writelog(repo, tr, "date.i", revstring)			return writelog(repo, tr, "date.i", revstring)


	# Tools			# Tools

	def _invertindex(rlog, indexorreverseindex):			def _invertindex(rlog, indexorreverseindex):
	return len(rlog) - 1 - indexorreverseindex			return len(rlog) - 1 - indexorreverseindex

	def _getrevlog(repo, filename):			def _getrevlog(repo, filename):
	path = 'undolog/' + filename			path = 'undolog/' + filename
	return revlog.revlog(repo.svfs, path)			return revlog.revlog(repo.vfs, path)

tests/test-undo.t

	$ hg book feature1			$ hg book feature1
	$ touch b && hg add b && hg ci -mb			$ touch b && hg add b && hg ci -mb
	$ hg up -q master			$ hg up -q master
	$ touch c1 && hg add c1 && hg ci -mc1			$ touch c1 && hg add c1 && hg ci -mc1
	created new head			created new head
	$ touch c2 && hg add c2 && hg ci -mc2			$ touch c2 && hg add c2 && hg ci -mc2
	$ hg book feature2			$ hg book feature2
	$ touch d && hg add d && hg ci -md			$ touch d && hg add d && hg ci -md
	$ hg debugindex .hg/store/undolog/command.i			$ hg debugindex .hg/undolog/command.i
	rev offset length delta linkrev nodeid p1 p2			rev offset length delta linkrev nodeid p1 p2
	0 0 0 -1 1 b80de5d13875 000000000000 000000000000			0 0 0 -1 1 b80de5d13875 000000000000 000000000000
	1 0 12 -1 1 440cdcef588f 000000000000 000000000000			1 0 12 -1 1 440cdcef588f 000000000000 000000000000
	2 12 8 -1 1 86fddc37572c 000000000000 000000000000			2 12 8 -1 1 86fddc37572c 000000000000 000000000000
	3 20 8 -1 1 388d40a434df 000000000000 000000000000			3 20 8 -1 1 388d40a434df 000000000000 000000000000
	4 28 14 -1 1 1cafbfad488a 000000000000 000000000000			4 28 14 -1 1 1cafbfad488a 000000000000 000000000000
	5 42 7 -1 1 8879b3bd818b 000000000000 000000000000			5 42 7 -1 1 8879b3bd818b 000000000000 000000000000
	6 49 13 -1 1 b0f66da09921 000000000000 000000000000			6 49 13 -1 1 b0f66da09921 000000000000 000000000000
	7 62 8 -1 1 004b7198dafe 000000000000 000000000000			7 62 8 -1 1 004b7198dafe 000000000000 000000000000
	8 70 8 -1 1 60920018c706 000000000000 000000000000			8 70 8 -1 1 60920018c706 000000000000 000000000000
	9 78 14 -1 1 c3e212568400 000000000000 000000000000			9 78 14 -1 1 c3e212568400 000000000000 000000000000
	10 92 7 -1 1 9d609b5b001c 000000000000 000000000000			10 92 7 -1 1 9d609b5b001c 000000000000 000000000000
	$ hg debugdata .hg/store/undolog/command.i 0			$ hg debugdata .hg/undolog/command.i 0
	$ hg debugdata .hg/store/undolog/command.i 1			$ hg debugdata .hg/undolog/command.i 1
	book\x00master (no-eol) (esc)			book\x00master (no-eol) (esc)
	$ hg debugdata .hg/store/undolog/command.i 2			$ hg debugdata .hg/undolog/command.i 2
	ci\x00-ma1 (no-eol) (esc)			ci\x00-ma1 (no-eol) (esc)
	$ hg debugdata .hg/store/undolog/command.i 3			$ hg debugdata .hg/undolog/command.i 3
	ci\x00-ma2 (no-eol) (esc)			ci\x00-ma2 (no-eol) (esc)
	$ hg debugdata .hg/store/undolog/bookmarks.i 0			$ hg debugdata .hg/undolog/bookmarks.i 0
	$ hg debugdata .hg/store/undolog/bookmarks.i 1			$ hg debugdata .hg/undolog/bookmarks.i 1
	master 0000000000000000000000000000000000000000 (no-eol)			master 0000000000000000000000000000000000000000 (no-eol)
	$ hg debugdata .hg/store/undolog/bookmarks.i 2			$ hg debugdata .hg/undolog/bookmarks.i 2
	master df4fd610a3d6ca792281e7576587fa18f940d37a (no-eol)			master df4fd610a3d6ca792281e7576587fa18f940d37a (no-eol)
	$ hg debugdata .hg/store/undolog/workingparent.i 0			$ hg debugdata .hg/undolog/workingparent.i 0
	0000000000000000000000000000000000000000 (no-eol)			0000000000000000000000000000000000000000 (no-eol)
	$ hg debugdata .hg/store/undolog/workingparent.i 1			$ hg debugdata .hg/undolog/workingparent.i 1
	df4fd610a3d6ca792281e7576587fa18f940d37a (no-eol)			df4fd610a3d6ca792281e7576587fa18f940d37a (no-eol)
	$ hg debugdata .hg/store/undolog/draftheads.i 1			$ hg debugdata .hg/undolog/draftheads.i 1
	df4fd610a3d6ca792281e7576587fa18f940d37a (no-eol)			df4fd610a3d6ca792281e7576587fa18f940d37a (no-eol)
	$ hg debugdata .hg/store/undolog/draftheads.i 2			$ hg debugdata .hg/undolog/draftheads.i 2
	b68836a6e2cac33ba33a20249b85a486eec78186 (no-eol)			b68836a6e2cac33ba33a20249b85a486eec78186 (no-eol)
	$ hg debugdata .hg/store/undolog/index.i 1			$ hg debugdata .hg/undolog/index.i 1
	bookmarks 8153d44860d076e9c328951c8f36cf8daebe695a			bookmarks 8153d44860d076e9c328951c8f36cf8daebe695a
	command 440cdcef588f9a594c5530a5b6dede39a96d930d			command 440cdcef588f9a594c5530a5b6dede39a96d930d
	date * (glob)			date * (glob)
	draftheads b80de5d138758541c5f05265ad144ab9fa86d1db			draftheads b80de5d138758541c5f05265ad144ab9fa86d1db
	workingparent fcb754f6a51eaf982f66d0637b39f3d2e6b520d5 (no-eol)			workingparent fcb754f6a51eaf982f66d0637b39f3d2e6b520d5 (no-eol)
	$ touch a3 && hg add a3			$ touch a3 && hg add a3
	$ hg commit --amend			$ hg commit --amend
	saved backup bundle to $TESTTMP/repo/.hg/strip-backup/db92053d5c83-e25f6bc1-amend.hg (glob)			saved backup bundle to $TESTTMP/repo/.hg/strip-backup/db92053d5c83-e25f6bc1-amend.hg (glob)
	$ hg debugdata .hg/store/undolog/command.i 11			$ hg debugdata .hg/undolog/command.i 11
	commit\x00--amend (no-eol) (esc)			commit\x00--amend (no-eol) (esc)

	Test debugundohistory			Test debugundohistory
	$ hg debugundohistory -l			$ hg debugundohistory -l
	0: commit --amend			0: commit --amend
	1: ci -md			1: ci -md
	2: book feature2			2: book feature2
	3: ci -mc2			3: ci -mc2