Summary:
Let's add an option that can be used to trigger full clean backup.
It can be useful in many situations, for example, in case of bug in
infinitepush backup or if the naming scheme of backup bookmarks were changed.
Test Plan: arc unit
Reviewers: #mercurial
Subscribers: #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D5180390
Tasks: 15389402
Summary:
Previously `_getrevstobackup()` would load manifest for each commit in memory
only to check if a file was deleted in this commit or not. Manifest can be
quite big and since every loaded flatmanifest is cached in fastmanifest then
memory usage can be huge.
Since we are doing this only to check if there are any commits that were
downloaded to the client without filelogs and then were stripped server-side.
This cases are rare and it would be easier to track these commits in the
config file.
Test Plan: arc unit
Reviewers: #mercurial, mjpieters, quark
Reviewed By: quark
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D5111372
Tasks: 15389402
Signature: t1:5111372:1495848867:4028bd48313ac0e2022c3695195bedc6eda80abf
Summary: Upstream has deprecated cmdutil.commands() in favor of registrar.commands()
Test Plan: Ran the tests
Reviewers: #mercurial, quark
Reviewed By: quark
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D5106486
Signature: t1:5106486:1495485074:0e20f00622cc651e8c9dda837f84dd84cc51099e
Summary:
One of the heads may point to filtered commit and isbackedup command fails in
this case. Let's use repo.unfiltered()
Test Plan: arc unit
Reviewers: #mercurial, rmcelroy
Reviewed By: rmcelroy
Subscribers: rmcelroy, mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D5001977
Signature: t1:5001977:1493907725:f6a138bd4e2cae48b64152fceaf73660ea91f43b
Summary: Infinitepush didn't clean up temp files during pulls.
Test Plan:
Choose any hg server that's not in vip (for example, hg014.lla2).
Run `hg pull ssh://hg014.lla2.facebook.com//data/scm/fbsource -r INFINITEPUSH_COMMIT_HASH`
Check that new /tmp/tmp* file has appeared.
Install fixed infinitepush extension on the server.
Run the same command again, make sure it works fine.
Note: no new unittests, because temp filenames are random and that's difficult to test.
Reviewers: #mercurial, dschatzberg, tja
Reviewed By: tja
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D5077767
Tasks: 17599765
Signature: t1:5077767:1495025042:d209d1e5342442ac8b05a4c80748780bbf36846c
Summary:
`bundlefile` variable wasn't initialized if `head in nodestobundle` were true.
Even worse, the previous version of `bundlefile` were used. That means that in
some cases infinitepush sends back a wrong bundle.
Adding a unittest is quite tricky because problem shows up only in one rare
case. Example: we have a stack of 2 commits A and B and each commit in the
stack has a bookmark that points to it. Then `getbundlechunks()` `heads`
parameter contains two heads: commit A and commit B in undefined order.
If it happens that first element in the list is commit A we'll create a bundle
that contains only commit A. And we'll reuse the same bundle when we process
commit B. That means that we'll send the same bundle with commit A twice,
but no bundles with commit B.
I refactored the code to make sure we won't get the same bug in
the future.
Test Plan: arc unit
Reviewers: #mercurial, durham
Reviewed By: durham
Subscribers: durham, mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4969547
Tasks: 17719083, 15389402
Signature: t1:4969547:1493749313:587f9e4446a3c21b47c081a0fe4cd9e200dab5cd
Summary:
As @quark suggested, it's better to use `infinitepush` path instead of
default-push, because `default-push` is facebook-specific configuration.
Test Plan: arc unit
Reviewers: #mercurial, rmcelroy
Reviewed By: rmcelroy
Subscribers: quark, rmcelroy, mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4962324
Tasks: 15389402
Signature: t1:4962324:1493727112:35c45c57a527d2de5a35ea83e1031dc1908ac28e
Summary:
Command that checks if a commit was backed up.
It does it by checking if revisions are in local backup state.
Test Plan: Run unit tests and lints
Reviewers: #mercurial, durham
Reviewed By: durham
Subscribers: durham, mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4938072
Tasks: 17519836
Signature: t1:4938072:1493072854:291765bb59d327db8504feb47d6089818ae1e11a
Summary:
infinitepushbackup.tempcleanworkingcopiesbackups option sends special command
to clean backup bookmarks from the server for the shared working copy (i.e.
if we have main repo `fbsource` and shared working copy `fbsource2` we want
to have backups only for `fbsource` not for `fbsource` and `fbsource2`).
Before this diff cleanup commands were send for every backup.
This diff makes them send only if backup state file is present.
Test Plan: arc unit
Reviewers: #mercurial, durham
Reviewed By: durham
Subscribers: durham, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4929489
Signature: t1:4929489:1493075761:a408300338a10864043b538540d03880a49c4e1a
Summary:
There was a bug that caused commits with file deletions to not be backed up.
This diff fixes it by first checking if file exists in the commit and only
then downloading context of the file.
Note:
In tests I had to ignore stdout of `hg pushbackup` because the output was
different on macs and linux.
On linux there was an additional line
remote: abort: data/committostripfirst.i@091b63e5e4: no match found!
Probably mac's remotefilelog closes stdout/stderr earlier, but I wasn't
able to find a root cause.
Test Plan: Run unit tests
Reviewers: #mercurial, quark
Reviewed By: quark
Subscribers: quark, mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4929244
Tasks: 15389402
Signature: t1:4929244:1492791804:77b2baa9eb54a53120a955e72e6c132be5db6b44
Summary:
`exchange.readbundle()` can return bundle2 unbundler or changegroup cg1unpacker.
In case of cg1unpacker let's just read it from the stream
Test Plan: arc unit
Reviewers: #mercurial, quark
Reviewed By: quark
Subscribers: quark, mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4913313
Tasks: 15389402
Signature: t1:4913313:1492623174:72abe4d1e449ae31d78a6c98b554c0406e8a2ea2
Summary:
Previously we had separate backup per working copy. That's very confusing
since all these working copies shares the same repo. This diff fixes it and
also adds config option to clean unnecessary working copy server-side.
Test Plan: Run infinitepush unittest
Reviewers: #mercurial, quark
Reviewed By: quark
Subscribers: rmcelroy, quark, mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4876230
Signature: t1:4876230:1492025747:3579e5046efc2ed309044fc3335c36ac4f7bdd04
Summary: Check owner of a log file before writing to it. See comments for details
Test Plan: arc unit
Reviewers: #mercurial, simpkins
Reviewed By: simpkins
Subscribers: net-systems-diffs@fb.com, mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4842933
Tasks: 17155924
Signature: t1:4842933:1491861137:e8a027fb7a930c0c5f553c75cb84214d24f66ce3
Summary:
Profiles show that rebundling is the slowest part in infinitepush that can
take up to 10 secs to run. In many cases rebundling is not needed at all.
For example, if bundle contains only one head and that head was requested
during pull then rebundling can be avoided at all.
Test Plan: arc unit
Reviewers: #mercurial, quark
Reviewed By: quark
Subscribers: quark, mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4834982
Tasks: 15389402
Signature: t1:4834982:1491450862:fe6984e6384676bcbab1c3fb9f0a47cb93902d0d
Summary:
It's a good way to check if backup is consistent. If backup is not consistent
then we set special exit code that will be logged. It let's us easily find users
with inconsistent backups and fix it.
Test Plan: arc unit
Reviewers: #mercurial
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4810442
Tasks: 15389402
Summary:
Give better names to the variables to underline what these variables really
store. And a bit of code movement.
Test Plan: arc unit
Reviewers: #mercurial
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4803405
Tasks: 15389402
Summary:
`headstobackup` won't point to bad nodes and they are only draft.
No need to check one more time
Test Plan: arc unit
Reviewers: #mercurial
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4803376
Tasks: 15389402
Summary:
Previously we backed up extinct node if bookmark pointed to them. But this not
is not present in the bundle because `findcommonoutgoing()` intentionally skips
extinct and secret commits. In this case we'll have infinitepush bookmark that
points to non-existent infinitepush commit. To fix it let's filter extinct heads
Test Plan: arc unit
Reviewers: #mercurial
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4803322
Tasks: 15389402
Summary: Let's add a table that will store metadata about infinitepush nodes.
Test Plan: arc unit
Reviewers: #mercurial
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4778867
Tasks: 16899722
Summary:
I forgot to insert it, fixing it now. It didn't cause problems before because
we delete bookmarks only during backups and backups usually have different
bookmark names
Test Plan: arc unit
Reviewers: #mercurial, quark
Reviewed By: quark
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4795310
Tasks: 15389402
Signature: t1:4795310:1490810441:0a044ebe776c65b03c633017c33771d32bfa3e77
Summary:
Real infinitepush tests are in fb-hgext/tests. These tests are useless and we
never run them. Let's remove them.
Test Plan: arc unit
Reviewers: #mercurial, rmcelroy
Reviewed By: rmcelroy
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4778858
Signature: t1:4778858:1490700082:69bcd2adee2a93ec5d3886747d47bc30b18f8944
Summary:
Previously we don't set findcommonincoming to True, and that bug was unnoticed
because we send this info anyway in server-side `getbundlechunks()` function.
But next diff in the stack uses a fast path server-side which won't work if
fincommonincoming is not set to True.
Test Plan: arc unit
Reviewers: #mercurial, durham
Reviewed By: durham
Subscribers: durham, mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4770830
Tasks: 15389402
Signature: t1:4770830:1490719209:c251002d992e244580b7dbfeca0c30dff95734d2
Summary:
Command to list available backups for the user.
--json will be used by automation
Test Plan: arc unit
Reviewers: #mercurial, simonfar
Reviewed By: simonfar
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4728729
Tasks: 15389402
Signature: t1:4728729:1490005762:41b9683cb7dfc9d84ae1032c807d1f0c3fe60dbf
Summary:
ui.shortuser() doesn't call util.shortuser() if ui.verbose equals to True.
But in our case it's very important to always use short username because
we'll get broken backups otherwise. Let's call util.shortuser() directly.
Test Plan: arc unit
Reviewers: #mercurial, mjpieters
Reviewed By: mjpieters
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4721949
Tasks: 15389402
Signature: t1:4721949:1489717973:b7e59929f530578e060b8cdef94ef92b54fa2647
Summary:
On macos sshpeer flushes remote output later and it causes tests to fail. Let's
add a call to cleanup() to make sure the output is flushed.
This diff also makes remotefilelog test more robust.
Test Plan: Run tests on macos
Reviewers: #mercurial, tja, mitrandir
Reviewed By: mitrandir
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4721276
Tasks: 15389402
Signature: t1:4721276:1489669109:4ce59f4a1d224d57dbb7c1eb341c4e6a659d2e8c
Summary:
By default debugcheckbackup checks only one backup for the user.
With --all option it checks all backups for the user from all host/reporoots
Test Plan: arc unit
Reviewers: #mercurial, durham
Reviewed By: durham
Subscribers: durham, mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4700993
Tasks: 15389402
Signature: t1:4700993:1489548921:800d08f420acc8ef4f807ffd17b31837dbb3fe82
Summary:
Backing up very big repo can cause problems. And very often old commits are
not necessary at all. Let's add a config option to limit the number of heads
that are being backed up.
Test Plan: arc unit
Reviewers: #mercurial, durham
Reviewed By: durham
Subscribers: durham, mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4700965
Tasks: 15389402
Signature: t1:4700965:1489549163:4e2c121f01debd7b495486a1f3b062926873399d
Summary:
Previously we saved bookmark hash and revision number as state of
the last backup. Storing last backed up revision is error-prone. I saw
a few corrupted backups where bookmarks pointed to non-backed up
nodes. Also this approach puts more pressure on mysql bundle index.
On every backup it first deletes all existing backup bookmarks and then it
writes new bookmarks. Even if user changed one head or one bookmark
(and that is the usual case) current approach still deletes bookmarks first
and then rewrites them.
Instead let's store all backed up heads and bookmarks locally in json format.
Json was chosen because it's simple and human-readable. Then during backup
let's compare last backed up state to the current state of the repo, and send
to the server only the difference.
Test Plan: arc unit
Reviewers: #mercurial, durham
Reviewed By: durham
Subscribers: durham, mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4700930
Tasks: 15389402
Signature: t1:4700930:1489613249:a34369bf53e0718c22258304493dfa27b578857f
Summary:
Let's rename _getbackupstate() to _downloadbackupstate() because it actually
downloads it from the server. Also return class instead of tuple
Test Plan: arc unit
Reviewers: #mercurial, durham
Reviewed By: durham
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4713633
Tasks: 15389402
Signature: t1:4713633:1489612870:3439a6a99ef311bf96784b3b1e53d7e9b94c0713
Summary: New logging will have rotation and will separate different users and repos
Test Plan: arc unit
Reviewers: #mercurial, durham
Reviewed By: durham
Subscribers: durham, mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4689191
Tasks: 16474976
Signature: t1:4689191:1489172091:d304d4b19ae6cf52f86c11f442fa760b50d1fdf9
Summary:
For now these command will be used in test because
`wait_for_background_backup.py` tied to the backup state file. And every change
to this file requires change to wait_for_background_backup.py. Let's create
simple internal backup command.
Test Plan: arc unit
Reviewers: #mercurial, durham
Reviewed By: durham
Subscribers: durham, mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4667056
Tasks: 15389402
Signature: t1:4667056:1489024344:6c43241e42b7c418baa4e1542f303239ba887c45
Summary:
There can be multiple backup processes running at the same time. Since these
processes can be quite heavy it makes sense to limit them. Let's use lock file
to do that.
Test Plan: arc unit
Reviewers: #mercurial, durham
Reviewed By: durham
Subscribers: durham, mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4667020
Tasks: 15389402
Signature: t1:4667020:1489170813:f2a685b1c224c553d3ee004d89d3eeeca816e824
Summary:
Command that checks that every head and bookmark node is present in the
bundlestore
Test Plan: arc unit
Reviewers: #mercurial
Subscribers: #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4658436
Tasks: 15389402
Summary: This functionality will be used in command to check backup consistency
Test Plan: arc unit
Reviewers: #mercurial, durham
Reviewed By: durham
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4658433
Tasks: 15389402
Signature: t1:4658433:1488828061:49aa5bfeba922d617cf1aab393f5e9f598aa33a4
Summary:
This small change makes a way better user experience. Users usually have one
big repo per host, but they may have many hosts (for example, laptop and
devserver). If we ask for reporoot first then it's very likely that next
`pullbackup` invocation will fail because there will be ambigious hosts.
Let's switch the order.
Test Plan: arc unit
Reviewers: #mercurial, quark
Reviewed By: quark
Subscribers: quark, mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4649946
Tasks: 15389402
Signature: t1:4649946:1488561645:330590fcf1dcd4af7fb572c5d4ccfd8a5ab78c60
Summary:
Next diff will add special option to specify whose backup to restore.
This diff does a necessary refactoring
Test Plan: arc unit
Reviewers: #mercurial, quark
Reviewed By: quark
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4650282
Tasks: 15389402
Signature: t1:4650282:1488562203:632a8c84d6d537663bd7d94ce9dfd18a4498ccfb
Summary:
This diff is part of the series to avoid downloading the same bundle
a few times.
Finally reuse the same bundlerepo.
Test Plan: arc unit
Reviewers: #mercurial
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4612094
Tasks: 15389402
Summary:
This diff is part of the series to avoid downloading the same bundle
a few times.
Now close all bundlerepos at once not one by one. This is necessary because
bundlerepos will be reused.
Test Plan: arc unit
Reviewers: #mercurial, durham
Reviewed By: durham
Subscribers: durham, mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4612089
Tasks: 15389402
Signature: t1:4612089:1488501776:e9fc2863adebc69bf44fa5dcb1610612027508f6
Summary:
This diff is part of the series to avoid downloading the same bundle
a few times.
Test Plan: arc unti
Reviewers: #mercurial, mjpieters
Reviewed By: mjpieters
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4612073
Tasks: 15389402
Signature: t1:4612073:1487955965:03af1a25acc21d0b6d913a60dd5ef82fa7f033ad
Summary:
There are a couple of reasons to avoid using ui.username():
1) If config option ui.username is set on the server then it will use it
instead of the name of the user that does push
2) It prints confusing warning `no username found, using '....' instead`
3) In some cases it fails (probably because it calls socket.getfqdn() which
does network request).
Let's use simple util.getuser() function and set username to 'unknown' if
it fails.
Test Plan: arc unit
Reviewers: #mercurial, simonfar
Reviewed By: simonfar
Subscribers: azich, mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4642457
Signature: t1:4642457:1488458040:5dfac435bf44dfa6ab7725d2f5800963e8c860ec
Summary:
Previously --list-remote wasn't user friendly. User have to always specify a
pattern, and specifying * just won't work because of bash globbing. Also
it wasn't possible to specify many patterns at once.
This diff allows specifying many patterns, it allows to not specify
patterns at all and also it allows to specify default scratch patterns to fetch
if no patterns were specified. This is useful if there are many auto-generated
bookmarks under the same scratch bookmark pattern.
Test Plan: arc unit
Reviewers: #mercurial, mjpieters
Reviewed By: mjpieters
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4605548
Signature: t1:4605548:1487934838:066a12c28dda16fa8f90674d736d21345631ef7e
Summary:
discovery.findcommonoutgoing() explicitly skips secret commits. Because of it
we also need to skip bookmarks that point to secret commits, otherwise they
will point to non-existent nodes.
Test Plan: arc unit
Reviewers: #mercurial, mjpieters
Reviewed By: mjpieters
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4611393
Tasks: 15389402
Signature: t1:4611393:1487933210:72e79923c944b13204f4cde64d415076703bbe47
Summary:
hasscratchbookmarks is True if scratchbookmarks is not empty and vice-versa.
No need to use this variable
Test Plan: arc unit
Reviewers: #mercurial, mjpieters
Reviewed By: mjpieters
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4590917
Signature: t1:4590917:1487682597:c2a6e0accbd8e539a885dd11e6819986e57d24df
Summary:
listkeyspatterns support batching. Let's use and avoid making many network
calls to the hg server.
I had to modify tests because scratchbookmarks are passed to the listkeyspatterns,
and scratchbookmarks is dict and the order of the keys is unpredictable.
Because of it bundles from the server will be sent in unpredictable order, and log
output may be different.
Test Plan: arc unit
Reviewers: #mercurial, simonfar
Reviewed By: simonfar
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4590914
Signature: t1:4590914:1487788586:dd5ba0dd41b0cd19f790755d7b3c1fc5ba4ade61
Summary:
Let's pull node during update if not found locally.
This is a part of selectivepull functionality.
See remotenames extensions for details about selectivepull.
Test Plan: arc unit
Reviewers: #sourcecontrol, durham
Reviewed By: durham
Subscribers: indragie, mjpieters, sergeyb
Differential Revision: https://phabricator.intern.facebook.com/D4536129
Tasks: 12479658
Signature: t1:4536129:1486667537:3d1df30cb5d1db0dd7451756102ccafee20789d5
Summary:
Default lock timeout value is very small (around couple of secs).
Let's set it for something bigger by default
Test Plan: arc unit
Reviewers: rmcelroy, mitrandir, durham
Reviewed By: durham
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4435236
Tasks: 15389402
Signature: t1:4435236:1484880747:d46276a82782ef551cda0165dea686f267c13ff9
Summary:
On a big repo with many old heads bundle that is sent to the server can become
big (in some cases even 500 Mb). That looks like a waste of bundlestore space
and will probably make backup and restore slower. Most of the space is taken
by manifest deltas because it prefers to diff manifest against previous commit
in the bundle. There are two possible approaches to reduce the size:
1) Send many small bundles (for example, one bundle per head)
2) Wrap deltaparent function and diff against actual parent previous commit in the bundle.
I chose the second approach for the following reasons:
1) It's easier to implement (main reason)
2) Many bundles probably means slower restore because there will be many requests to the bundlestore instead of just one
With this diff bundle size was reduced from 500 Mb to 8 Mb.
It can potentially increase CPU usage. I'm not sure how bad is it and will investigate it more.
Test Plan: Run `test-infinitepush-*`
Reviewers: rmcelroy, mitrandir, durham
Reviewed By: durham
Subscribers: simpkins, mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4384053
Tasks: 12479677
Signature: t1:4384053:1483664446:62ec30fad433e8d279758926199a8330cb73ed2b
Summary:
If client pulled a commit without filelogs (because of remotefilelog) and
this commit was later stripped on the server, then attempt to backup this
commit fails because of missing filelogs. It's a rare issue but unfortunately
it happens sometimes. To fix it let's exclude from the backup all the
commits and their descendants that doesn't have all necessary filelogs.
Test Plan: Run infinitepush tests
Reviewers: mitrandir, rmcelroy, durham
Reviewed By: rmcelroy
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4365747
Signature: t1:4365747:1483466164:560cdf5cd2369cd2603dfd0fe6b30d7a70951f00
Summary:
`pushbackup` command became huge. This diff splits it into a few
smaller functions
Test Plan: Run `test-infinitepush-*`
Reviewers: rmcelroy, mitrandir, durham
Reviewed By: durham
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4384008
Tasks: 12479677
Signature: t1:4384008:1483659549:a97b63a38b702a55d19cf6b47a6fd6b2547b9168
Summary: __init__.py became huge and I suggest to split it
Test Plan: Run `test-infinitepush-*`
Reviewers: rmcelroy, mitrandir, durham
Reviewed By: durham
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4383661
Tasks: 12479677
Signature: t1:4383661:1483659134:eaf3d7e217633895a65ad568831d9c21ea0f18d8
Summary:
`pushbackup` failed when there are hidden heads in the repo but no visible
heads (see test for example). This diff fixes it.
Test Plan: Run `test-infinitepush-*`
Reviewers: durham, mitrandir, rmcelroy
Reviewed By: rmcelroy
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4365677
Tasks: 12479677
Signature: t1:4365677:1483465909:fd1d96211909d3eeccab4fd17a364af4a1997f85
Summary:
The '&>>' operator only works in linux, not OSX. So let's replace it with a more
verbose equivalent. 'man bash' says these two are equivalent.
Test Plan:
Running the tests normally on OSX don't actually catch this, because
this particular test is skipped because it doesn't see that evolve is installed.
We need to run the tests with:
PYTHONPATH=/opt/facebook/hg/lib/python2.7/site-packages/:/opt/homebrew/lib/python2.7/site-packages/ /opt/homebrew/opt/python27/bin/python2.7 ../../facebook-hg-rpms/hg-crew/tests/run-tests.py test-infinitepush-backup.t
To get coverage. And the test is now passing on OSX and linux.
Reviewers: stash, #mercurial, quark
Reviewed By: quark
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4379593
Signature: t1:4379593:1483488383:ce8a045bbc29d719eafa372db56453b83f1d8df8
Summary:
Default path was used during background backup even if non-default path was
passed on the command line. So `hg push somepath --background` is
equivalent to `hg push --background`. This is not correct and this diff fixes it
Test Plan: Run `test-infinitepush-*`
Reviewers: durham, rmcelroy, mitrandir
Reviewed By: mitrandir
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4357295
Tasks: 12479677
Signature: t1:4357295:1482330125:d2b043f8035046ab43667b4387bf642e53261681
Summary:
As the name suggest it will restore backup made by `hg pushbackup`.
If user has only one backup for the `dest` repo then it will be restored.
But user may have backed up many local repos that points to `dest` repo.
These local repos may reside on different hosts or in different
repo roots. It makes restore ambiguous; `--reporoot` and `--hostname`
options are used to disambiguate.
Example situation:
1) User has only one laptop with mercurial repo `repo` that was cloned
from remote server. He or she run `hg pushbackup`. Then laptop breaks
and user gets a new one, clones the `repo` again and runs `hg restore`.
It automatically restores the backup.
2) User has devserver and laptop and backups were made from both.
Then if user decides to switch devserver and run `hg restore` on the new
devserver he or she has to specify `--hostname`.
Future plans:
1) Add `--user` option to make it possible to restore another user's backup
Test Plan: Run `test-infinitepush-*`
Reviewers: rmcelroy, mitrandir, durham
Reviewed By: durham
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4280832
Tasks: 12479677
Signature: t1:4280832:1481565335:2a21ceafa2ff80242076a79693046514434afb40
Summary:
`bundlerepo` should be closed to ensure that temp file is deleted.
`bundlerepo` creates temp file when bundle is compressed. This is *not* the
case in infinitepush (all our bundle are uncompressed). But seems that
upstream cg1unpacker.compressed() returns True even if bundle is uncompressed.
Also this diff makes `_getoutputbundleraw()` read output bundle in memory
instead of returning a generator. The reason for doing it is because generator
becomes invalid as soon as `bundlerepo` is closed.
This approach will obviously increase memory usage but it shouldn't be a
problem since bundles are small (no more than 10s Mb).
#thanks @quark for reporting it
Test Plan: Run `test-infinitepush-*`
Reviewers: durham, rmcelroy, mitrandir, quark
Reviewed By: quark
Subscribers: quark, mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4327186
Tasks: 12479677
Signature: t1:4327186:1481738559:2c6b22c305da4d572da9de21dfdf1179f7281744
Summary:
infinitepush has been storing bundle1 with changegroups v1. Turned out that in
some cases generated bundles can become huge because v1 changegroups don't
support generaldelta. It makes sense to support all changegroup versions (v1 for BC,
v2 for new infinitepush bundles, and maybe later we'll switch to v3).
To do this we have to store bundle2 instead of bundle1 because bundle1
supports only changegroups v1.
Test Plan: Run `test-infinitepush-*`
Reviewers: rmcelroy, mitrandir, quark, durham
Reviewed By: durham
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4299151
Tasks: 12479677
Signature: t1:4299151:1481564714:810be69447d0b35aa57328c60aab72ad374e994d
Summary:
Writing to bundlestore can take a lot of time. It doesn't make sense to do it
inside indexapi transaction
Test Plan: Run `test-infinitepush-*`
Reviewers: rmcelroy, mitrandir, durham
Reviewed By: durham
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4305903
Tasks: 12479677
Signature: t1:4305903:1481312165:6aca8c908367d9715f8ba3f5643cd56650b4456f
Summary:
@simon suggested to rename `debugbackup` because `debug-` prefix
implies that command is internal only and it's not safe for user to run it.
This is not the case for the backup because user can safely run it.
Test Plan: Run `test-infinitepush-*`
Reviewers: simon, rmcelroy, mitrandir, durham, mjpieters
Reviewed By: mjpieters
Subscribers: simon, mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4292516
Tasks: 12479677
Signature: t1:4292516:1481213170:a03c3eaebd3459887bbb3ff1c80f20977139cc90
Summary:
`hg push` takes a lock because it updates phases and may receive other bundle2
parts from remote server. We don't need it in debugbackup. And since
debugbackup is intended to run in background we want to avoid taking locks.
Instead let's use raw `unbundle()` wireproto method and disable pushback
bundle2 parts.
Test Plan: Run `test-infinitepush-*`
Reviewers: durham, rmcelroy, mitrandir, quark
Reviewed By: quark
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4291740
Tasks: 12479677
Signature: t1:4291740:1481118369:1b20f771c5db6e4fed4fc18681cc66ef4e91dd3a
Summary:
We could've avoided creating this diff if bundlerepo supported many bundles.
Unfortunately it doesn't. Adding support requires changes in upstream mercurial and it can take a lot of time.
This implementation wraps `listkeys` to set phases properly and `exchange._changegrouppart` to send more
than one bundle at a time.
It has unsolved problem with phases. For example in this situation
# Publishing server creates commit A (it is marked as draft on the server).
# Client pulls commit A from publishing server (in this case commit A is marked on the client but it's still draft on the server).
# Another client make pull from scratch bookmark. Commit A will be draft on client because we delete publishing = True from listkeys.
We assume that this case is quite rare.
Test Plan: Run `test-infinitepush-*`
Reviewers: mitrandir, rmcelroy, quark, durham
Reviewed By: durham
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4074889
Tasks: 12479677
Signature: t1:4074889:1480446669:97e7f1d8ad23d457d7984a4cde0efb2b6e89eb2e
Summary:
There are lots of breakages if upstream tests are run with infinitepush
enabled. This patch fixes one issue in test-pull-update.t test.
Test Plan:
Run `test-infinitepush-*`
Run test-pull-update.t with --extra-config-opt=extensions.infinitepush=PATH_TO_INFINITEPUSH.
Make sure that it works fine up to line 115 (later lines may still be broken)
Reviewers: durham, rmcelroy, mitrandir
Reviewed By: mitrandir
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4250678
Tasks: 12479677
Signature: t1:4250678:1480689565:792f3d77e08c969a9ffed904815fe7e9aa72f61d
Summary:
There are lots of breakages if upstream tests are run with infinitepush.
It fixes test-default-push.t upstream test.
Test Plan:
Run `test-infinitepush-*`
Run `test-default-push.t` upstream test
Reviewers: durham, rmcelroy, mitrandir
Reviewed By: mitrandir
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4250684
Tasks: 12479677
Signature: t1:4250684:1480689556:7d6bc4788cfebb5aa62c631a104e9d735eca3e47
Summary:
We want to backup bookmarks even if no new commits were added. To do this let's
save the hash of all the backuped bookmarks and backup only if saved hash is
different from the hash of the current bookmarks.
In the case when no new commits need to be backuped revs=['null'] is passed
(remotenames does the same in `push --delete`).
Test Plan: Run `test-infinitepush-*`
Reviewers: durham, rmcelroy, mitrandir, quark
Reviewed By: quark
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4251589
Tasks: 12479677
Signature: t1:4251589:1480611524:6faaa9d6d1b1aba9f7a45a4bf9a4c0df3d039ebe
Summary:
Backuping changesets is not enougn for restore. We also want to backup heads
and bookmarks to completely capture the state of the repo.
This diff is the first step in implementing this functionality. It adds new
bundle2 part which contains encoded dict (simple json encoding is used).
If value in the dict is empty then key is the bookmark pattern to delete.
If value is not empty then key is the bookmark name to save and value is a
node hash. The reason to put them in the same part is to make it possible to
delete and insert into indexapi in one transaction. It's also possible to pass
patterns to delete in part parameters but there is a bug in upstream hg that
limits parameters' size to 256 and we can potentially have longer bookmarks.
Local bookmarks are saved in infinitepush in the following form:
infinitepush/backups/USERNAME/HOSTNAME/REPOROOT/bookmarks/LOCAL_BOOKMARK_NAME
Local heads are saved in infintiepush in the following form:
infinitepush/backups/USERNAME/HOSTNAME/REPOROOTheads/HEAD_HASH
Hostname, username and repo root is necessary to distinguish different backups.
Test Plan: Run `test-infinitepush-*`
Reviewers: durham, rmcelroy, mitrandir, quark
Reviewed By: quark
Subscribers: quark, mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4245080
Tasks: 12479677
Signature: t1:4245080:1480518959:aa199d67fac4e2cd2f543651ff56fdd649dac729
Summary:
Right now `hg debugbackup` backups only changesets. We also want to backup
bookmarks. Since user may have many bookmarks let's add a separate method
that will save many bookmarks at once.
Test Plan: Will be tested in next diffs
Reviewers: durham, rmcelroy, mitrandir, quark
Reviewed By: quark
Subscribers: quark, mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4244823
Tasks: 12479677
Signature: t1:4244823:1480596116:7114128dafe2b8a10f599d05d68c1a6dce522f4a
Summary:
This methods accepts list of bookmark patterns to delete (patterns are not
supported yet, but they will be supported).
Test Plan: Will be tested in subsquent diffs
Reviewers: andrasbelo, rmcelroy, durham
Reviewed By: rmcelroy, durham
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4176016
Tasks: 12479677
Signature: t1:4176016:1479144865:8c9121970878d4d462cc2c679a414999dfdce1f4
Summary:
Not setting all default options may result in KeyValue errors.
For example pushvars extensions does
repo._shellvars = opts['pushvars']
It results in failure if 'pushvars' is not set.
Let's fix it by explicitly setting default values.
Also we need to set `allow_anon` to remotenames because otherwise
debugbackup will fail.
Test Plan:
Run `test-infinitepush-*`
Run `hg debugbackup` inside fbsource make sure there were no failures
Reviewers: rmcelroy, mitrandir, durham
Reviewed By: durham
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4237230
Tasks: 12479677
Signature: t1:4237230:1480446802:36e0630ce3e60c947be47d83bbc6deff8624048e
Summary:
Tests failed on mac. My guess is that timeout is too low.
Let's increase it to 10 seconds (it's fine because we want wait for 10 sec
if debugbackup finishes faster).
Also let's add logging
Test Plan: Run `test-infinitepush-*`
Reviewers: #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4213330
Tasks: 12479677
Summary:
`backup` was renamed to `debugbackup` a few commits ago.
Rename it there too
Test Plan: Run `test-infinitepush-*`
Reviewers: #sourcecontrol, andrasbelo
Reviewed By: andrasbelo
Subscribers: mjpieters
Differential Revision: https://phabricator.intern.facebook.com/D4212539
Tasks: 12479677
Signature: t1:4212539:1479721307:1ac25c7dc8951d655910e2b0c41b2c9555353f74
Summary:
`hg backup --background` will be used as a `txnclose` hook to backup all of the
local commits to infinitepush.
Test Plan: Run `test-infinitepush-*`
Reviewers: rmcelroy, mitrandir, durham
Reviewed By: durham
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4175953
Tasks: 12479677
Signature: t1:4175953:1479145307:e698903b519361b376f6e182db7c49869c992617
Summary:
During `hg backup` bundle with many heads may be pushed. Let's support it too.
Many heads bundle is not allowed only when we are pushing it with `--to`
because in this case we don't know the node for the bookmark.
Test Plan: Run `test-infinitepush-*`
Reviewers: rmcelroy, durham
Reviewed By: durham
Subscribers: mjpieters, #sourcecontrol
Differential Revision: https://phabricator.intern.facebook.com/D4175280
Tasks: 12479677
Signature: t1:4175280:1479231056:fb0c47a7752319d77a6cfc83a29c57e9e2dced16
