Gregory Szorc <gregory.szorc@gmail.com> [Mon, 24 Sep 2018 09:48:02 -0700] rev 39864
wireprotov2server: port to emitrevisions()
We now have a proper storage API to request data on multiple
revisions. We can drop it into wire protocol version 2 with
minimal effort.
The new API handles pretty much everything we were doing manually to
build up the delta request. So we were able to delete a lot of code.
As a bonus, wireprotov2 code is no longer accessing some low-level
storage APIs. This includes the assumption that a node has an
associated numeric revision number! This should make it drastically
simpler to implement a server that doesn't have the concept of
revision numbers.
Differential Revision: https://phab.mercurial-scm.org/D4724
Gregory Szorc <gregory.szorc@gmail.com> [Fri, 21 Sep 2018 14:54:59 -0700] rev 39863
tests: use more complex file storage test
The previous test was attempting to to test delta storage behavior.
It didn't do a very good job at it because there was a good chance
a delta wasn't being used in storage.
Let's switch the test to yield a delta in storage so an upcoming
change to delegate delta logic to storage has the desired effect.
Differential Revision: https://phab.mercurial-scm.org/D4723
Gregory Szorc <gregory.szorc@gmail.com> [Fri, 21 Sep 2018 14:28:21 -0700] rev 39862
revlog: new API to emit revision data
I recently refactored changegroup generation code to make it more
storage agnostic. I made significant progress. But there is still
a bit of work to be done. Specifically:
* Changegroup code is looking at low-level storage attributes to
influence sorting. Sorting should be done at the storage layer.
* The linknode lookup and sorting code for ellipsis is very
complicated.
* Linknodes are just generally wonky because e.g. file storage doesn't
know how to translate a linkrev to a changelog node.
* We regressed performance when introducing the request-response
objects.
Having thought about this problem a bit, I think I've come up with
a better interface for emitting revision deltas.
This commit defines and implements that interface. See the docstring
in repository.py for more info.
This API adds 3 notable features over the previous one.
First, it defers node ordering to the storage implementation in
the common case but allows overriding as necessary. We have a
facility for requesting an exact ordering (used in ellipsis
mode). We have another facility for storage order (used for changelog).
Second, we have an argument specifying assumptions about parents
revisions. This can be used to force a fulltext revision when we
don't know the receiver has a parent revision to delta against.
Third, we can control whether revision data is emitted. This makes
the API suitable as a generic "index data retrieval" API as well
as for producing revision deltas - possibly in the same operation!
The new API is much simpler: we no longer need a complicated "request"
object to encapsulate the delta generation request. I'm optimistic
this will restore performance loss associated with
emitrevisiondeltas().
Storage unit tests for the new API have been implemented.
Future commits will port existing consumers of emitrevisiondeltas()
to the new API then remove emitrevisiondeltas().
Differential Revision: https://phab.mercurial-scm.org/D4722
Gregory Szorc <gregory.szorc@gmail.com> [Mon, 24 Sep 2018 09:41:42 -0700] rev 39861
changegroup: remove reordering control (BC)
This logic - including the experimental bundle.reorder option -
was originally added in
a8e3931e3fb5 in 2011 and then later ported
to changegroup.py.
The intent of this option and associated logic is to control
the ordering of revisions in deltagroups in changegroups. At the
time it was implemented, only changegroup version 1 existed
and generaldelta revlogs were just coming into the world. Changegroup
version 1 requires that deltas be made against the last revision
sent over the wire. Used with generaldelta, this created an
impedance mismatch of sorts and resulted in changegroup producers
spending a lot of time recomputing deltas.
Revision reordering was introduced so outgoing revisions would be
sent in "generaldelta order" and producers would be able to
reuse internal deltas from storage.
Later on, we introduced changegroup version 2. It supported denoting
which revision a delta was against. So we no longer needed to
sort outgoing revisions to ensure optimal delta generation from the
producer. So, subsequent changegroup versions disabled reordering.
We also later made the changelog not store deltas by default. And
we also made the changelog send out deltas in storage order. Why we
do this for changelog, I'm not sure. Maybe we want to preserve revision
order across clones? It doesn't really matter for this commit.
Fast forward to 2018. We want to abstract storage backends. And having
changegroup code require knowledge about how deltas are stored
internally interferes with that goal.
This commit removes reordering control from changegroup generation.
After this commit, the reordering behavior is:
* The changelog is always sent out in storage order (no behavior
change).
* Non-changelog generaldelta revlogs are reordered to always be in DAG
topological order (previously, generaldelta revlogs would be emitted
in storage order for version 2 and 3 changegroups).
* Non-changelog non-generaldelta revlogs are sent in storage order (no
behavior change).
* There exists no config option to override behavior.
The big difference here is that generaldelta revlogs now *always* have
their revisions sorted in DAG order before going out over the wire. This
behavior was previously only done for changegroup version 1. Version 2
and version 3 changegroups disabled reordering because the interchange
format supported encoding arbitrary delta parents, so reordering wasn't
strictly necessary.
I can think of a few significant implications for this change.
Because changegroup receivers will now see non-changelog revisions
in DAG order instead of storage order, the internal storage order of
manifests and files may differ substantially between producer and
consumer. I don't think this matters that much, since the storage
order of manifests and files is largely hidden from users. Only
the storage order of changelog matters (because `hg log` shows the
changelog in storage order). I don't think there should be any
controversy here.
The reordering of revisions has implications for changegroup producers.
Previously, generaldelta revlogs would be emitted in storage order.
And in the common case, the internally-stored delta could effectively
be copied from disk into the deltagroup delta. This meant that emitting
delta groups for generaldelta revlogs would be mostly linear read I/O.
This is desirable for performance. With us now reordering generaldelta
revlog revisions in DAG order, the read operations may use more random
I/O instead of sequential I/O. This could result in performance
loss. But with the prevalence of SSDs and fast random I/O, I'm not
too worried. (Note: the optimal emission order for revlogs is actually
delta encoding order. But the changegroup code wasn't doing that before
or after this change. We could potentially implement that in a later
commit.)
Changegroups in DAG order will have implications for receivers.
Previously, receiving storage order might mean seeing a number of
interleaved branches. This would mean long delta chains, sparse
I/O, and possibly more fulltext revisions instead of deltas, blowing
up storage storage. (This is the same set of problems that sparse
revlogs aims to address.) With the producer now sending revisions in DAG
order, the receiver also stores revisions in DAG order. That means
revisions for the same DAG branch are all grouped together. And this
should yield better storage outcomes. In other words, sending the
reordered changegroup allows the receiver to have better storage
order and for the producer to not propagate its (possibly sub-optimal)
internal storage order.
On the mozilla-unified repository, this change influences bundle
generation:
$ hg bundle -t none-v2 -a
before: time: real 355.680 secs (user 256.790+0.000 sys 16.820+0.000)
after: time: real 382.950 secs (user 281.700+0.000 sys 17.690+0.000)
before: 7,150,228,967 bytes (uncompressed)
after: 7,041,556,273 bytes (uncompressed)
before: 1,669,063,234 bytes (zstd l=3)
after: 1,628,598,830 bytes (zstd l=3)
$ hg unbundle
before: time: real 511.910 secs (user 466.750+0.000 sys 32.680+0.000)
after: time: real 487.790 secs (user 443.940+0.000 sys 30.840+0.000)
00manifest.d size:
source: 274,924,292 bytes
before: 304,741,626 bytes
after: 245,252,087 bytes
.hg/store total file size:
source: 2,649,133,490
before: 2,680,888,130
after: 2,627,875,673
We see the bundle size drop. That's probably because if a revlog
internally isn't storing a delta, it will choose to delta against
the last emitted revision. And on repos with interleaved branches
(like mozilla-unified), the previous revision could be an unrelated
branch and therefore be a large delta. But with this patch, the
previous revision is likely p1 or p2 and a delta should be small.
We also see the manifest size drop by ~50 MB. It's worth noting that
the manifest actually *increased* in size by ~25 MB in the old
strategy and decreased ~25 MB from its source in the new strategy.
Again, my explanation for this is that the DAG ordering in the
changegroup is resulting in better grouping of revisions in the
receiver, which results in more compact delta chains and higher
storage efficiency.
Unbundle time also dropped. I suspect this is due to the revlog having
to work less to compute deltas since the incoming deltas are more
optimal. i.e. the receiver spends less time resolving fulltext
revisions as incoming deltas bounce around between DAG branches and
delta chains.
We also see bundle generation time increase. This is not desirable.
However, the regression is only significant on the original repository:
if we generate a bundle from the repository created from the new,
always reordered bundles, we're close to baseline (if not at it with
expected noise):
$ hg bundle -t none-v2 -a
before (original): time: real 355.680 secs (user 256.790+0.000 sys 16.820+0.000)
after (original): time: real 382.950 secs (user 281.700+0.000 sys 17.690+0.000)
after (new repo): time: real 362.280 secs (user 260.300+0.000 sys 17.700+0.000)
This regression is a bit worrying because it will impact serving
canonical repositories (that don't have optimal internal storage
unless they are reordered - possibly as part of running
`hg debugupgraderepo`). However, this regression will only be
noticed by very large changegroups. And I'm guessing/hoping that
any repository that large is using clonebundles to mitigate server
load.
Again, sending DAG order isn't the optimal send order for servers:
sending in storage-delta order is. But in order to enable
storage-optimal send order, we'll need a storage API that handles
sorting. Future commits will introduce such an API.
Differential Revision: https://phab.mercurial-scm.org/D4721
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 20 Sep 2018 19:31:07 -0700] rev 39860
filelog: drop index attribute (API)
The previous commit removed the last consumer of the "index" attribute
on the file storage interface. The index is an extremely low-level
data structure that is revlog specific and isn't appropriate to
expose as part of a generic storage API. There may be a market for
an efficient data structure to obtain metadata on every revision for
a file. But if there is, it should be designed using e.g. named
attributes for lookup instead of a list-like of 8-tuples.
Let's drop the attribute from filelog and remove the attribute from
the file storage interface.
Differential Revision: https://phab.mercurial-scm.org/D4720
Gregory Szorc <gregory.szorc@gmail.com> [Mon, 24 Sep 2018 09:38:27 -0700] rev 39859
upgrade: use rawsize() instead of revlog index
The revlog index is a very low-level data structure and it shouldn't
be exposed to the storage interface - at least not in its current
form.
upgrade.py is the only consumer of the index attribute on file storage
in the repository.
This commit rewrites that final consumer to use rawsize() instead of
going through the index. This is actually the more proper API to use,
as rawsize() will accurately report the size of revisions which have
a negative size in the index.
Differential Revision: https://phab.mercurial-scm.org/D4719
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 20 Sep 2018 19:20:01 -0700] rev 39858
manifest: add rawsize() proxy (API)
I'm not keen about doing this. But it unblocks efforts to remove
"index" from the file storage interface. We will probably remove this
once we have a better upgrade API in place.
Differential Revision: https://phab.mercurial-scm.org/D4718
Gregory Szorc <gregory.szorc@gmail.com> [Mon, 24 Sep 2018 09:37:19 -0700] rev 39857
upgrade: report size of backing files, not internal storage size
upgrade.py is the only consumer of filelog.index, which I'd like
to eliminate from the file storage interface.
This commit changes the upgrade code to report the storage size
of files by looking at the size of the files backing its storage
instead of looking at the index.
I'm not convinced the approach in this patch will live very long
because it is relying on low-level attributes like "opener" and
"files," which may behave very differently on non-revlog storage.
But the data is only used for reporting purposes and it does get
us one step closer to eliminating "index."
A side-effect of this change is we now report the size of the revlog
index data - not just the revision data. I think this is more
accurate.
Differential Revision: https://phab.mercurial-scm.org/D4717
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 20 Sep 2018 18:07:42 -0700] rev 39856
filelog: store filename directly on revlog instance
This attribute is only used by LFS. It is used by one of the revlog
flag processor functions, which gets an instance of the revlog - not
the file storage type. So, it makes sense to store this attribute on
the revlog instead of the filelog.
With this change, I'm pretty confident that LFS is no longer directly
accessing file storage interface members that are revlog centric. i.e.
it gets us one step closer to eliminating revlog-centric APIs from the
file storage interface!
Differential Revision: https://phab.mercurial-scm.org/D4715
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 20 Sep 2018 17:47:34 -0700] rev 39855
lfs: access revlog directly
LFS is monkeypatching filelog.filelog and is then accessing
various filelog attributes in the monkeypatched function. This is all
fine.
But some of the attributes being accessed by LFS are revlog centric
and shouldn't be exposed on the file storage interface.
This commit changes the monkeypatched functions to access proxied
attributes on self._revlog instead of self.
This should be safe to do because non-revlog repositories should not
be using filelog instances: instead they should have a separate class
to represent file storage. So it is reasonable for LFS to assume the
_revlog attribute exists and points to a revlog.
Differential Revision: https://phab.mercurial-scm.org/D4714
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 20 Sep 2018 15:30:00 -0700] rev 39854
largefiles: automatically load largefiles extension when required (BC)
This is very similar to what we just did for LFS but for largefiles.
See recent commit messages for the rationale here.
Differential Revision: https://phab.mercurial-scm.org/D4713
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 20 Sep 2018 15:18:13 -0700] rev 39853
lfs: don't add extension to hgrc after clone or share (BC)
Now that repository loading in core supports automatically loading
the lfs extension when the "lfs" requirement is present, we no
longer need to update the .hg/hgrc of newly-created repos to load
the lfs extension!
I'm marking this as BC because it is a change in behavior. But users
should not notice unless they create an LFS repo with new Mercurial
and then attempt to use it with an old versions that doesn't support
automatic extension loading.
Differential Revision: https://phab.mercurial-scm.org/D4712
Gregory Szorc <gregory.szorc@gmail.com> [Thu, 20 Sep 2018 15:06:43 -0700] rev 39852
localrepo: automatically load lfs extension when required (BC)
If an unrecognized requirement is present (possibly due to an unloaded
extension), the user will get an error message telling them to go to
https://mercurial-scm.org/wiki/MissingRequirement for more info.
And some requirements clearly map to known extensions shipped by
Mercurial.
This commit teaches repository loading to automatically map
requirements to extensions. We implement support for loading the
lfs extension when the "lfs" requirement is present.
This behavior feels more user-friendly to me and I'm having trouble
coming up with a compelling reason to not do it. The strongest
argument I have against is that - strictly speaking - requirements
are general repository features and there could be N providers of that
feature. e.g. in the case of LFS, there could be another extension
implementing LFS support. And the user would want to use this
non-official extension rather than the built-in one. The way this
patch implements things, the non-official extension could be
missing and Mercurial would load the official lfs extension, leading
to unexpected behavior. But this feels like a highly marginal use
case to me and doesn't outweigh the user benefit of "it just works."
If someone really wanted to e.g. use a custom LFS extension, they
could prevent the built-in one from being loaded by either defining
"extensions.lfs=/path/to/custom/extension" or "extensions.lfs=!",
as the automatic extension loading only occurs if there is no config
entry for that extension.
Differential Revision: https://phab.mercurial-scm.org/D4711
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 19 Sep 2018 13:48:59 -0700] rev 39851
lfs: add repository feature denoting the use of LFS
Whether LFS is enabled seems like a useful feature to expose. This
will also facilitate some future work around LFS feature compatibility.
Differential Revision: https://phab.mercurial-scm.org/D4710
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 19 Sep 2018 14:36:57 -0700] rev 39850
localrepo: define "features" on repository instances (API)
There are a handful of attributes/methods on repository instances that
describe the behavior of the repository. Furthermore, there is
an unbound set of repository descriptors that we may wish to expose.
For example, an extension may wish to add a descriptor and have
monkeypatched functions look for the presence of an attribute before
taking actions.
This commit introduces a "features" mechanism to allow repositories
to self-advertise an arbitrary set of strings that describe repository
behavior or capabilities.
We implement basic support for advertising a few features to give an
idea of what I want to use this for.
Differential Revision: https://phab.mercurial-scm.org/D4709
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 19 Sep 2018 17:27:37 -0700] rev 39849
localrepo: support writing shared file (API)
Now that we can create a shared repository via creation options, we
can handle other special actions related to share at repo creation time
as well.
One of the things we do after creating a shared repository is write
out a .hg/shared file containing the list of additional things to
share. Of which only "bookmarks" is supported.
We add a creation option to hold the set of additional items to
share. If items are defined, we write out the .hg/shared file at
repo creation time.
As part of this, we no longer hold the repo lock when writing the
file. I'm pretty sure we don't care about the tiny race condition
window. I'm also pretty sure the reason we used the lock was because
the vfs auditor on the repo instance complained otherwise. Since the
repo creation code doesn't have an audited vfs, we don't need to
appease it.
Because we no longer need to tell the post share hook what items
are shared, the "bookmarks" argument to that function has been
dropped, incurring an API change.
Differential Revision: https://phab.mercurial-scm.org/D4708
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 19 Sep 2018 17:05:59 -0700] rev 39848
localrepo: support shared repo creation
Previously, hg.share() had its own logic for creating a new
repository on the filesystem.
With the recent introduction of the createopts dict for passing
options to influence repository creation, it is now possible
to consolidate the repo creation code for both the normal and
shared use cases.
This commit teaches the repo creation code in localrepo to
recognize when we're creating a shared repo and to act
appropriately.
Meaningful behavior should be identical. However, there are a
few subtle changes:
* The .hg/requires file is written out in sorted order (rather
than having share-related requirements appended at end).
* The .hg directory is created with notindexed=True when a shared
repo is being created.
Differential Revision: https://phab.mercurial-scm.org/D4707
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 19 Sep 2018 16:51:57 -0700] rev 39847
localrepo: validate directories before creating any
There is no meaningful change in behavior because wdir would
already exist in the case where we raised RepoError. But I think
the code is easier to read if we do all validation first then
take actions with side-effects.
Differential Revision: https://phab.mercurial-scm.org/D4706
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 19 Sep 2018 16:15:22 -0700] rev 39846
localrepo: add missing join()
Differential Revision: https://phab.mercurial-scm.org/D4705
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 19 Sep 2018 11:38:05 -0700] rev 39845
revlog: use proper version comparison during verify
Verify appears to want to compare the changelog's revlog version
number with the version number of filelogs and error if they are
different. But what it was actually doing was comparing the full
32-bit header integer, which contains 2 shorts: 1 for the revlog
version number and 1 for feature flags.
This commit tweaks the verification code so it only looks at the
version number component of the header and emits a warning if they
differ.
The new code is more robust because it accounts for future revlog
version numbers without them needing to be special cased.
Differential Revision: https://phab.mercurial-scm.org/D4704
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 19 Sep 2018 11:22:56 -0700] rev 39844
filelog: stop proxying checksize() (API)
This was only used by verify code. And the check using it is now
implemented as part of verifyintegrity(). The method is unused
and is revlog-centric, which means it isn't appropriate for the file
storage interface. So remove it.
Differential Revision: https://phab.mercurial-scm.org/D4703
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 19 Sep 2018 11:20:02 -0700] rev 39843
filelog: remove version attribute (API)
This was only used by verify code. The check it was used for is now
implemented as part of the verifyintegrity() implementation. The
attribute is now unused, is revlog-specific, and isn't appropriate
to be exposing on the file storage interface. So drop it.
Differential Revision: https://phab.mercurial-scm.org/D4702
Gregory Szorc <gregory.szorc@gmail.com> [Wed, 19 Sep 2018 11:17:28 -0700] rev 39842
verify: start to abstract file verification
Currently, the file storage interface has a handful of attributes
that are exclusively or near-exclusively used by repo verification
code. In order to support verification on non-revlog/alternate
storage backends, we'll need to abstract verification so it can
be performed in a storage-agnostic way.
This commit starts that process.
We establish a new verifyintegrity() method on revlogs and expose
it to the file storage interface. Most of verify.verifier.checklog()
has been ported to this new method.
We need a way to represent verification problems. So we invent an
interface to represent a verification problem, invent a revlog type
to implement that interface, and use it.
The arguments to verifyintegrity() will almost certainly change in
the future, once more functionality is ported from the verify code.
And the "revlogv1" version check is very hacky. (The code in verify
is actually buggy because it is comparing the full 32-bit header
integer instead of just the revlog version short. I'll likely fix
this in a subsequent commit.)
Differential Revision: https://phab.mercurial-scm.org/D4701
Gregory Szorc <gregory.szorc@gmail.com> [Mon, 24 Sep 2018 08:58:57 -0700] rev 39841
unionrepo: remove _constructmanifest()
It is unused after a previous refactor. Spotted in D4641.
Differential Revision: https://phab.mercurial-scm.org/D4700