Summary: We don't want users to see scary exceptions or errors from worker processes. They are unexpected and hard to deal with.
Test Plan: introduce syntax errors into the code. run hg boo -d abc in repo with fastmanifest enabled. saw no errors. then ran the same command with --config=fastmanifest.silentworker=False and saw the error.
Reviewers: lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3357657
Signature: t1:3357657:1464365560:0cb101ab8a886f8e586a275bd91090331ad20982
Summary: When --debug is turned on for commands, there's a ton of spew. This makes it easier to spot the parts I care about.
Test Plan: update tests
Reviewers: lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3357974
Signature: t1:3357974:1464365624:83edc815109db8533465991413a7c7741f1056a8
Summary: This mostly replicates the logic in `manifest.py::manifest::add(..)`, except it interfaces with the fastmanifest cache instead.
Test Plan:
used `hg debugcachedmanifest` to populate the cache, then did a commit with `pdb.set_trace()`.
Future Test Plan:
# Write a Real Unit Test™
# Instrument revlog to ensure we're not actually reading from the revlog.
Reviewers: lcharignon, simonfar
Reviewed By: simonfar
Subscribers: simonfar, mitrandir, mjpieters, #mercurial
Differential Revision: https://phabricator.intern.facebook.com/D3337102
Tasks: 11294278
Signature: t1:3337102:1464087802:b1cd63fc6be820e713f176463bac372a8042f6e8
Summary: I've done it wrong in one place so let's centralize that.
Test Plan: ran tests.
Reviewers: #mercurial, ttung, durham
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3354724
Summary: `fixedcachelimit` is removed by D3355182 so the rest is just debug cruft.
Test Plan: pass existing unit tests.
Reviewers: lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3355456
Signature: t1:3355456:1464304289:07a16dec879f0caa3947fbc4fe5db353149d5209
Summary: I think it makes sense.
Test Plan: run unit tests
Reviewers: lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3355211
Signature: t1:3355211:1464304354:fb04fcd1ed496da3a7f0a176c4bc04f4fc3711b3
Summary: We want to wrap the fastmanifest cache priming code in a lock, but we want it to be easily stealable. If the lock is more than X seconds in age, we just assume it's stale and steal it. We remember that we stole it so we don't blow away the lock, but we do update the time of the lock so another process doesn't try to steal the lock.
Test Plan: write test to cover basic functionality (mutual exclusivity). write test to cover stealing, and that only one thief is permitted at a time (subject to race conditions).
Reviewers: durham, lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters, #mercurial
Differential Revision: https://phabricator.intern.facebook.com/D3343478
Tasks: 11385124
Signature: t1:3343478:1464304447:18658ebea60f98bfda0f034414991ffd6c334ca7
Summary: If we default the proxy to flatmanifest's or fastmanifest's implementation of `filesnotin`, we can end up in situations where we're dealing with different types of manifests.
Test Plan: hg amend works w/ fastmanifest
Reviewers: lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3354668
Signature: t1:3354668:1464304470:6a7097a15043d4639111a5243c702569bc5fe004
Summary:
This makes the code smaller, more readable and prints a warning when
the config is malformed.
Test Plan: Test pass and tests exercise this code path.
Reviewers: ttung
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3354470
Summary:
This test is mostly a copy of test-shelve.t from mercurial with
minor changes (removing the hooks usage).
The normal tests for shelve unfortunately are using out of proces
hooks which doesn't work good with sqldirstate. As shelve is
important extension for us we want to test is with shelve anyway.
Test Plan:
test passes on clowncopter with this patch applied:
http://patchwork.serpentine.com/patch/15207/
Reviewers: #mercurial, wez, quark, durham
Reviewed By: durham
Subscribers: quark
Differential Revision: https://phabricator.intern.facebook.com/D3350194
Signature: t1:3350194:1464260281:c85a1d5ae50d9488a8bcdb552343f466bebb9e05
Summary: This combines D3351047, D3351086, and D3351137 into one diff. This allows mercurial to better remember where things came from since these are all hg cp commands followed by some edits. However, they were separated into multiple diffs for ease of reviews.
Test Plan: run unit tests
Reviewers: lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3354405
Signature: t1:3354405:1464291057:da140a02b9c87186b1dd9f03f8236d5996319fff
Summary:
fastmanifesttocache was getting a little messy so we break down the
function.
Test Plan: No test output change.
Reviewers: ttung
Differential Revision: https://phabricator.intern.facebook.com/D3353446
Summary: This is a documentation change to make the code clearer
Test Plan: No test change
Reviewers: ttung
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3353435
Summary: mannodes wasn't a descriptive name, revstomannodes is better
Test Plan: test output does not change as expected
Reviewers: ttung
Differential Revision: https://phabricator.intern.facebook.com/D3353411
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:
This diff adds documentation for all the config options and their
default values.
Test Plan: No test change.
Reviewers: ttung, durham
Differential Revision: https://phabricator.intern.facebook.com/D3346491
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: This will be useful to implement the new cache filling strategy.
Test Plan: Refactoring does not change test output (y)
Reviewers: ttung, simonfar, mitrandir
Differential Revision: https://phabricator.intern.facebook.com/D3344111
Summary:
This will be useful later on and exercised by the new cache filling
strategy
Test Plan: Not exercised yet
Reviewers: mitrandir, ttung
Differential Revision: https://phabricator.intern.facebook.com/D3344107
Summary:
shelve is woking thanks to the hack where it abandons the transaction but
preserves the dirstate anyway by copying the dirstate to the side and restoring
it. We can do something that works as well and is way faster.
Test Plan:
ran tests
tried it on fbsource - it was nice and fast
Reviewers: #mercurial, ttung, simonfar, durham
Reviewed By: durham
Subscribers: simonfar, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3345280
Signature: t1:3345280:1464170069:cfb22b7034f8a8c44fd4284cd35ba17e85dabb99
Summary:
the transaction won't be commited anyway.
The journal will be rolled back on next database open.
Test Plan: ran mercurial tests with sqldirstate
Reviewers: #mercurial, durham, ttung, simonfar
Reviewed By: simonfar
Subscribers: simonfar, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3345277
Signature: t1:3345277:1464168844:69206055d24ae0349985f0038845122e7cea0a19
Summary:
In the dirstate world we don't maintain in-memory cache over dirstate - there
is no need to invalidate it then.
Test Plan: ran mercurial tests with sqldirstate
Reviewers: #mercurial, ttung, durham
Reviewed By: durham
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3345268
Signature: t1:3345268:1464169114:e55b709c4acbc89c4e2239f7534c6e13ef9b4861
Summary: I discovered that mercurial has better mechanics for extensions to use
Test Plan: ran mercurial tests with sqldirstate
Reviewers: #mercurial, durham, ttung, simonfar
Reviewed By: simonfar
Subscribers: simonfar, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3345260
Signature: t1:3345260:1464168562:32b09a0f86bc19b8622f8fe51ecb727ef3c44bbc
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:
This causes the read-only dirstate operations to be readonly from
db standpoint so they can run while there is a pending transaction.
Test Plan: ran mercurial tests with sqldirstate
Reviewers: simonfar, durham
Reviewed By: simonfar, durham
Subscribers: simonfar, mjpieters, #mercurial
Differential Revision: https://phabricator.intern.facebook.com/D3345257
Signature: t1:3345257:1464168444:a98bc04c23e174d0c10ed07eb914930949a67b7d
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 diff changes the fastmanifestocache revset to also cache parents
of revisions in the revset.
Test Plan: Check that it works calling debugcachemanifest manually with
hg sl -r "fastmanifesttocache()"
Reviewers: durham, ttung
Differential Revision: https://phabricator.intern.facebook.com/D3344222
Summary:
This will be useful to improve the cache filling process to stop once the limit
hits the total size.
Test Plan: Test output does not change
Reviewers: mitrandir, ttung, simonfar
Differential Revision: https://phabricator.intern.facebook.com/D3344099
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:
This time I tested the full workflow filling up the cache entierly respecting
the system limit. It went well with no crash.
Test Plan:
Tested filling up the cache using the systemawarecachelimit and then
pruning some of the cache to exercise all the codepaths in systemawarecachelimit.
Reviewers: ttung
Differential Revision: https://phabricator.intern.facebook.com/D3343474
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: Wrong variable name lead to a crash
Test Plan: The crash does not happen anymore
Reviewers: ttung, simonfar, quark
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3341687
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: We don't actually want to daemonize the process. We just want to spawn a worker process to do some work. Instead of exitting the initial process, we fork, do a bit of cleanup of the output streams, and then fork once again to prevent zombies.
Test Plan: run unit tests
Reviewers: lcharignon
Reviewed By: lcharignon
Subscribers: mitrandir, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D3339788
Tasks: 11385109, 11385154
Signature: t1:3339788:1464107186:95bdaeb1f612ab058c4cc004af70c4d0bd9d2c58
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
