From d832ea7afa2fc9a02f87b55726ff3e4f10716885 Mon Sep 17 00:00:00 2001 From: Durham Goode Date: Wed, 16 Sep 2020 21:36:57 -0700 Subject: [PATCH] treemanifest: change local tree sending to depend on phases Summary: When sending trees and files we try to avoid sending trees that are available from the main server. To do so, we currently check to see if the tree/file is from the local store (i.e. .hg/store instead of $HGCACHE). In a future diff we'll be moving trees to use the Rust store, which doesn't expose the difference between shared and local stores. So we need to stop depending on logic to test the local store. Instead we can test if the commit is public or not, and only send the tree/file is the commit is not public. This is technically a revert of the 2018 D7992502 (https://github.com/facebookexperimental/eden/commit/5e95b0e32ec2508b189cb548240928473882bafa) diff, which stopped depending on phases because we'd receive public commits from svn there were not public on the server yet. Since svn is gone, I think it's safe go back to that way. This code was usually to help when the client was further ahead than another client and in some commit cloud edge cases, but 1) we don't do much/any p2p exchange anymore, and 2) we did some work this year to ensure clients have more up-to-date remote bookmarks during exchange (as a way of making phases and discovery more reliable), so hopefully we can rely on phases more now. Reviewed By: quark-zju Differential Revision: D23639017 fbshipit-source-id: 34c13aa2b5ef728ea53ffe692081ef443e7e57b8 --- .../tests/integration/test-commitcloud.t | 58 ++++--------------- .../hgext/remotefilelog/shallowbundle.py | 34 +---------- .../edenscm/hgext/treemanifest/__init__.py | 15 +---- .../tests/test-commitcloud-sync-bookmarks.t | 3 +- .../tests/test-commitcloud-sync-omission.t | 2 + .../test-fb-hgext-remotefilelog-bundle2.t | 12 +++- .../scm/tests/test-fb-hgext-snapshot-backup.t | 2 + .../tests/test-remotenames-selective-pull.t | 3 + 8 files changed, 34 insertions(+), 95 deletions(-) diff --git a/eden/mononoke/tests/integration/test-commitcloud.t b/eden/mononoke/tests/integration/test-commitcloud.t index ed205a97ff..6bddec25aa 100644 --- a/eden/mononoke/tests/integration/test-commitcloud.t +++ b/eden/mononoke/tests/integration/test-commitcloud.t @@ -277,62 +277,24 @@ On the first client make 2 stacks Make one of the commits public when it shouldn't be. $ hgmn debugmakepublic 8d621fa11677 + $ hgmn cloud sync 2>&1 | grep "Error while uploading data for changesets" + remote: Error while uploading data for changesets, hashes: [HgChangesetId(HgNodeHash(Sha1(ec61bf312a03c1ae89f421ca46eba7fc8801129e)))] + remote: context: "Error while uploading data for changesets, hashes: [HgChangesetId(HgNodeHash(Sha1(ec61bf312a03c1ae89f421ca46eba7fc8801129e)))]", + remote: Error while uploading data for changesets, hashes: [HgChangesetId(HgNodeHash(Sha1(8d621fa1167779dffcefe5cb813fc11f2f272874))), HgChangesetId(HgNodeHash(Sha1(ec61bf312a03c1ae89f421ca46eba7fc8801129e)))] + remote: context: "Error while uploading data for changesets, hashes: [HgChangesetId(HgNodeHash(Sha1(8d621fa1167779dffcefe5cb813fc11f2f272874))), HgChangesetId(HgNodeHash(Sha1(ec61bf312a03c1ae89f421ca46eba7fc8801129e)))]", + + $ hgmn debugmakepublic --delete 8d621fa11677 $ hgmn cloud sync commitcloud: synchronizing 'client1' with 'user/test/default' - backing up stack rooted at ec61bf312a03 - remote: Command failed - remote: Error: - remote: bundle2_resolver error - remote: - remote: Root cause: - remote: Bonsai changeset not found for hg changeset 8d621fa1167779dffcefe5cb813fc11f2f272874 - remote: - remote: Caused by: - remote: Error while uploading data for changesets, hashes: [HgChangesetId(HgNodeHash(Sha1(ec61bf312a03c1ae89f421ca46eba7fc8801129e)))] - remote: Caused by: - remote: While creating Changeset Some(HgNodeHash(Sha1(ec61bf312a03c1ae89f421ca46eba7fc8801129e))), uuid: * (glob) - remote: Caused by: - remote: While waiting for parents to complete - remote: Caused by: - remote: Bonsai changeset not found for hg changeset 8d621fa1167779dffcefe5cb813fc11f2f272874 - remote: - remote: Debug context: - remote: Error { - remote: context: "bundle2_resolver error", - remote: source: Error { - remote: context: "Error while uploading data for changesets, hashes: [HgChangesetId(HgNodeHash(Sha1(ec61bf312a03c1ae89f421ca46eba7fc8801129e)))]", - remote: source: SharedError { - remote: error: Error { - remote: context: "While creating Changeset Some(HgNodeHash(Sha1(ec61bf312a03c1ae89f421ca46eba7fc8801129e))), uuid: *", (glob) - remote: source: Error { - remote: context: "While waiting for parents to complete", - remote: source: SharedError { - remote: error: BonsaiMappingNotFound( - remote: HgChangesetId( - remote: HgNodeHash( - remote: Sha1(8d621fa1167779dffcefe5cb813fc11f2f272874), - remote: ), - remote: ), - remote: ), - remote: }, - remote: }, - remote: }, - remote: }, - remote: }, - remote: } - push failed: stream ended unexpectedly (got 0 bytes, expected 4) - retrying push with discovery - searching for changes - backing up stack rooted at 77a917e6c3a5 + backing up stack rooted at 8d621fa11677 commitcloud: commits synchronized - finished in * (glob) - + finished in 0.00 sec Commit still becomes available in the other repo $ cd ../client2 $ hgmn cloud sync commitcloud: synchronizing 'client2' with 'user/test/default' - pulling ec61bf312a03 88d416aed919 from ssh://user@dummy/repo + pulling 88d416aed919 ec61bf312a03 from ssh://user@dummy/repo searching for changes adding changesets adding manifests diff --git a/eden/scm/edenscm/hgext/remotefilelog/shallowbundle.py b/eden/scm/edenscm/hgext/remotefilelog/shallowbundle.py index d8f8bf12ec..753b7aa0c4 100644 --- a/eden/scm/edenscm/hgext/remotefilelog/shallowbundle.py +++ b/eden/scm/edenscm/hgext/remotefilelog/shallowbundle.py @@ -141,34 +141,6 @@ class shallowcg1packer(changegroup.cg1packer): else: # If not using the fast path, we need to discover what files to send if not fastpathlinkrev: - localmfstore = None - if len(repo.manifestlog.localdatastores) > 0: - localmfstore = repo.manifestlog.localdatastores[0] - sharedmfstore = None - if len(repo.manifestlog.shareddatastores) > 0: - sharedmfstore = contentstore.unioncontentstore( - *repo.manifestlog.shareddatastores - ) - - def containslocalfiles(mfnode): - # This is a local tree, then it contains local files. - if localmfstore and not localmfstore.getmissing([("", mfnode)]): - return True - - # If not a local tree, and it doesn't exist in the store, - # then it is to be generated and may contain local files. - # This can happen while serving an infinitepush bundle that - # contains flat manifests. It will need to generate trees - # for that manifest. - if ( - repo.svfs.treemanifestserver - and sharedmfstore - and sharedmfstore.getmissing([("", mfnode)]) - ): - return True - - return False - # If we're sending files, we need to process the manifests filestosend = self.shouldaddfilegroups(source) if filestosend is not NoFiles: @@ -176,11 +148,11 @@ class shallowcg1packer(changegroup.cg1packer): with progress.bar(repo.ui, _("manifests"), total=len(mfs)) as prog: for mfnode, clnode in pycompat.iteritems(mfs): prog.value += 1 - if filestosend == LocalFiles and not containslocalfiles( - mfnode + if ( + filestosend == LocalFiles + and repo[clnode].phase() == phases.public ): continue - try: mfctx = mflog[mfnode] p1node = mfctx.parents[0] diff --git a/eden/scm/edenscm/hgext/treemanifest/__init__.py b/eden/scm/edenscm/hgext/treemanifest/__init__.py index a3117a1467..417e7bfd96 100644 --- a/eden/scm/edenscm/hgext/treemanifest/__init__.py +++ b/eden/scm/edenscm/hgext/treemanifest/__init__.py @@ -2121,25 +2121,14 @@ def createtreepackpart(repo, outgoing, partname, sendtrees=shallowbundle.AllTree basemfnodes = [] directories = [] - localmfstore = None - if len(repo.manifestlog.localdatastores) > 0: - localmfstore = repo.manifestlog.localdatastores[0] - - def shouldsend(ctx): - mfnode = ctx.manifestnode() - if sendtrees == shallowbundle.AllTrees or ctx.phase() != phases.public: - return True - - # Else LocalTrees - return localmfstore and not localmfstore.getmissing([("", mfnode)]) - linknodemap = {} for node in outgoing.missing: ctx = repo[node] - if shouldsend(ctx): + if sendtrees == shallowbundle.AllTrees or ctx.phase() != phases.public: mfnode = ctx.manifestnode() mfnodes.append(mfnode) linknodemap.setdefault(mfnode, node) + basectxs = repo.set("parents(%ln) - %ln", outgoing.missing, outgoing.missing) for basectx in basectxs: basemfnodes.append(basectx.manifestnode()) diff --git a/eden/scm/tests/test-commitcloud-sync-bookmarks.t b/eden/scm/tests/test-commitcloud-sync-bookmarks.t index 2a6d5e077f..6dc5012a2b 100644 --- a/eden/scm/tests/test-commitcloud-sync-bookmarks.t +++ b/eden/scm/tests/test-commitcloud-sync-bookmarks.t @@ -1,6 +1,6 @@ #chg-compatible - $ enable amend commitcloud infinitepush rebase remotenames pullcreatemarkers + $ enable amend commitcloud infinitepush rebase remotenames treemanifest pullcreatemarkers $ configure dummyssh $ setconfig commitcloud.hostname=testhost $ setconfig remotefilelog.reponame=server @@ -9,6 +9,7 @@ $ cd server $ setconfig infinitepush.server=yes infinitepush.reponame=testrepo $ setconfig infinitepush.indextype=disk infinitepush.storetype=disk + $ setconfig treemanifest.server=True $ touch base $ hg commit -Aqm base $ hg bookmark master diff --git a/eden/scm/tests/test-commitcloud-sync-omission.t b/eden/scm/tests/test-commitcloud-sync-omission.t index 299fc4a9d2..e4e1f833c9 100644 --- a/eden/scm/tests/test-commitcloud-sync-omission.t +++ b/eden/scm/tests/test-commitcloud-sync-omission.t @@ -18,6 +18,8 @@ > indextype = disk > storetype = disk > reponame = testrepo + > [treemanifest] + > server = True > EOF $ touch base $ hg commit -Aqm base diff --git a/eden/scm/tests/test-fb-hgext-remotefilelog-bundle2.t b/eden/scm/tests/test-fb-hgext-remotefilelog-bundle2.t index 81506b7c2d..1bcd3854bd 100644 --- a/eden/scm/tests/test-fb-hgext-remotefilelog-bundle2.t +++ b/eden/scm/tests/test-fb-hgext-remotefilelog-bundle2.t @@ -15,6 +15,8 @@ preferuncompressed = False so that we can make both generaldelta and non-general > bundle2-exp = True > [server] > preferuncompressed = False + > [treemanifest] + > server=True > EOF $ echo x > x $ hg commit -qAm x @@ -23,6 +25,8 @@ preferuncompressed = False so that we can make both generaldelta and non-general $ hgcloneshallow ssh://user@dummy/master shallow-generaldelta -q --pull --config experimental.bundle2-exp=True 1 files fetched over 1 fetches - (1 misses, 0.00% hit ratio) over *s (glob) (?) + fetching tree '' bc0c2c938b929f98b1c31a8c5994396ebb096bf0 + 1 trees fetched over 0.00s $ grep generaldelta shallow-generaldelta/.hg/requires generaldelta $ hgcloneshallow ssh://user@dummy/master shallow-plain -q --pull --config format.usegeneraldelta=False --config format.generaldelta=False --config experimental.bundle2-exp=True @@ -42,8 +46,10 @@ pull from generaldelta to generaldelta adding manifests adding file changes added 1 changesets with 0 changes to 0 files - 1 files updated, 0 files merged, 0 files removed, 0 files unresolved 1 files fetched over 1 fetches - (1 misses, 0.00% hit ratio) over *s (glob) (?) + fetching tree '' 126c4ddee02e922d5f05b4304b80e383a53a82e6, found via 2fbb8bb2b903 + 1 trees fetched over 0.00s + 1 files updated, 0 files merged, 0 files removed, 0 files unresolved push from generaldelta to generaldelta $ echo b > b $ hg commit -qAm b @@ -63,8 +69,10 @@ pull from generaldelta to non-generaldelta adding manifests adding file changes added 2 changesets with 0 changes to 0 files - 2 files updated, 0 files merged, 0 files removed, 0 files unresolved 1 files fetched over 1 fetches - (1 misses, 0.00% hit ratio) over *s (glob) (?) + fetching tree '' bc905f0fce7a7e7dfb60db06ddf9df54b3983840, found via d6788bd632ca + 1 trees fetched over 0.00s + 2 files updated, 0 files merged, 0 files removed, 0 files unresolved push from non-generaldelta to generaldelta $ echo c > c $ hg commit -qAm c diff --git a/eden/scm/tests/test-fb-hgext-snapshot-backup.t b/eden/scm/tests/test-fb-hgext-snapshot-backup.t index 2dcb8b35a4..02df232573 100644 --- a/eden/scm/tests/test-fb-hgext-snapshot-backup.t +++ b/eden/scm/tests/test-fb-hgext-snapshot-backup.t @@ -14,6 +14,7 @@ $ hg init server $ cd server $ setupserver + $ setconfig treemanifest.server=True $ hg bookmark master $ cd .. @@ -138,6 +139,7 @@ $ cd ../restored $ hg checkout "$OID" pulling '751f5ef10bc73a8f549197b380773d4f680daa8c' from 'ssh://user@dummy/server' + 2 trees fetched over 0.00s abort: 751f5ef10bc7 is a snapshot, set ui.allow-checkout-snapshot config to True to checkout on it [255] diff --git a/eden/scm/tests/test-remotenames-selective-pull.t b/eden/scm/tests/test-remotenames-selective-pull.t index da024cc46e..8f0045aa1d 100644 --- a/eden/scm/tests/test-remotenames-selective-pull.t +++ b/eden/scm/tests/test-remotenames-selective-pull.t @@ -5,6 +5,9 @@ Set up repos $ hg init remoterepo + $ cd remoterepo + $ setconfig treemanifest.server=True + $ cd .. $ hg clone -q ssh://user@dummy/remoterepo localrepo Pull master bookmark