As far as I can tell, 'nodechunk' is internal to remotefilelog (i.e. this
should not be called by mercurial directly) and every callsite has nodes
instead of revisions here.
Summary:
Sometimes the cache directory has wrong group set and our hg code fails with
permission errors. Try to solve that by detecting wrong groups or modes and
reset them.
Test Plan:
```
In [2]: from remotefilelog import shallowutil
In [5]: ui.setconfig('remotefilelog', 'cachegroup', 'kvm')
In [6]: shallowutil.mkstickygroupdir(ui, '/tmp/d1/d2')
# make sure /tmp/d1, /tmp/d1/d2 have group=kvm and the sticky bit set.
# run `sudo chown -R quark:quark /tmp/d1`
# run `sudo chmod g-s -R /tmp/d1`
In [7]: shallowutil.mkstickygroupdir(ui, '/tmp/d1/d2')
# make sure /tmp/d1/d2 is owend by "kvm" group and has the sticky bit
# again, while /tmp/d1 remains unchanged.
```
Reviewers: #sourcecontrol, stash
Reviewed By: stash
Subscribers: stash, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4701627
Tasks: 16473317
Signature: t1:4701627:1489481138:64a6038ec5cc067cf05bad3b14ee9985e0bf6d96
Summary:
This code that reused deltas if the delta parent wasn't available was bugged
because it meant you could end up with a cycle in the delta chains. This was an
old optimization from before trees had history, so let's drop the optimization
(since trees now have history and can be correctly repacked).
Test Plan:
Ran repack on a packfile that previously caused cycles. Verified the
new version did not with `hg debugdatapack foo.datapack'
Reviewers: #mercurial
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4724520
Summary:
This adds ui.log() output for prefetch statistics. Extensions who hook into
ui.log() can now log this data to external metrics systems.
Test Plan:
Ran a hg prefetch with the config flags enabled, while ptailing the dev command
timer. Verified the result contained remotefilelogfetches*
```
CHGDISABLE=1 FB_HG_DIAGS=1 hg --config
extensions.remotefilelog=../fb-hgext/remotefilelog/ --config
sampling.key.remotefilelog.prefetch=perfpipe_dev_command_timers prefetch -r .~9
ptail -f perfpipe_dev_command_timers | grep durham
```
Reviewers: #mercurial, simonfar
Reviewed By: simonfar
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4711096
Signature: t1:4711096:1489591144:1c91a4fbd118a3c10c2a2c68391c9f5b0dbcedf3
Summary:
Previously, tree repacks did not take into account tree history. It would just
look at the delta base and if the base existed, it would just reuse the delta.
This would A) result in very long chains, and B) result in chains where the full
text was the oldest version, instead of the newest (recent full texts means
faster access to recent versions).
This patch threads tree history into the repacker, which already knows how to
use history for repacks.
Test Plan:
Updated the tests, and inspected the new test results to ensure tree
entries that were not deltas before the repack became reverse deltas during the
repack.
Reviewers: #mercurial, simonfar
Reviewed By: simonfar
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4647359
Signature: t1:4647359:1488882710:dba72cf488766ce827b7641735164fa0efc9a303
Summary:
To support treemanifests in history packs we need to support the empty filename
(i.e. the root of the repo). This removes some checks that prevented that from
working.
Test Plan:
A future patch will add history pack support for treemanifests,
including tests that cover this.
Reviewers: #mercurial, quark
Reviewed By: quark
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4637917
Signature: t1:4637917:1488429178:c25d03b73eb379d4126ebbcee4bb5797f7b841b2
Summary:
Adds a treemanifest.usecunionstore config flag for enabling and disabling use of
the native code uniondatapackstore.
Since we haven't implemented the repack APIs on the native datapack stores, we
currently have to force repack to use the old python implementations. Instead of
trying to expose just the appropriate APIs through the python interface, I think
we'll rewrite all of repack to be in C++ at a future time, since we can take
advantage of parallelism, etc.
Test Plan:
Updated test-treemanifest.t to use the c datapackstore. Also run all
the tests with --extra-config-opt=treemanifest.usecunionstore=True.
These tests caught a missing null check in the C++ code as well.
Reviewers: #mercurial, simonfar
Reviewed By: simonfar
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4609795
Signature: t1:4609795:1488365341:203362db5f470b613c4d6484686cd32c3fa8458f
Summary:
This makes the `_getfiles` batch size configurable. Let me know if some other config name will serve this purpose better.
**Reason for this fix**
Currently, `_getfile` will write 10000 lines of text into a pipe and only upon the success of this operation, will read file blobs from another pipe. Serving process will start writing file blobs into a second pipe as soon as it sees something in the first pipe. Second pipe's buffer will fill up as it is not read from by the client until client writes 10K file requests. 10K file requests fill the buffer of the first pipe and we have a deadlock.
Ideally, we should make client check whether it can write to the first pipe and if not, go and read from the second pipe, but that is a bigger fix.
Test Plan:
- run local tests, see them all passing
- except for `test-cstore.t`, but it fails for me without my changes as well
- this generally makes sense
Reviewers: #sourcecontrol, durham
Reviewed By: durham
Subscribers: durham, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4620152
Signature: t1:4620152:1488221258:04555177926d129c6ba41bc982ad4e913cb31b20
Summary:
As part of unifying our storage layer into a single library, let's move
py-cdatapack into the new cstore directory. Future patches will move
ctreemanifest and the upcoming datapackstore into here as well.
py-cdatapack.h required some reordering since it seems forward declarations work
a little differently between C and C++. There were no code changes though,
except one int->size_t fix.
Test Plan: Ran the tests
Reviewers: #mercurial, simonfar
Reviewed By: simonfar
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4581320
Signature: t1:4581320:1487788968:e8a34c7a03a16db282214c7dd476b749b92a1bfa
Summary:
`mercurial.revset` was recently split into `revset + revsetlang + smartset`.
Update our code accordingly.
D4604848 has fixed the `revsetlang` part. This patch fixes the remaining
`smartset` part.
Also fixes some test failures introduced by D4547080.
Test Plan: `arc unit`
Reviewers: #mercurial
Subscribers: jeroenv, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4595417
I would like to make an extension that replaces one of the layers (by replacing
makecachestores) with a layer that reads from an existing machine-local caching
system. This would speed up my accesses a lot and reduce a lot of duplication
on my disk.
Summary: Many operations that did not require access to histpacks were constructing them regardless. By postponing histpack initialization until we require access to it, we save up to 55ms on such operations.
Test Plan:
1. Stat profiler no longer shows ~6.5% time spent in basepack constructor during `hg book`. Also no longer shows up in the `hg show` profile.
2. Timed how much time was spent in basepack constructor, this was up to 55ms (mostly around 35ms).
3. Ran tests, they all pass.
Reviewers: #sourcecontrol, durham, quark, simonfar
Reviewed By: simonfar
Subscribers: simonfar, quark, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4500999
Signature: t1:4500999:1486502571:cda6cfb7be54eacb341b58fbff209113ea9cf670
Summary:
Sometimes we pass in two revs because we're creating a merge commit.
Let's handle that case better.
Test Plan: tests still pass
Reviewers: #mercurial, durham
Reviewed By: durham
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4528594
Signature: t1:4528594:1486576897:6bd80017dfe2933e4c1a9685ba35bb1af0440eb0
Summary:
See comment in code for details. This addresses a performance
problem that arises when the same remotefilelog repo makes two changes
to a file far apart in history with no other repo making changes to the
same file in between.
Test Plan: new test verifies that linkrevs are fixed up using remote blob
Reviewers: #mercurial, durham
Reviewed By: durham
Subscribers: stash, quark, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4475183
Tasks: 12958591
Signature: t1:4475183:1486576777:81f4f9815cb677c94954c23ecd8aa7edb006ffd9
Summary:
We will be reusing this logic in the next patch; for now we're simply
factoring it out of the main loop
Test Plan: tests continue to pass
Reviewers: #mercurial, durham, stash
Reviewed By: stash
Subscribers: stash, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4528290
Tasks: 12958591
Signature: t1:4528290:1486567826:cb17e2fa1ab71f491482eba1a88c107a18333781
Summary:
The remotefilelog extension previously unconditionally added a sparsematch()
method to repository objects, which just called super.sparsematch() if this is
a sparse repository, and returned None if this isn't a sparse repository.
However, defining a sparsematch() method that returns None confuses the sparse
extension. The sparse extension expects that all repositories that define
sparesmatch() are actually sparse repositories, and never expects this method
to return null.
This updates the remotefilelog code to call its method maybesparsematch()
instead.
Test Plan:
Confirmed existing remotefilelog unit tests pass, and that the sparse extension
no longer crashes when it is used with non-sparse repositories. (This happens
when using the share extension, if the current working directory is non-sparse
but the sharedpath repository configuration loads the sparse extension.)
Reviewers: tja, durham
Reviewed By: durham
Subscribers: net-systems-diffs@, yogeshwer, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4466108
Signature: t1:4466108:1485451483:287a519151e35bdb99f5be0d9287b4698386183e
Summary:
On Windows, the default SSH client shipped with TortoiseHg is TortoisePlink.
It is very slow and for some reason, we did not experiment deadlocks using this
client. (My unverified hypothesis is that it may have an internal buffer to
manage large requests).
Using another SSH client such as MSYS ssh.exe (shipped with git Windows
installer) dramatically speeds up tranfers (~8 times). However, using this client
yields the deadlock problem.
NOTE: I don't know why it's not a problem on linux boxes.
The fix is to issue requests from a separate thread, so we don't enter a
deadlock on the client.
Summary:
this is neccessary for the new metaedit to not spend time in
remotefilelog
Test Plan: tests are passing
Reviewers: #sourcecontrol, rmcelroy, durham
Reviewed By: durham
Subscribers: durham, rmcelroy, quark, jeroenv, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4326589
Tasks: 14548013
Signature: t1:4326589:1485343332:df8f7f6169e148b4724b088878ca8dfff34e9275
Summary:
Treemanifest had a bug where the pack files it created were 400 instead of 444.
This meant people sharing a cache on the same machine couldn't access them. In
most of the remotefilelog code we had set opener.createmode before creating the
pack but we forgot to for treemanifest.
This patch moves the opener creation and createmode setting into the mutable
pack class so we can't screw this up again.
Test Plan:
Tests that wrote to the packs before, now failed and had to be
updated to chmod the packs before writing to them.
Reviewers: #mercurial, rmcelroy
Reviewed By: rmcelroy
Subscribers: rmcelroy, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4411580
Tasks: 15469140
Signature: t1:4411580:1484321293:9aa78254677548a6dc2270c58cee0ec6f57dd089
Summary:
We have run into some cases where users ended up with empty pack file in their
packs directory. Just log a warning in this case and skip this pack file,
rather than letting the exception propagate up and crashing the command.
Test Plan:
Created empty 0000000000000000000000000000000000000000.histpack and
0000000000000000000000000000000000000000.histidx files in my repository's
hgcache directory, and confirmed that "hg log" now simply warns about them
instead of crashing.
I didn't really test the perftest.py or treemanifest_correctness.py extensions
much. They seem to throw exceptions, and look like they have maybe gotten a
bit stale. I fixed one minor typo, but didn't dig into the other exceptions
too much.
Reviewers: durham
Reviewed By: durham
Subscribers: net-systems-diffs@, yogeshwer, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4402516
Tasks: 15428659
Signature: t1:4402516:1484155254:96d2980efcec2d735257b08910e1ca437ef1dad6
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
