This is an archive of the discontinued Mercurial Phabricator instance.

Differential D9677

upgrade: use copy+delete instead of rename while creating backup
Changes PlannedPublic

Authored by pulkit on Dec 31 2020, 11:43 AM.

Details

Reviewers
marmoute
Group Reviewers
hg-reviewers
Summary

A lot of times, we do an upgrade operation which does not touches all the parts
of the stores. But right not, we have a blind logic which processes everything.
To selectively upgrade parts of repository, we need to persist existing data
which is untouched.

However while creating current repository backup, we rename the whole store
leaving no option to persist untouched files.

We switch to copy+delete so that we can only delete data files which are changed
by the operation and leave rest untouched.

Diff Detail

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

Event Timeline

pulkit created this revision.Dec 31 2020, 11:43 AM
marmoute added a subscriber: marmoute.Jan 5 2021, 4:10 AM

I worried about having a much wider inconsistency window if we don't use rename. I'll have a deeper look later.

• ulrickaka added a subscriber: • ulrickaka.Jan 5 2021, 2:04 PM

Help With Essay
from the Academic Experts. Our essay writing service is designed to get you the extra help you need in completing your next university essay

marmoute requested changes to this revision.Jan 7 2021, 9:52 PM

As pointed in a pa previous comment, this is problematic, because this does not preserve the same consistency as the previous code

This revision now requires changes to proceed.Jan 7 2021, 9:52 PM
pulkit added a comment.Jan 8 2021, 11:36 AM
In D9677#146243, @marmoute wrote:

As pointed in a pa previous comment, this is problematic, because this does not preserve the same consistency as the previous code

While the copy consistency can be maintained here, future changes (WIP) will break the consistency more. In upcoming work, we want to only touch parts of repository which needs to be updated. This will lead us to do selectively moving data from the upgraded repository to the current one.

mharbison72 added a subscriber: mharbison72.Jan 12 2021, 8:20 PM
In D9677#146367, @pulkit wrote:
In D9677#146243, @marmoute wrote:

As pointed in a pa previous comment, this is problematic, because this does not preserve the same consistency as the previous code

While the copy consistency can be maintained here, future changes (WIP) will break the consistency more. In upcoming work, we want to only touch parts of repository which needs to be updated. This will lead us to do selectively moving data from the upgraded repository to the current one.

@marmoute are you OK with this explanation?

My question is, would it be better to hardlink and then delete to avoid the copy overhead? (Of course not every filesystem supports that, but I think it falls back to a copy in that case.)

pulkit updated this revision to Diff 24774.Jan 13 2021, 5:31 AM
marmoute requested changes to this revision.Jan 13 2021, 5:34 AM
In D9677#147085, @mharbison72 wrote:
In D9677#146367, @pulkit wrote:
In D9677#146243, @marmoute wrote:

As pointed in a pa previous comment, this is problematic, because this does not preserve the same consistency as the previous code

While the copy consistency can be maintained here, future changes (WIP) will break the consistency more. In upcoming work, we want to only touch parts of repository which needs to be updated. This will lead us to do selectively moving data from the upgraded repository to the current one.

@marmoute are you OK with this explanation?

Not at all :-)

Currently we have:

  1. a fully consistent repository in place (while the upgrade run)
  2. a missing store during a split second (old store was rename)
  3. a fully consistent repository in place (the upgraded store was rename in place)

The process ensure a very narrow inconsistency window and a very clear invalid state "the store is either valid or missing."

The change in this part move the (2) step to a much longer operation where elements get copied one by one. As a result we get a much longer window of inconsistency with a much wonkier inconsistency state as we get "mixed" files within the resulting store.

So overall this change is going in the wrong direction.

We are currently looking for lighter upgrade for 2 feature "persistent nodemap" and "share-safe". And I don't think either of them need this kind of change. A upgrade processs for persistent nodemap could be:

  1. write new persistent nodemaps (data file, then docket) that nobody will read without the requirements.
  2. add the new requirement

The downgrade being the reverse (remove the requirement, remove the nodemaps)

And something similar could be done for the share-safe upgrade

  1. write a new .hg/store/requirements that nobody will read yet
  2. rewrite .hg/requirements
This revision now requires changes to proceed.Jan 13 2021, 5:34 AM
pulkit added a comment.Jan 13 2021, 10:56 AM
In D9677#147172, @marmoute wrote:
In D9677#147085, @mharbison72 wrote:
In D9677#146367, @pulkit wrote:
In D9677#146243, @marmoute wrote:

As pointed in a pa previous comment, this is problematic, because this does not preserve the same consistency as the previous code

While the copy consistency can be maintained here, future changes (WIP) will break the consistency more. In upcoming work, we want to only touch parts of repository which needs to be updated. This will lead us to do selectively moving data from the upgraded repository to the current one.

@marmoute are you OK with this explanation?

Not at all :-)
Currently we have:

  1. a fully consistent repository in place (while the upgrade run)
  2. a missing store during a split second (old store was rename)
  3. a fully consistent repository in place (the upgraded store was rename in place)

The process ensure a very narrow inconsistency window and a very clear invalid state "the store is either valid or missing."
The change in this part move the (2) step to a much longer operation where elements get copied one by one. As a result we get a much longer window of inconsistency with a much wonkier inconsistency state as we get "mixed" files within the resulting store.

