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
