The impetus of this is the extreme load we place the try repository under, to the point of causing many failed clones. We need to stop doing full repo clones whenever it's avoidable. I don't think this necessarily has to be "use a shared source directory", it could be as simple as "don't clobber repos in try/nightly builds; use make clean/hg up -C". Lots of other possibilities. Leaving in triage because I strongly feel that this should be done in the next few weeks.

I just ran this in staging and found that after the initial cloning of the /source on the slave, the time to do the hg_update step was greatly improved and certainly would help with network usage. From 12 minutes on a clean checkout of source to 4 minutes for a copy (when /source was already present). Considering it took 34 minutes for hg_update to run on the most recent try build, this is a huge improvement.

did you test this for the mobile builds? i don't know for sure, but it is probably still clobbering mobile-browser. Mobile-browser is a much smaller repo, so it probably doesn't matter at this point.

Does mode=copy clobber the objdir ? I'm guessing not.

http://hg.mozilla.org/build/buildbot/file/d6cc48dea8f8/buildbot/steps/source.py#l54 says yes.

Nice, carry on!

Lukas, can you point me at a log and slave that is doing this? I'd like to poke around a little bit at it. Thanks a ton for taking this on.

Filing a dep bug to upgrade to Mercurial 1.6.2 on the build slaves because in some cases we hit "414 Request-URI Too Large" when trying to pull the repo due to a mercurial issue with sending all the heads in the query (http://mercurial.selenic.com/bts/issue1650), this is an issue on try obviously cause it's got a lot of heads. Two options were discussed, one would be patching buildbot to send the --rev in as is done in clobber build pulls (http://mxr.mozilla.org/build-central/source/buildbot/buildbot/slave/commands/vc s.py#1472) but upgrading Mercurial will fix this cleaner, easier and with other benefits to our automation.

Comment on attachment 468543 [details] [diff] [review] [tested] change try repo source step to 'copy' mode This patch should do it for most of the builds, I think you still need to update MaemoTryBuildFactory in tryserverfactory.py though. And of course, we need Mercurial updated before we can roll it out.

(In reply to comment #8) > This patch should do it for most of the builds, I think you still need to > update MaemoTryBuildFactory in tryserverfactory.py though. actually, that file isn't used for mobile try builds afaik. Try server is using the standard MaemoBuildFactory with a special try_mode factory argument. If we aren't using this file anymore (i dont think we are), could we hg rm it?

yes, please!

ideas from #mozbuild: < catlee> bhearsum: what do you think developing something like 'hgtool' that is permanently installed on the slaves. it would know how to do clones from a shared directory on the slave. < bhearsum> i think having such a tool is a great idea. what's the motivation for it being on the slaves vs. in the tools repo? < catlee> bootstrapping problem < bhearsum> ah < bhearsum> right :-) < catlee> we do rm -rf srcdir; hg clone ... srcdir way too much < catlee> with configs, tools, etc. < bhearsum> well, that's a pretty good reason....it's going to be a PITA to deploy updates to such a tool, though < catlee> nah < bhearsum> it'll take a day or so for them to get around to everything < catlee> puppet/opsi < catlee> yeah < catlee> anyway, that makes your release clones fast < bhearsum> that's the only negative i can see, though < catlee> then we can get rid of Mercurial too < bhearsum> oh, it would keep all repositories, in a central place? < catlee> yeah < bhearsum> +++ < catlee> like /builds/slave/repos < catlee> (cd /builds/slaves/repos; if [ -d mozilla-central ]; then hg -R mozilla-central pull); hg clone /builds/slaves/repo/mozilla-central srcdir < bhearsum> yeah < bhearsum> hopefully with some error recovery :-) < catlee> yes :) < bhearsum> i like this more the more i think about it < catlee> and then, with hg >= 1.3, we can use the share extention < catlee> which speeds up the local clone quite a bit * bhearsum googles that < catlee> it basically shares .hg across working directories < bhearsum> ah cool < bhearsum> so a clone is just "hg up" then < bhearsum> timewise, i mean < bhearsum> (which might even use hardlinks on the local fs?) < catlee> not sure if it uses hardlinks or not < bhearsum> either way, still a big speed up < catlee> yup < catlee> on my laptop, hg clone mozilla-central test takes 8 minutes < catlee> hg share mozilla-central test takes 4 < bhearsum> huh, i'm surprised it takes that long, even < bhearsum> oh < bhearsum> no, i get it now < bhearsum> hg clone from hg.m.o would take even longer < catlee> yeah < catlee> alright! let's get to it!

Some thoughts that catlee commented on IRC: [10:30am] catlee: my approach would be to modify hgtool to accept a -s <shared_dir> argument [10:30am] catlee: which would make it clone/update the shared_dir first, and then update the working copy [10:31am] armenzg: would the working dir be kept away from the shared_dir? [10:32am] catlee: yes [10:32am] catlee: e.g you'd have repos cloned in /builds/slave/repos [10:33am] catlee: so hgtool would do something like 'hg -R /builds/slave/repos/mozilla-central pull; hg clone /builds/slave/repos/mozilla-central /builds/slave/linux-try-build/build' [10:33am] catlee: err, except with the try repo [10:34am] catlee: we'd also use it for buildbot-configs, build-tools [10:34am] catlee: which are used for almost everything

Tested this patch locally with hg 1.4.3. This will require a change to how hgtool.py is invoked, as if -s is not specified, it does a full clone/pull to the working dir as before. The other thing I learned whilst implementing this is that history changes are *not* isolated between these hg shares. So a commit, tag, etc made on share `foo` will affect the history of the original shared clone, and all other shares the originate from that clone. Not sure if that affects us or not, but I thought it was worth mentioning.

Comment on attachment 484099 [details] [diff] [review] use share extension to pull/clone to shared source checkout Seems fine to me, please add a test or two for it though :-).

Two main changes: I've added a few unit tests to the lib/python suite to test the share command. One problem I came across in testing this further was that if a non-shared repo already exists, the share command immediately assumes it's a shared clone and simply updates it. With this patch, I explicitly check for the hg sharedpath and if the destination path exists but is not a shared clone, it gets clobbered.

Comment on attachment 484379 [details] [diff] [review] shared_source checkout w/ unit tests Yeah, the share extension really shares everything...which is fine for our purposes I think. So, this adds a dependency on the share extension. Is it worth falling back to just doing a local clone/pull/update in the case where the share extension isn't available?

Comment on attachment 484379 [details] [diff] [review] shared_source checkout w/ unit tests Per the meeting yesterday, this is still being worked on.

by passing in -o|--outgoing-changes hgtool.py now checks the share for locally committed outgoing change-sets, and clobbers if any are found. This change means we have to be extra careful when updating shares, since shares might suddenly find unknown parent revisions appearing in their histories when they try to update. Added another test case as well to test this scenario.

Comment on attachment 485069 [details] [diff] [review] check outgoing changes w/ -o >+ def testMercurialShareOutgoing(self): >+ # ensure that outgoing changesets in a clone always get clobbered >+ repo5 = os.path.join(self.tmpdir, 'repo5') >+ repo6 = os.path.join(self.tmpdir, 'repo6') >+ mercurial(self.repodir, repo5) >+ share(repo5, repo6) >+ open(os.path.join(repo6, 'test.txt'), 'w').write("hello!") >+ # modify the history of the new clone >+ run_cmd(['hg', 'add', os.path.join(repo6, 'test.txt')], cwd=repo6) Does this add work? The cwd is repo6 so you should only need to say 'hg add test.txt'. >+ run_cmd(['hg', 'commit', '-m', 'adding changeset'], cwd=repo6) >+ self.assertNotEquals(self.revisions, getRevisions(repo6)) You should run the same assertion against repo5, and for completeness, it'd be good to assert that repo5 and repo6 are the same. >+ # clone the mercurial directory; ensure that the outgoing changesets are clobbered >+ # and that the working directory has been purged >+ if outgoing_changes(repo5): >+ remove_path(repo5) >+ mercurial(self.repodir, repo5) >+ share(repo5, repo6) >+ self.failUnless(not os.path.exists(os.path.join(repo6, 'test.txt'))) >+ self.assertEquals(self.revisions, getRevisions(repo6)) This test needs a bit of work. It looks like you're testing two different things in one test. Everything after the first assertNotEquals should move to a different test. I'm not really sure what the second part is testing -- that if you share to directory, then share to it again, that it gets overwritten? I'm not really sure why you have the if outgoing_changes() check either. >+def share(source, dest, branch=None, revision=None): >+ """Uses the hg share extension to update a working dir from a shared repo >+ """ >+ if os.path.exists(os.path.join(dest, ".hg", "sharedpath")): >+ log.debug("path exists, just going to update") >+ #attempt to update from the shared history. If this fails >+ try: >+ return update(dest, branch, revision) >+ except subprocess.CalledProcessError: >+ remove_path(dest) >+ elif os.path.exists(dest): >+ log.warning("Error %s exists and is not an hg share! Clobbering", dest) >+ remove_path(dest) >+ log.debug("sharing hg repo to %s" % dest) >+ #fall back to creating a local clone if the share extension is unavailable >+ try: >+ cmd = ['hg', 'share', source, dest] >+ run_cmd(cmd) >+ #if it fails for whatever reason, use mercurial() to force a local clone >+ except subprocess.CalledProcessError: Hmm, I don't think you're merely catching the share extension being unavailable here. You're going to need to save the output from the command and check it to determine that. >+ return mercurial(source, dest, branch, revision) >+ return update(dest, branch, revision) You can move this update() to inside of the try block for better readability. >+ >+def outgoing_changes(repo, remote=None): Hmmm, this is going to conflict with the out() function I wrote in bug 508896. Can you just pull it in, instead, and modify it if you need to?

I've refined that unit test a lot based on ben's comments. I think I was trying to fit in testing the outgoing_changes functionality and making sure that share and clone played nicely with it, all in one test. I've now restricted that test to simply test the share functionality of shared histories. I've also removed the outgoing_changes function entirely, since it will be added soon in the patch for bug 508896. We don't actually capture stderr if we receive a subProcess.CalledProcessError, and the returncode for a missing extension (255) doesn't uniquely identify the problem. I have settled for errlogging the problem and continuing with a full clone, which I think is what we want anyway, as the error logs should get caught if anything goes wrong without derailing hgtool.py. Once the problem (whatever it may have been gets fixed), the share() function automatically replaces the local clone with a share, since it checks for the sharedpath file every time.

Comment on attachment 485417 [details] [diff] [review] refine unit tests, add more stuff to >+def share(source, dest, branch=None, revision=None): >+ """Uses the hg share extension to update a working dir from a shared repo >+ """ >+ if os.path.exists(os.path.join(dest, ".hg", "sharedpath")): >+ log.debug("path exists, just going to update") >+ #attempt to update from the shared history. If this fails >+ try: >+ return update(dest, branch, revision) >+ except subprocess.CalledProcessError: >+ remove_path(dest) >+ elif os.path.exists(dest): >+ log.warning("Error %s exists and is not an hg share! Clobbering", dest) >+ remove_path(dest) >+ log.debug("sharing hg repo to %s" % dest) >+ #fall back to creating a local clone if the share extension is unavailable >+ #or if share fails for any other reason >+ try: >+ cmd = ['hg', 'share', source, dest] >+ run_cmd(cmd) >+ return update(dest, branch, revision) >+ #if it fails for whatever reason, use mercurial() to force a local clone >+ except subprocess.CalledProcessError: >+ log.error("Failed to hg_share on hg version %s.%s.%s, attemping local clone" % hg_ver()) >+ return mercurial(source, dest, branch, revision) If we don't have the share extension, I think this will clobber the checkout every time (the first check for .hg/sharedpath fails, but dest exists).

You're right, since the except block catches this case anyway, it can make a much smarter decision about whether to clobber since the mercurial() call will try pull && up before clobbering and cloning.

Comment on attachment 485429 [details] [diff] [review] removes redundant clobber You're going to need to include out() and common_args() in your patch, because this is likely to land before bug 508896, but this looks fine.

Comment on attachment 485429 [details] [diff] [review] removes redundant clobber I wouldn't have '-o' as the short version of --check-outgoing. I'm not sure we need a short version for this option, it won't be used that often. You do need to put 'outgoing' in the list of defaults though. Does 'dest' need to be clobbered too in the case where there are outgoing patches? In the share method, I wouldn't rely on hg_ver() returning a 3-tuple. Just use %s in the debug message.

Done, passes unit tests and a few quick smoke tests. Since it's essentially the same functionality as before, just subbing in the conflicting functions from your patch for 508896, I haven't gone through too much additional testing.

Comment on attachment 485868 [details] [diff] [review] include out() common_args() from bug 508896 Hmm, I'm a little surprised the tests still pass, there's at least one that's calling pull without proper keyword args: http://hg.mozilla.org/build/tools/file/b5160161827d/lib/python/test/test_util_hg.py#l102. lgtm otherwise

(In reply to comment #25) > Created attachment 485868 [details] [diff] [review] > include out() common_args() from bug 508896 > > Done, passes unit tests and a few quick smoke tests. Since it's essentially the > same functionality as before, just subbing in the conflicting functions from > your patch for 508896, I haven't gone through too much additional testing. Did you see comment #24?

Sorry about that, included recommended changes from comment #24.

based on IRC conversation with catlee, there is a possible failure case in the old patch where a mercurial local clone might have outgoing changesets and the share extension is unavailable, the outgoing changeset will not get clobbered immediately.

Comment on attachment 486201 [details] [diff] [review] ensure that both destination and shared-checkout are clobbered if we detect outgoing changesets > parser.set_defaults( > revision=os.environ.get('HG_REV'), > branch=os.environ.get('HG_BRANCH', 'default'), >+ outgoing=None, Since this is being used a boolean value, would False make more sense here? >- got_revision = mercurial(repo, dest, options.branch, options.revision) >+ #look for and clobber outgoing changesets >+ if options.outgoing: >+ if out(dest): >+ remove_path(dest) >+ if options.shared_dir and out(options.shared_dir): >+ remove_path(hgdir) hgdir isn't defined anywhere.

Also noticed that the usage string didn't match the option, so I fixed that as well. In addition, after some more testing, I noticed that hg out returns 1 if no outgoing changesets are found, which raises an exception, so I have wrapped the call with a try..except block to catch that specific case for ret==1

TOuches code that nanojit/fuzzing use, should land in a reconfig window. Note: landing this doesn't enable shared source dirs, just adds support for them in hgtool.py.

Comment on attachment 486341 [details] [diff] [review] address issues and fix usage statement changeset: 882:294bfc3eac25

Need to create this directory so we have a good place to store shared clones. I'll also be appending the following to .hgrc when this lands: [extensions] share=

So..I think my previous patch was a bit hasty. When I went to integrate it with my l10n work I realized that I had to use share() everywhere -- including library functions. I don't think that's the right thing to do, especially library functions, so I wrote this. This should have no behaviour change unless HG_SHARE_BASE_DIR is set in the environment, or a consumer overrides util.hg.SHARE_BASE_DIR. With this, we can set HG_SHARE_BASE_DIR per platform in the buildbot configs and should end up using share in anything using the mercurial() method, including hgtool.py uses.

Comment on attachment 490170 [details] [diff] [review] integrate hg share into mercurial() function New patch incoming soon based on a conversation with catlee.

We can't use share on 32-bit Mac yet because of an old hg version, but we may as well create the directory.

This comes out of a conversation with Catlee on Friday. His main objection to the prior patch was how I was using cloneFunc and SHARE_BASE_DIR being at module level. This patch should address both of those. It also works better -- the previous one didn't work properly in cases where the working directory existed and sharing was enabled. Passes unit tests and works fine in my l10n builders.

Forgot to update hgtool.py in v2.

Comment on attachment 490924 [details] [diff] [review] v3, with hgtool.py updated >-def mercurial(repo, dest, branch=None, revision=None): >+def mercurial(repo, dest, branch=None, revision=None, >+ shareBase=DefaultShareBase): > """Makes sure that `dest` is has `revision` or `branch` checked out from > `repo`. > > Do what it takes to make that happen, including possibly clobbering > dest.""" >+ dest = os.path.abspath(dest) >+ if shareBase is DefaultShareBase: >+ shareBase = os.environ.get("HG_SHARE_BASE_DIR", None) > >- # If dest exists, try pulling first >- log.debug("mercurial: %s %s", repo, dest) >+ # If the working directory already exists and isn't using share we update >+ # the working directory directly from the repo, ignoring the sharing >+ # settings > if os.path.exists(dest): >- log.debug("%s exists, pulling", dest) >+ if not os.path.exists(os.path.join(dest, ".hg", "sharedpath")): >+ try: >+ return pull(repo, dest, branch=branch, revision=revision) >+ except subprocess.CalledProcessError: >+ log.warning("Error pulling changes into %s from %s; clobbering", dest, repo) >+ log.debug("Exception:", exc_info=True) >+ remove_path(dest) >+ >+ # If that fails for any reason, and sharing is requested, we'll try to >+ # update the shared repository, and then update the working directory from >+ # that. >+ if shareBase: >+ sharedRepo = os.path.join(shareBase, get_repo_path(repo)) > try: >- # TODO: If revision is set, try updating before pulling? >- return pull(repo, dest, branch=branch, revision=revision) >+ mercurial(repo, sharedRepo, shareBase=None) Want to pass in branch and revision here? > def share(source, dest, branch=None, revision=None): >- """Uses the hg share extension to update a working dir from a shared repo >- """ Please update the docstring to explain what this function now does.

(In reply to comment #40) > >+ if shareBase: > >+ sharedRepo = os.path.join(shareBase, get_repo_path(repo)) > > try: > >- # TODO: If revision is set, try updating before pulling? > >- return pull(repo, dest, branch=branch, revision=revision) > >+ mercurial(repo, sharedRepo, shareBase=None) > > Want to pass in branch and revision here? > Yeah, I guess we should. No need to pull what we won't be updating to.

Comment on attachment 490924 [details] [diff] [review] v3, with hgtool.py updated changeset: 1265:e1d19ab31284

I updated all of the Puppet masters for this change, I've seen no issues with it.

Left to do here: use hg share on Mac, Windows. Use hgtool.py to clone everything rather than the Buildbot classes.

adds useSharedCheckouts to MercurialBuildFactory, which defaults to False. This can be set per platform / per branch in config.py. Currently only TryBuildFactory pays attention to it, but porting won't be hard once it's proven to work.

Comment on attachment 508430 [details] [diff] [review] add useSharedCheckouts to MercurialBuildFactory We should drop the hg_try_lock or bump it up down the road, when most/all machines already have most of the repo. Do we know what happens to the shared checkout when the try repo is reset? I'd hope mercurial() would blow it away and reclone, but I'm not 100% sure.

(In reply to comment #46) > Comment on attachment 508430 [details] [diff] [review] > --> https://bugzilla.mozilla.org/attachment.cgi?id=508430 > add useSharedCheckouts to MercurialBuildFactory > > We should drop the hg_try_lock or bump it up down the road, when most/all > machines already have most of the repo. > > Do we know what happens to the shared checkout when the try repo is reset? I'd > hope mercurial() would blow it away and reclone, but I'm not 100% sure. By my testing it looks like hgtool does the right thing if the repo is reset: it blows away the shared repo and tries again. It doesn't however handle the case if you try and pull an completely unrelated repo into your shared working copy. I'll fix that up.

Comment on attachment 508498 [details] [diff] [review] Set enable_shared_checkouts to True for linux platforms I think you need to update preproduction_config.py, too. Looks fine otherwise.

- clone now defaults to using -U if we're not updating the destination - clone won't pass branch to hg if a revision is set - mercurial checks that share extension works before trying to use it - if share fails, we will clone from remote repo instead of from local shared repo - more tests!

Comment on attachment 508756 [details] [diff] [review] hgtool.py, hg.py changes Can we assert at the top of clone, et. al that revision and branch aren't both passed? Allowing both seems like it'd be confusing to consumers since only revision would be obeyed. Looks fine otherwise.

Now with mac64 and pre-production goodness!

Comment on attachment 508430 [details] [diff] [review] add useSharedCheckouts to MercurialBuildFactory http://hg.mozilla.org/build/buildbot-configs/rev/09bcb624bdbc

Comment on attachment 508756 [details] [diff] [review] hgtool.py, hg.py changes http://hg.mozilla.org/build/tools/rev/e4dd9afd34bd

Comment on attachment 508850 [details] [diff] [review] Set enable_shared_checkouts to True for linux,mac64 platforms http://hg.mozilla.org/build/buildbotcustom/rev/a8b584729600

mercurial-1.7.5-x86.msi will be in opsi-binaries

When we're comparing the shared path in a working copy vs. our calculated shared path, we need to normalize paths to account for / vs \ fun.

Comment on attachment 511373 [details] [diff] [review] Handle windows paths properly http://hg.mozilla.org/build/tools/rev/68753d5ad455

Comment on attachment 512160 [details] [diff] [review] Set enable_shared_checkouts to True for win32 platforms http://hg.mozilla.org/build/buildbot-configs/rev/d3d54dd41579

Comment on attachment 513489 [details] [diff] [review] ...and finally on macosx (10.5) Woot!

Comment on attachment 513489 [details] [diff] [review] ...and finally on macosx (10.5) http://hg.mozilla.org/build/buildbot-configs/rev/6d410ecbf114

Comment on attachment 514187 [details] [diff] [review] for realz on 10.5! http://hg.mozilla.org/build/buildbot-configs/rev/08c9e8185318