Nope, this patch does not do that. We have three stores here: backupstore (containing backup of current store), current store and upgraded store. Below is comparison of split second as part 2) mentioned above.

Before:

  1. Move current store to backup store
  2. Move upgraded store to current store

Store inconsistency starts when step 1) starts and remains till step 2) finishes since both are move operation.

After this patch:

  1. Copy current store to backup store
  2. Move upgraded store to current store

In this one, store inconsistency happens only at step 2) since step 1) is a copy.

So, this patch reduces the time when store was inconsistent.

marmoute added a comment.Jan 13 2021, 4:17 PM

Note: that renaming a directory is instantaneous on any descent file system. copying full hierarchy is not.

I am afraid your explanation is now confusing me. Can you try to explain what this patch is doing again ?

pulkit planned changes to this revision.Jan 21 2021, 9:42 AM

Dropping this from stack for now to focus on getting other work pushed.

Djokovich edited the summary of this revision. (Show Details)Apr 17 2021, 8:59 AM
marmoute edited the summary of this revision. (Show Details)Apr 17 2021, 9:41 AM
Djokovich added a subscriber: Djokovich.Apr 18 2021, 4:20 AM

I have been looking for this information for a long time, I was very surprised when I found it here. 192.168.0.1

udychris009 added a subscriber: udychris009.May 10 2021, 4:22 AM

The Joint Admission and Matriculations Board (JAMB) has opened the portal for checking of admission status for the 2021/2022 academic session. Also see <a href="https://www.currentschoolnews.com/exam/jamb-admission-status-checker/">check JAMB Admission Status</a>

soap added a subscriber: soap.Feb 16 2022, 3:42 PM

Watch Free Online Movies and TV Shows onsoap2day

Revision Contents
Changeset List

Diff 24564

mercurial/upgrade_utils/engine.py

currentrepo: repo object of current repository currentrepo: repo object of current repository
upgradedrepo: repo object of the upgraded data upgradedrepo: repo object of the upgraded data
backupvfs: vfs object for the backup path backupvfs: vfs object for the backup path
upgrade_op: upgrade operation object upgrade_op: upgrade operation object
to be used to decide what all is upgraded to be used to decide what all is upgraded
""" """
# TODO: don't blindly rename everything in store # TODO: don't blindly rename everything in store
# There can be upgrades where store is not touched at all # There can be upgrades where store is not touched at all
util.rename(currentrepo.spath, backupvfs.join(b'store')) backupstorevfs = vfsmod.vfs(backupvfs.join(b'store'))
util.makedirs(backupstorevfs.base)
for path, kind, st in sorted(currentrepo.store.vfs.readdir(b'', stat=True)):
# Skip transaction related files.
if path.startswith(b'undo'):
continue
# Only copy regular files.
if kind != stat.S_IFREG:
continue
# Skip other skipped files.
if path in (b'lock',):
continue
src = currentrepo.store.rawvfs.join(path)
dst = backupstorevfs.join(path)
util.copyfile(src, dst, copystat=True)
if currentrepo.svfs.exists(b'data'):
util.copyfiles(
currentrepo.svfs.join(b'data'),
backupstorevfs.join(b'data'),
hardlink=False,
)
if currentrepo.svfs.exists(b'meta'):
util.copyfiles(
currentrepo.svfs.join(b'meta'),
backupstorevfs.join(b'meta'),
hardlink=False,
)
currentrepo.vfs.rmtree(b'store', forcibly=True)
util.rename(upgradedrepo.spath, currentrepo.spath) util.rename(upgradedrepo.spath, currentrepo.spath)
def finishdatamigration(ui, srcrepo, dstrepo, requirements): def finishdatamigration(ui, srcrepo, dstrepo, requirements):
"""Hook point for extensions to perform additional actions during upgrade. """Hook point for extensions to perform additional actions during upgrade.
This function is called after revlogs and store files have been copied but This function is called after revlogs and store files have been copied but
before the new store is swapped into the original location. before the new store is swapped into the original location.
ui.status( ui.status(
_( _(
b'finalizing requirements file and making repository readable ' b'finalizing requirements file and making repository readable '
b'again\n' b'again\n'
) )
) )
scmutil.writereporequirements(srcrepo, upgrade_op.new_requirements) scmutil.writereporequirements(srcrepo, upgrade_op.new_requirements)
# The lock file from the old store won't be removed because nothing has a
# reference to its new location. So clean it up manually. Alternatively, we
# could update srcrepo.svfs and other variables to point to the new
# location. This is simpler.
backupvfs.unlink(b'store/lock')
return backuppath return backuppath

tests/test-upgrade-repo.t

$ ls -d .hg/upgradebackup.*/ $ ls -d .hg/upgradebackup.*/
.hg/upgradebackup.*/ (glob) .hg/upgradebackup.*/ (glob)
$ ls .hg/upgradebackup.*/store $ ls .hg/upgradebackup.*/store
00changelog.i 00changelog.i
00manifest.i 00manifest.i
data data
fncache fncache
phaseroots phaseroots
undo
undo.backup.fncache
undo.backupfiles
undo.phaseroots
unless --no-backup is passed unless --no-backup is passed
$ rm -rf .hg/upgradebackup.*/ $ rm -rf .hg/upgradebackup.*/
$ hg debugupgraderepo --run --no-backup $ hg debugupgraderepo --run --no-backup
upgrade will perform the following actions: upgrade will perform the following actions:
requirements requirements