Summary:
Before this patch, we were caching in the background in remotenames
test. This could result in a race condition making the test non deterministic.
Test Plan: Test changed
Reviewers: ttung
Differential Revision: https://phabricator.intern.facebook.com/D3353406
Summary:
As discussed it is not relevant as the tests cache so little it should
not make the test non-deterministic.
Test Plan: The test change accordingly since the limit is no longer None
Reviewers: ttung
Differential Revision: https://phabricator.intern.facebook.com/D3353394
Summary: This allows us to use fastmanifest as a directory to drop in the python module.
Test Plan: compiles, passes existing tests.
Reviewers: lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3351021
Signature: t1:3351021:1464284417:6cbcde514ab1fd7b5caa6c83cb5577f3502dbc58
Summary:
Before this patch, we didn't test that caching could be triggered by
remotename changes. When I tried testing it, I noticed that it wasn't working
because manifest for revisions with remotenames were not cached. I added
remote/master as a revision to cache to the fastmanifesttocache revset and
included a test.
Test Plan: Added a new test
Reviewers: simonfar, quark, ttung, durham
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3335931
Summary: This will limit what we cache in the first place
Test Plan:
No test yet, I will add a test in a diff later checking cache
eviction.
Reviewers: ttung, durham
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3346515
Summary:
As discussed in the group, we want to shuffle by batches to keep
an approximate ordering and still avoid caching process fighting for the
same entries.
Test Plan: Add a new test
Reviewers: durham, ttung
Differential Revision: https://phabricator.intern.facebook.com/D3344144
Summary:
If we want to cache revision [300, 350, 320, 380], we would first order the
revisions to be [380, 350, 320, 300] and cache them in that order. Assuming that
only 3 revisions can fit in the cache, we write all revisions and prune the
one that overflew. Afterwards, we change the mtime of all the cached entries
to ensure that for each of the newly cached entries, more recent mtime == more
relevant entry.
Test Plan: Test output changes slightly, as expected
Reviewers: mitrandir, ttung, simonfar
Differential Revision: https://phabricator.intern.facebook.com/D3344125
Summary: also remove some tests that are passing now from the list
Test Plan: ran mercurial tests with sqldirstate
Reviewers: #mercurial, ttung, durham
Reviewed By: durham
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3345259
Signature: t1:3345259:1464168975:1f6e26ce730b32e5b0542ab3ef7e1dc7088c0b66
Summary: Interpret revrange inside the worker process. The reason this is needed is that `triggercacheondirstatechange` happens in the wlock release callback. However, the lock is not released at this point. Interpreting the revrange requires wlock, so then we try to acquire it but we'll never succeed (because we need to finish the wlock release callback). If we do it in the scope of the worker process, it'll try to acquire it, find that the primary process owns it, and wait to acquire it.
Test Plan: run unit tests
Reviewers: durham, lcharignon
Reviewed By: lcharignon
Subscribers: quark, mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3339805
Tasks: 11385141
Signature: t1:3339805:1464209208:ec2424f84570d489aa5c0c629a15b69b6b126c60
Summary: This could be useful to have the number of valid entries in the cache
Test Plan: Test output changes accordingly
Reviewers: ttung
Differential Revision: https://phabricator.intern.facebook.com/D3343488
Summary:
Before this patch with fastmanifest.cacheonchange on, we would cache
manifest on change to dirstate, bookmarks and remotenames. This patch makes
these operation in the background by default. Also it changes the cache strategy
used by default to be the new strategy that adapts based on the system resources.
Test Plan: Tests does not change
Reviewers: ttung, simonfar, quark
Differential Revision: https://phabricator.intern.facebook.com/D3341681
Summary:
I didn't fix the non determinism properly before, this patch fixes
the issue with a more defensive code.
Test Plan: Ran the test 32 times and the result didn't vary
Reviewers: ttung, simonfar, quark
Differential Revision: https://phabricator.intern.facebook.com/D3341032
Summary: If we don't, then a lazily-evaluated set is returned. That lazily-evaluated set will suppress the hidden bit when dirstate changes are involved.
Test Plan: on a dirstate change with a lot of hidden revs, observe that only a small set of revs are cached.
Reviewers: lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters, #mercurial
Differential Revision: https://phabricator.intern.facebook.com/D3338252
Signature: t1:3338252:1464107195:fd0e342fb4e4eb485057b7fda9679f7ff18f4e7b
Summary:
Before this patch, we were iterating over the cache entries in a non-
deterministic fashion. This patch makes the iteration stable by ordering the
entries by filename.
Test Plan: Changed the tests accordingly
Reviewers: ttung
Differential Revision: https://phabricator.intern.facebook.com/D3336880
Summary:
We order by mtime and then by filename and not just by mtime. Otherwise, we
wrongfully relied on os.listdir to return the same thing on different platforms.
Test Plan: Change the test
Reviewers: ttung
Differential Revision: https://phabricator.intern.facebook.com/D3336874
Summary:
When we start worker process to cache revisions, since we don't use
a lock, it is better to have the workers cache revisions in a random order to
avoid duplicated effort as much as possible.
Test Plan: No test for now, not sure how to test that.
Reviewers: simonfar, quark, ttung
Differential Revision: https://phabricator.intern.facebook.com/D3335918
Summary:
This adds a new strategy for the cache limit that depends on the free
space availabe in the system. For system with more than 100GB of free space
we allocate 5GB for the cache. With more than 20 GB of free space we allocate
2GB of free space and otherwise 10% of the free space at most.
Test Plan: Add a new test
Reviewers: ttung, durham, rmcelroy
Differential Revision: https://phabricator.intern.facebook.com/D3330894
Summary:
This adds the first, basic implementation of size limitation for
the fastmanifest cache
Test Plan: Add a few tests that cover edge cases
Reviewers: ttung, simonfar, durham
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3326149
Summary:
This patch makes it possible to cache fastmanifest asynchronously. I added
a test an also tested with fbsource:
hg debugcachemanifest --debug --config extensions.fastmanifest=/data/users/lcharignon/facebook-hg-rpms/fb-hgext/fastmanifest.py -R ~/fbsource --all
hg debugcachemanifest --debug --config extensions.fastmanifest=/data/users/lcharignon/facebook-hg-rpms/fb-hgext/fastmanifest.py -R ~/fbsource --list
This is a tricky code, and I think @quark would be best to review it.
Test Plan: Added a new test
Reviewers: quark, durham, ttung
Subscribers: quark
Differential Revision: https://phabricator.intern.facebook.com/D3323540
Summary: This is a refactoring of the _cachemanifest function
Test Plan: test output does not change
Reviewers: ttung, simonfar
Differential Revision: https://phabricator.intern.facebook.com/D3330501
Summary:
We use the manifestnode from the changelog and avoid having to reach the manifest
completely to check if it is already in cache.
This goes with a refactoring of the contains method. Before this patch, the
contains method was misleading because an entry could be contained in the cache
but not show up when you iterate over the cache. This happened because contains
was operating on nodes and iterating showed filesnames (so keys + prefix). To
make it clear that contains operates on nodes, we change its name to containsnode
We also rename key to hexnode to make it clearer that the keys are hexnodes.
Test Plan: Existing tests pass. We also add a new test to show that the fast
pass is hit.
Reviewers: durham, ttung
Differential Revision: https://phabricator.intern.facebook.com/D3325335
Summary: This will be useful for implementing garbage collection later.
Test Plan: Add a new test that exercises the new function
Reviewers: ttung
Differential Revision: https://phabricator.intern.facebook.com/D3322426
Summary:
This patch changes fastmanifest to add automatic caching of relevant
manifest when bookmarks change or if the parent of the working copy changes.
Test Plan: Added a test
Reviewers: ttung
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3318775
Summary:
We move wrapfilecache from reflog to extutil to be able to reuse it
in fastmanifest
Test Plan: Tests pass
Reviewers: ttung
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3318762
Summary:
Adding a pruneall method to prune all cache manifest. This will be
useful to test the caching logic without having to run shell commands to remove
manifests.
Test Plan: Add a new test
Reviewers: ttung, durham
Differential Revision: https://phabricator.intern.facebook.com/D3314158
Summary:
When people combine --root-relative with a file pattern, they would probably
expect the output to be relative to the root -- but it's not! In this case, it
would fall back to the default file pattern behavior, which is cwd-relative.
Instead of confusing the user with apprarently errorneous output, let's abort
explicitly and provide a helptful hint.
Test Plan: added a new test, existing tests still pass
Reviewers: #mercurial, simpkins, ttung, quark
Reviewed By: quark
Subscribers: quark, net-systems-diffs@, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3319366
Signature: t1:3319366:1463610688:3f129c97f68f43ac85d2b31b55fac5c859e85c04
Summary:
* `_manifest()` should attempt to retrieve a fastmanifest if possible.
* `self.incache` represents a tristate indicating True if the fastmanifest is available, False if the fastmanifest is not, and None if we haven't tried to determine its availablility.
* clarified the debug messages a bit
* re-introduce the test removed in D3277498 (since it actually can work now).
Test Plan: pass unit tests!
Reviewers: lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3288967
Signature: t1:3288967:1463416544:416f28ecdc5e6a26545f3d4215fc8baa9222af7c
Summary: since it's a singleton, we can have one instance of the code for retrieving the instance.
Test Plan: run tests/test-fastmanifest.t
Reviewers: lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3284113
Signature: t1:3284113:1463416610:207c7e9954cc0b830e77f2fecc0a65acfcfd1096
Summary: Flat manifests are cached in memory by manifest.py. Unclear whether there's any reason for us to cache flat manifests on disk.
Test Plan: ran test-fastmanifest.py
Reviewers: durham, lcharignon
Reviewed By: lcharignon
Subscribers: trunkagent, mitrandir, mjpieters, #mercurial
Differential Revision: https://phabricator.intern.facebook.com/D3277498
Tasks: 10589051
Signature: t1:3277498:1463416673:74cab287b9045166dc0322a45f8a325f65da5d1f
Summary:
I want to re-use this elsewhere, so robustify it a bit
and move it to phabricator.diffprops.
Test Plan: run-tests.py, also verified in my www repo.
Reviewers: #sourcecontrol, ttung
Reviewed By: ttung
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3230147
Signature: t1:3230147:1463081787:799f232e2ce73395218db3a0fff37dec9a0b02e0
Summary: Previously, we were just reading the first rev we accessed. This is modified to read the entire file.
Test Plan:
I don't see any non-determinism any more. hammered with
```
while true; do python ~/work/mercurial/facebook-hg-rpms/hg-crew/tests/run-tests.py --keep-tmpdir test-fastmanifest.t; if [ $? -ne 0 ]; then break; fi; done
```
Reviewers: durham, lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters, #mercurial
Differential Revision: https://phabricator.intern.facebook.com/D3278649
Tasks: 10604335
Signature: t1:3278649:1463063754:1905372c2da75dcc42d09a3a8cda4ff5a93f1c04
Summary:
The manifests consumed by mercurial does not appear to be nondeterministic, which is reassuring. There is, however, a small bug in the test code. That is resolved in the next diff.
This is *not* a straight revert, as this file has changed. The minimal set of changes were done merge the two.
Test Plan: next diff, sorry.
Reviewers: durham, lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters, #mercurial
Differential Revision: https://phabricator.intern.facebook.com/D3278623
Signature: t1:3278623:1463067584:30ea81ac48fb4c8df1dcf7bf6d17233369b47cc3
Summary:
An extension replacing dirstate file with sqlite database so we can have incremental changes and we don't have to read the whole dirstate on every op. This makes sense only when hgwatchan/fsmonitor is on so we don't iterate through whole dirstate.
This is also using the sqlite transactions to handle dirstate transactions instead of copying db around. As a result of that "hg rollback" doesn't work anymore. You can fall back to copying things by setting sqldirstate.skipbackups to False.
Needs those to go to upstream to work: https://phabricator.intern.facebook.com/P56319612
(will send them once the freeze is over)
To use make sure that the extension is loaded *before* hgwatchman (watchman
should be outmost layer).
Test Plan:
Passing all but few mercurial tests (when running with skipbackups=False)
The failures are described in blacklist file.
Reviewers: lcharignon, wez, quark, durham
Reviewed By: durham
Subscribers: laurent, mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D3242547
Signature: t1:3242547:1462577481:fdbfb5287fb8d3e58f7b4d587c01de79ce6b78df
Summary:
When running with chg, `sys.argv` is not a reliable source to get command line
arguments. Hacking chgserver with setting `sys.argv` is possible but that
is not welcomed by upstream (http://patchwork.serpentine.com/patch/14318/)
Instead, wrap `dispatch.runcommand` to get the arguments for now. We will
lose all the `--config` arguments but that seems okay.
Test Plan: Run test-reflog.t
Reviewers: #mercurial, ttung, durham
Reviewed By: durham
Subscribers: durham, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3264615
Tasks: 11114741
Signature: t1:3264615:1462487500:44e5cd91b5d4b5bc60a7d01a816fb91ed54aa6bb
Summary:
Shelve and unshelve use commits under the hood to bundle up data. dirsync sees
these commits happening and performs mirrors, which then shows up at unbundle
time, making it impossible to mirror the change again. Example: if I change X,
and it gets mirrored to Y, if I go back and change X later after a unshelve, hg
commit now fails because Y has pending changes and differs from X.
Test Plan: Added a test
Reviewers: #mercurial, ttung, quark
Reviewed By: quark
Subscribers: quark, wez, mjpieters, frantic, wluh
Differential Revision: https://phabricator.intern.facebook.com/D3266602
Signature: t1:3266602:1462484017:738f67c0ab4b5af999819d3855c1f4ba6b2ea338
Summary:
API to write trees to disk and read trees from disk.
Some interesting things to keep in mind:
* To keep valgrind happy, we have to clear all the memory we allocate but never initialize. That can be padding between fields in structs, fields that don't make sense to initialize, the extra byte in checksums that doesn't get used normally, or portions of bitfields that are never used. All this logic is kept compartmentalized in `initialize_unused_bytes(..)`. Normal runtime does not invoke this code, though I suspect the cost is probably not significant.
* A significant chunk of the write path is made up of the `CHECKED_WRITE_XXX` macros. They're calls to ensure that writes succeed. If not, they jump to a fixed exit point.
* The write path is not very optimized. We can use a bottom-up traversal, similar to tree_convert, to make the write less costly. However, our design doesn't require a fast write, so tentatively, I'm going to reuse `tree_copy(..)` to compact a tree.
Depends on D3255048
Test Plan: pass unit tests!
Reviewers: lcharignon, wez
Reviewed By: wez
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3255656
Tasks: 10589048
Signature: t1:3255656:1462342423:c28d32c610b2351614d7648c03f35f931370a770
Summary:
this reduces some of the grovelling around that we're doing
in the full diff structure, and is a smaller amount of data to receive
and process in any case.
Test Plan:
test-diff-since-last-arc-diff.t
```
PYTHONPATH=/data/users/wez/facebook-hg-rpms/fb-hgext /data/users/wez/facebook-hg-rpms/hg-crew/hg --config extensions.publish=/data/users/wez/facebook-hg-rpms/fb-hgext/arcdiff.py --config extensions.errorredirect=! diff --since-last-arc-diff
```
Reviewers: #sourcecontrol, ttung, mitrandir
Reviewed By: mitrandir
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3230136
Signature: t1:3230136:1461810350:54cea1026e86ef33b398ee070eb8bbe7a8667ed6
Summary: We want out phabricator diff parsers to recognize both https://phabricator.intern.facebook.com/ and https://phabricator.fb.com/ URLs (and a bigger class as well).
Test Plan:
- updated some tests (for `phabdiff` and `pullcreatemarkers` other files don't even seem to be used)
- ran tests
- tested phabdiff manually as well:
{F60696023}
Reviewers: #sourcecontrol, andersonmat, mitrandir, simpkins, lcharignon, quark, ttung, ikostia, rmcelroy
Reviewed By: ikostia, rmcelroy
Subscribers: wez, rmcelroy, net-systems-diffs@, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3229776
Tasks: 11013909, 11017978
Signature: t1:3229776:1461839346:08b9b3414e43ff9966bc05591ca5662ef9691aac
Summary:
In addition to being duplicated between these places,
I'd like to re-use this elsewhere.
Test Plan: run-tests continues to pass
Reviewers: #sourcecontrol, ttung, ikostia
Reviewed By: ikostia
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3230128
Tasks: 11013909
Signature: t1:3230128:1461775513:6fc79fda68cd15ded7fb11d52024b5aab56ee880
Summary:
I pretty much stole this from our libfb.py.conduit client, but
removed the python 2.7 and 3 specific aspects of it.
This is an HTTP client for conduit, rather than shelling out to arcanist.
I've added a very simple mechanism for replaying conduit results in the
test harness and used this to build out some tests for the `arcdiff.py`
and `phabstatus.py` extensions.
Test Plan:
```
$ ../../hg-crew/tests/run-tests.py -j8
```
In addition to the new tests, manually tested the actual HTTP functionality:
```
$ /data/users/wez/facebook-hg-rpms/hg-crew/hg --config extensions.phabstatus=/data/users/wez/facebook-hg-rpms/fb-hgext/phabstatus.py --config extensions.errorredirect=! ssl
```
Does not error out and shows the diff status.
Reviewers: #sourcecontrol, ttung
Subscribers: mjpieters
Differential Revision: https://phabricator.fb.com/D3200713
