This is an archive of the discontinued Mercurial Phabricator instance.

Differential D11784

update: filter the ambiguous mtime in update directly
ClosedPublic

Authored by marmoute on Nov 24 2021, 6:11 AM.

Details

Reviewers
Alphare
Group Reviewers
hg-reviewers
Commits
rHG6dc3dc5e9636: update: filter the ambiguous mtime in update directly
Summary

Right now, this filtering is done by dirstate.write using the time of
dirstate.write method call. However that filtering is done "too late"
It works "fine" as most command are "fast enough", and race rare enough.
We are about to change the mtime filtering logic in the dirstate to be more
accurate and reliable.

However hg update will still need such filtering (mostly because it is
actually quite racy, even with the existing filtering). So we explicitly
implement a similar logic here. Before removing the older one later in the
series.

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

marmoute created this revision.Nov 24 2021, 6:11 AM
Alphare requested changes to this revision.Nov 24 2021, 8:18 AM
Alphare added a subscriber: Alphare.
Alphare added inline comments.
mercurial/merge.py
2190

Not sure what this sentence was supposed to be

This revision now requires changes to proceed.Nov 24 2021, 8:18 AM
marmoute edited the summary of this revision. (Show Details)Nov 30 2021, 6:53 PM
marmoute updated this revision to Diff 31225.
Alphare accepted this revision.Dec 1 2021, 5:09 AM
This revision is now accepted and ready to land.Dec 1 2021, 5:09 AM
marmoute added a commit: rHG6dc3dc5e9636: update: filter the ambiguous mtime in update directly.Dec 1 2021, 10:21 AM
Closed by commit rHG6dc3dc5e9636: update: filter the ambiguous mtime in update directly (authored by marmoute). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/merge.py (65 lines)

Diff 31246

mercurial/merge.py

# no filedata until mergestate is updated to provide it # no filedata until mergestate is updated to provide it
for a in acts: for a in acts:
getfiledata[a[0]] = None getfiledata[a[0]] = None
assert len(getfiledata) == ( assert len(getfiledata) == (
mresult.len((mergestatemod.ACTION_GET,)) if wantfiledata else 0 mresult.len((mergestatemod.ACTION_GET,)) if wantfiledata else 0
) )
with repo.dirstate.parentchange(): with repo.dirstate.parentchange():
### Filter Filedata
#
# We gathered "cache" information for the clean file while
# updating them: mtime, size and mode.
#
# At the time this comment is written, they are various issues
# with how we gather the `mode` and `mtime` information (see
# the comment in `batchget`).
#
# We are going to smooth one of this issue here : mtime ambiguity.
#
# i.e. even if the mtime gathered during `batchget` was
# correct[1] a change happening right after it could change the
# content while keeping the same mtime[2].
#
# When we reach the current code, the "on disk" part of the
AlphareUnsubmitted
Not Done

Not sure what this sentence was supposed to be

Alphare: Not sure what this sentence was supposed to be
# update operation is finished. We still assume that no other
# process raced that "on disk" part, but we want to at least
# prevent later file change to alter the content of the file
# right after the update operation. So quickly that the same
# mtime is record for the operation.
# To prevent such ambiguity to happens, we will only keep the
# "file data" for files with mtime that are stricly in the past,
# i.e. whose mtime is strictly lower than the current time.
#
# This protect us from race conditions from operation that could
# run right after this one, especially other Mercurial
# operation that could be waiting for the wlock to touch files
# content and the dirstate.
#
# In an ideal world, we could only get reliable information in
# `getfiledata` (from `getbatch`), however the current approach
# have been a successful compromise since many years.
#
# At the time this comment is written, not using any "cache"
# file data at all here would not be viable. As it would result is
# a very large amount of work (equivalent to the previous `hg
# update` during the next status after an update).
#
# [1] the current code cannot grantee that the `mtime` and
# `mode` are correct, but the result is "okay in practice".
# (see the comment in `batchget`). #
#
# [2] using nano-second precision can greatly help here because
# it makes the "different write with same mtime" issue
# virtually vanish. However, dirstate v1 cannot store such
# precision and a bunch of python-runtime, operating-system and
# filesystem does not provide use with such precision␌, so we
# have to operate as if it wasn't available.
if getfiledata:
ambiguous_mtime = {}
now = timestamp.get_fs_now(repo.vfs)
if now is None:
# we can't write to the FS, so we won't actually update
# the dirstate content anyway, no need to put cache
# information.
getfiledata = None
else:
now_sec = now[0]
for f, m in pycompat.iteritems(getfiledata):
if m is not None and m[2][0] >= now_sec:
ambiguous_mtime[f] = (m[0], m[1], None)
for f, m in pycompat.iteritems(ambiguous_mtime):
getfiledata[f] = m
repo.setparents(fp1, fp2) repo.setparents(fp1, fp2)
mergestatemod.recordupdates( mergestatemod.recordupdates(
repo, mresult.actionsdict, branchmerge, getfiledata repo, mresult.actionsdict, branchmerge, getfiledata
) )
# update completed, clear state # update completed, clear state
util.unlink(repo.vfs.join(b'updatestate')) util.unlink(repo.vfs.join(b'updatestate'))
if not branchmerge: if not branchmerge: