Summary: Adding progress bar to slow prefetch process.
Test Plan: Perform a prefetch with :-10 or :-100 and see a progress bar displayed.
Reviewers: hanw, mjpieters
Reviewed By: mjpieters
Subscribers: mjpieters, hanw
Differential Revision: https://phabricator.intern.facebook.com/D4398078
Tasks: 9993344
Signature: t1:4398078:1484071212:f906493464340bea103be66133aa7026284143c7
Summary:
fastannotate will tell remotefilelog what nodes of a file is already known in
linelog + revmap, so remotefilelog will not prefetch those files. Previously,
fastannotate either prevents all prefetching or allows all prefetching, which
is sub-optimal.
fastannotate could now donate its sshpeer to remotefilelog, so remotefilelog
won't need to start another one (assuming they can share a same sshpeer,
could be turned off via config options). This should reduce run time if SSH
handshake is expensive.
fastannotate could now also steal sshpeer from remotefilelog, so fastannotate
won't need to start another one. Combined with the above change, there would
always be only one sshpeer shared by fastannotate and remotefilelog.
Test Plan: Modified existing tests
Reviewers: #sourcecontrol, durham
Reviewed By: durham
Subscribers: ikostia, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4325382
Signature: t1:4325382:1481933531:39d6344b2c076fbbff1f07997cd268e7cee25e80
Summary:
This fixed a user report (P56867550) where the error message does not get
shown correctly, because ResponseError takes two parameters.
Also did a minor change for the first ResponseError to comply the format.
Test Plan: `arc diff`
Reviewers: durham, #sourcecontrol, simonfar
Reviewed By: simonfar
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4280775
Signature: t1:4280775:1481016196:4762161be699e5d215ec86b2d1385493d977a2b6
Summary:
This adds a test for hg repack --incremental handling tree packs. It fixes a few
bugs that were caught in the process:
1. Since remotefilelog was being imported via it's file path, it was being
loaded into the process as hgext_remotefilelog. When treemanifest loaded it into
the process via 'import remotefilelog' it was being imported as 'remotefilelog'.
This meant we had the same types imported into the same process with two
different names, which meant 'isinstance()' checks could fail when they
shouldn't (which affects incremental repacks). So we just drop the filepath.
2. We need to allow repacking local tree manifest data even if the full delta
chain isn't available (since part of the delta chain may be in the cache). So we
add allowincomplete to the data pack in this case.
Test Plan: Ran it
Reviewers: #mercurial, rmcelroy
Reviewed By: rmcelroy
Subscribers: rmcelroy, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4262412
Signature: t1:4262412:1480706110:45bb0a1e1b031f9cfd4658a5071bbac5df2f6543
Summary:
Previously 'hg debug[data|hist]pack' required passing the filepath without the
suffix (just up to the hash). This was kind of a pain when using tab complete,
and a pain in tests when trying to use 'find' to run hg debugdatapack on a
number of files. Let's make the debug commands more forgiving.
Test Plan: Manual verification
Reviewers: #mercurial, dsyang, rmcelroy
Reviewed By: rmcelroy
Subscribers: rmcelroy, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4261586
Signature: t1:4261586:1480705548:1aa5dbe44ed5e29c28a29a8256de12ffee8d7387
Summary:
treemanifest also has the concept of repack and shares the same code as
remotefilelog's repack. We want treemanifest to be usable even without
remotefilelog, so let's update the repack code to not require the presence of
remotefilelog configured members on the repo object.
In the long term we'll probably move the repack and pack code out of
remotefilelog entirely.
Test Plan: A future patch adds a test that caught this
Reviewers: #mercurial, dsyang, rmcelroy
Reviewed By: rmcelroy
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4261571
Signature: t1:4261571:1480705343:2ceabbbbdeaf7408ef8b94ad3e670b9423162399
Summary:
Previously we only supported full repacks of the manifest stores. This patch
adds incremental repacking for both the shared manifest store and the local
manifest store.
Test Plan:
Did a few pulls to produce a few treemanifest pack files. Ran hg
repack --incremental after each one and verified it was a no-op until there were
enough pack files to trigger the repack.
Will add tests later today.
Reviewers: #mercurial, rmcelroy
Reviewed By: rmcelroy
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4260602
Signature: t1:4260602:1480705304:b17d3ec5ff8d4caafe935b2c4e941454052fe3ec
Summary:
Previous hg repack would only repack the shared manifest cache store. This patch
makes it also repack the local manifest store too.
Test Plan:
Made a local commit in my test repo with treemanifests enabled.
Rebased the commit to master so now there were two local tree packs. Ran hg
repack and verified they were combined into one pack in
.hg/store/packs/manifests
I'll add a test later today
Reviewers: #mercurial, mjpieters
Reviewed By: mjpieters
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4260580
Signature: t1:4260580:1480696151:8f989e299dda50281ca63489e870202eb195d714
Summary:
Previously the repack logic was hardcoded to only work on the shared cache
stores. This patch refactors that knowledge to a higher level so a future patch
can also repack the local stores as well.
Test Plan: Ran the tests
Reviewers: #mercurial, mjpieters
Reviewed By: mjpieters
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4260569
Signature: t1:4260569:1480694802:30ab24dd031661227b4da1febc3d3daf9ad598fb
Summary:
If the annotate cache is up-to-date on the main branch, there is likely no need
to prefetch file contents, unless the user is annotating a side branch, which
requires a deeper integration between fastannotate and remotefilelog to just
prefetch the missing part correctly - I'll think about it later while this diff seems
good enough for common cases.
Test Plan: Added a new test
Reviewers: durham, #sourcecontrol, stash
Reviewed By: stash
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4256370
Signature: t1:4256370:1480612767:c2e2a9225fb63ff36ffb579f9641851eb8cd8b39
Summary:
Previously hg repack would only repack file content pack files. This patch makes
it also repack tree manifest pack files.
Test Plan:
Ran pull repack in a repo and verified the manifest packs were
repacked. I'll add some tests around this at some point.
Reviewers: #mercurial
Differential Revision: https://phabricator.intern.facebook.com/D4240723
Summary:
Previously, if there was no history available for a given revision, we would
just store the full text in the pack file. This patch makes it attempt to reuse
the existing delta base instead. This will be useful for repacking
treemanifests, since we currently don't have histpacks for them (we just delta
them efficiently when they are first added to the repo).
Test Plan: Ran tests
Reviewers: #mercurial
Differential Revision: https://phabricator.intern.facebook.com/D4240705
Summary:
Using `testedwith = 'internal'` is not a good habit [1]. Having it
auto-updated in batch would also introduce a lot of churn. This diff makes
them "ships-with-fb-hgext". If we do want to fill the ideal "testedwith"
information, we could put it in a centric place, like a "fbtestedwith"
extension rewriting those "ships-with-fb-hgext" on the fly.
Maybe having in-repo tags for tested Mercurial releases is also a good idea.
[1]: www.mercurial-scm.org/repo/hg/rev/2af1014c2534
Test Plan: `arc lint`
Reviewers: #sourcecontrol, rmcelroy
Reviewed By: rmcelroy
Subscribers: rmcelroy, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4244689
Signature: t1:4244689:1480440027:3dc18d017b48beba1176fbfd120351889259eb4b
Summary:
The debugindex and debugindexdot commands have moved and are not registered
unless you import the new mercurial.debugcommands module.
Test Plan:
Run
hg --config=extensions.remotefilelog=fb-hgext/remotefilelog help
and observe that you get help info, rather that the error
hg: unknown command 'debugindex'
then run the fb-hgext test suite.
Reviewers: rmcelroy, quark, simonfar
Reviewed By: simonfar
Subscribers: mjpieters, #mercurial
Differential Revision: https://phabricator.intern.facebook.com/D4244047
Signature: t1:4244047:1480427216:dcaa1ca441ea189bdf68f1f619b4078d8c1d09dc
Summary:
I'm going to use runshellfast in infinitepush.
To avoid copy-paste let's move it to the separate package.
Note: fastmanifest also has runshellfast but it's implementation
is a bit different and seems that it doesn't work with remotefilelog.
So I'll leave it alone for now.
Test Plan: python run-tests.py -j20
Reviewers: #sourcecontrol
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4175836
Summary:
Upstream has refactored the manifest class, so we need to update remotefilelog
to work with the new structure.
Also fix a mismatch with the new adjustlinkrev signature.
Test Plan: Ran the tests
Reviewers: #mercurial, rmcelroy
Reviewed By: rmcelroy
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4180879
Signature: t1:4180879:1479285946:d5ac954c495dc8b802d276ab5ac71626a1726612
Summary:
The upstream getchangegroup function changed back in August and our attempt at
fixing it here did not correclty handle the case where remotefilelog is loaded
but not enabled for this repository.
This patches fixes the signature, and makes our wrapping more resitant to future
signature changes.
Test Plan: Added a test.
Reviewers: #mercurial, jsgf
Reviewed By: jsgf
Subscribers: jsgf, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4099519
Signature: t1:4099519:1477698801:adcb406f1908c4f7e508e5f99e126f383c6f2574
Summary:
We've gotten reports of hg gc failing on some service machines because
`peer._repo.name` complains that repo has no attribute 'name'. I'm not sure how
this could happen, but it makes sense to make the hg gc loop more robust to the
possibility that the repos in the 'repos' file have changed their configuration
since they were added to the file.
Test Plan: Ran the tests
Reviewers: #mercurial, simonfar
Reviewed By: simonfar
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4072719
Signature: t1:4072719:1477385020:24d532b9442292ce8234cc91bc7de503d3b0c88f
I keep tripping over bugs where this angers some part of our
stack. It's dumb to even be fetching these, but it's harmless to skip
them, so issue a developer warning when we encounter one and refuse to
fetch it.
Summary:
Upstream changed the signature of computenonoverlap. Let's change our wrapping
of it to be more robust to signature changes.
Test Plan: ./run-tests.py test-remotefilelog* test-check*
Reviewers: #mercurial, quark
Reviewed By: quark
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4062705
Tasks: 14037455
Signature: t1:4062705:1477096049:5a011a7a5edf9bb01475694777c738cdb02453f5
Summary:
In a future patch we will start writing user local tree data into a local
directory. This patches add the local store to the union store so the contents
will be accessible once we start writing it.
It also renames manifest to manifests for consistency with the other store names
(like 'packs').
Test Plan: Ran the tests
Reviewers: #mercurial, quark
Reviewed By: quark
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4055827
Signature: t1:4055827:1477059457:0d5b0d999b47d88c73f5ab2721d8d27deacc01bc
Summary:
In a future diff we will be introducing packs into .hg/store, so we need to
differentiate between cache packs and local packs. This patch renames
getpackpath to getcachepackpath. A future diff will add getlocalpackpath.
This exposed a pyflakes error, so we fix that too.
Test Plan: Ran the tests
Reviewers: #mercurial, quark
Reviewed By: quark
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4055815
Signature: t1:4055815:1477059415:e0221557bbeec6701820c826f00390d3a71cd2d3
Summary:
This makes mutablepacks close and abort function accessible from outside the
__exit__ logic. This will be useful later when we tie the lifetime of a
mutablepack to a transaction.
Test Plan: Ran the tests
Reviewers: #mercurial, quark
Reviewed By: quark
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4055730
Signature: t1:4055730:1477059602:ffdfd66e65279ddf3ff43d7c2ee65b00f1fd2600
Summary:
This fixes all the pyflaks and module errors for the main remotefilelog
code base.
Test Plan: ./run-tests.py test-check* test-remotefilelog*
Reviewers: #mercurial, quark
Reviewed By: quark
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4055537
Signature: t1:4055537:1477049663:ee904d311d17d3659e055e2c109c68c9023cfd1f
Summary:
Calculating linkrev can be expensive for remotefilelog, while
non-remotefilelog code would usually expect `fctx.linkrev()` to be very cheap.
So let's cache it.
Test Plan: `arc diff`
Reviewers: #sourcecontrol, durham
Reviewed By: durham
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3936757
Signature: t1:3936757:1475078470:7630d3246a5ca9407456384ae0a9aff1035d5201
Summary:
readfast and readdelta have moved onto the manifestctx structures in upstream
hg, so let's change remotefilelog to use them.
This breaks the ability for this version of remotefilelog to work with old
versions of Mercurial, but we never really guaranteed this to begin with.
Test Plan: Ran the tests, they now pass
Reviewers: #sourcecontrol
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3915785
Summary:
Originally we wanted to use treemanifest.copy so it was explicit when a user was
performing a copy. Unfortunately, pytreemanifest contains a treemanifest as a
field (not a pointer), which means we can't call copy and assign the result to
that location. Let's use a copy constructor instead which lets us perform the
copy using a placement allocation.
There is no change to the consumers of this api, because nothing actually
consumed it. The pytreemanifest code was performing a copy using the default
copy constructor, so things were just broken before.
Test Plan: Later patches worked
Reviewers: #fastmanifest
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3885018
Summary:
FindContext did not initialize any of its values, and some of its callers did
not either. This meant 'invalidate_checksums' was set to a random value, which
meant it was almost always treated as true. Let's initialize the fields.
Test Plan: Later patches worked
Reviewers: #fastmanifest
Differential Revision: https://phabricator.intern.facebook.com/D3885017
Summary:
diffrecurse was still calling the fetcher directly. This caused it to not access
'resolved' manifests correctly in some cases. Let's change all uses to use
entry->get_manifest() which handles this stuff correctly.
Test Plan: Later patches worked
Reviewers: #fastmanifest
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3885016
Summary:
entry->initialize(filename, filenamelen, ...) function also sets the this->resolved
to a new empty Manifest. This has a couple bad side effects. 1) If
this->resolved was already present, it is overwritten and the memory leaks (such
as when entry->update() is called after it has already been resolved. 2) For the
root level manifest, it means when something goes to access it, it doesn't
bother to try to load the actual manifest data from the store. It just serves an
empty manifest instead. So this completely broke reading from the store.
This temporary fix is to handle entry->resolved manually outside the call in a
couple spots (root manifestentry creation, and entry->update). Once some of my
other changes go in, we should come back here and probably remove the "new
Manifest()" from entry->initialize() entirely.
Test Plan: Later patches worked
Reviewers: #fastmanifest
Differential Revision: https://phabricator.intern.facebook.com/D3885015
Summary:
The serialization was completely broken because std::string.append("\0") treated
the argument as a c-string which was completely empty. Let's use the
append(char*, size_t) version instead to make sure it appends the correct
characters.
Also makes othere uses of append in that function match the same pattern.
Test Plan: Later patches worked
Reviewers: #fastmanifest
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3885014
Summary:
The Manifest(PythonObj) constructor parses the given python string and creates a
list of entries for them. During a copy, if the list of entries on the copy
source has been modified to differ from the original PythonObj, this results in
the new copy having incorrect entries. Instead of using the
ManifestEntry(PythonObj) constructor, let's use the default constructor, then
manually fill in the entries ourselves.
We still need to copy the PythonObj over to the new version, since that
increments the reference count and keeps the string alive (since some of our
entries may be referencing it).
This also cleans up some usages of iterators to be less dependent on the
internal side effects of called functions (i.e. don't depend on the called
function to move the iterator forward).
Test Plan: Later patches worked
Reviewers: #fastmanifest
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3885013
Summary:
The old ManifestEntry init used a reference char* so it could move the pointer
to the end of the current read before it return, so the caller could then call
the function again to get the next entry. This caused problems when we tried to
use that initialization function to copy ManifestEntry's later, since it moved
the internal pointer of the entry we were copying.
Let's get rid of the & and make the function just return the pointer to the end
of the read, and the caller can decide to use it or not.
Test Plan: Later patches worked
Reviewers: #fastmanifest
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3885012
Summary:
This is used by markforrefresh, which starts being called in a later
patch.
Test Plan: Later patches worked
Reviewers: #fastmanifest
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3885011
Summary:
This adds a new write() function to treemanifest that allows us to serialize a
treemanifest instance into a provided data store.
Test Plan: A future diff uses this to serialize trees into a pack file.
Reviewers: #fastmanifest
Differential Revision: https://phabricator.intern.facebook.com/D3838787
Summary:
This adds a useful function for performing name comparisons on manifest entries.
This will be used in a future patch to simplify iterating over two manifests at
the same time.
Test Plan:
Used this as part of a future diff that serializes manifests into
pack files.
Reviewers: #fastmanifest
Differential Revision: https://phabricator.intern.facebook.com/D3837743
Summary:
This adds a function for computing the final node of a manifest, given its two
parent nodes.
Test Plan:
Used this as part of a future diff that serialize manifests into a
pack file.
Reviewers: #fastmanifest
Differential Revision: https://phabricator.intern.facebook.com/D3837737
Summary:
This is required to get the proper sorting with characters with the MSb set.
Wrote unit test to cover it.
Test Plan:
1. passed that unit test.
2. `PYTHONPATH=~/work/mercurial/facebook-hg-rpms/fb-hgext/:~/work/mercurial/facebook-hg-rpms/remotenames/:~/work/mercurial/facebook-hg-rpms/lz4revlog/ /opt/local/bin/python2.7 ~/work/mercurial/facebook-hg-rpms/hg-crew/hg --config extensions.treemanifest_correctness=~/work/mercurial/facebook-hg-rpms/fb-hgext/tests/treemanifest_correctness.py --config remotefilelog.fastdatapack=True testtree --build "master~50000::master" --revs 'master + master~5000'` is clean now!
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3869292
Signature: t1:3869292:1473922557:4c691f696ea991a578a151b8091ae44beff528df
Summary:
Mercurial core does this sometimes where it sets the new entries before it removes the old entries. In that case, you might have:
old manifest:
```
abc
```
operations:
```
+ abc/def
- abc
```
If you add `abc/def` to a manifest already containing `abc`, it will throw a `TypeError` today. With this change, we accept this weird state.
Test Plan: Update the tests to cover both versions of this scenario (directory was there first vs file was there first). Pass all tests.
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: durham, mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3865617
Signature: t1:3865617:1473897481:4cf4f122dfd3a3759fe84ea167b8cb0e78238bc2
Summary:
fileiter/stackframe gain a `sorted` field, which it uses to determine whether to use a ManifestIterator or a SortedManifestIterator.
add a unit test to test this scenario
Test Plan: pass unit tests.
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: durham, mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3868088
Signature: t1:3868088:1474225427:64d19f3f58376682850e46a3547e7a6ce1de97f4
Summary:
Mercurial has this odd sort order where the full path is used in the sort order. Therefore 'abc.def' would precede 'abc/def'.
This introduces a comparator that follows this rule. We add a new class, SortedManifestIterator, which returns entries in this order.
Test Plan: compiles
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: durham, mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3862339
Signature: t1:3862339:1473900496:7f6fed3cdba055bb1791fbc8ae08546e24c96f10
Summary: This way, we can have a true nullable (null, a.k.a., not present, and any character).
Test Plan: make local and pass tests
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3855340
Signature: t1:3855340:1473807404:54c1ae17c735665935a52b0140b58094f0b45a26
Summary: `initialize()` does it already.
Test Plan: run unit tests.
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3869524
Signature: t1:3869524:1474225262:57ccfd92a4d02bc92674007bcafaeafedef413a0
Summary: D3842603 broke reads from datapacks because `.initialize()` creates a blank manifest and attaches it to the manifest entry. When we subsequently try to read the root, we don't properly actually resolve it to the manifest in the datapack. Instead, we use that blank manifest which is wrong.
Test Plan: `PYTHONPATH=~/work/mercurial/facebook-hg-rpms/fb-hgext/:~/work/mercurial/facebook-hg-rpms/remotenames/:~/work/mercurial/facebook-hg-rpms/lz4revlog/ /opt/local/bin/python2.7 ~/work/mercurial/facebook-hg-rpms/hg-crew/hg --config extensions.perftest=~/work/mercurial/facebook-hg-rpms/fb-hgext/tests/perftest.py --config remotefilelog.fastdatapack=False testtree --kind flat,ctree,fast --test fulliter --build "master~50000::master" --revs 'master + master~5000'
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: durham, mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3862319
Signature: t1:3862319:1474225268:f01ef876d569bc09c1e0ace71492bbaae017404e
Summary: Any modifying operation sholud set the `invalidate_checksum` flag, and as we unwind the stack, we clear the nodes if the flag is set.
Test Plan: not yet tested
Reviewers: #fastmanifest, durham
Reviewed By: durham
Subscribers: durham, mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3842843
Tasks: 13268688
Signature: t1:3842843:1473709799:89a82ffade1e56fe64ccae50abecd3de4b46f6bd
