Switch off repo sync --jobs, and turn on repo --trace.

Switch off repo sync --jobs, and turn on repo --trace.

repo sync --jobs doesn't check for errors, so we shouldn't use it in
automated scripts. repo --trace is needed so that we can diagnose
whether git is hanging.

BUG=chromium-os:7048, chromium-os:6774
TEST=Ran test suite

Change-Id: I9e68cfffe841a231f5fabef951ea2d45b81c1d5e

Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=7bf8d27

[anush, 2010-09-25 17:34:31]

LGTM. So is this only temporary? i.e once we get our new git infra in place
can we switch back to regular "repo sync". Also hopefully it doesn't add too
much to the build log clutter.

Anush

On Fri, Sep 24, 2010 at 10:32 PM, <davidjames@chromium.org> wrote:

> Reviewers: sosa, anush,
>
> Description:
> Switch off repo sync --jobs, and turn on repo --trace.
>
> repo sync --jobs doesn't check for errors, so we shouldn't use it in
> automated scripts. repo --trace is needed so that we can diagnose
> whether git is hanging.
>
> BUG=chromium-os:7048, chromium-os:6813
> TEST=Ran test suite
>
> Change-Id: I9e68cfffe841a231f5fabef951ea2d45b81c1d5e
>
> Please review this at http://codereview.chromium.org/3419024/show
>
> SVN Base: http://git.chromium.org/git/crosutils.git
>
> Affected files:
>  M bin/cbuildbot.py
>
>
> Index: bin/cbuildbot.py
> diff --git a/bin/cbuildbot.py b/bin/cbuildbot.py
> index
>
cd9a3a5bebfefde70c2ba5afb9ab67da897c35a1..14e22d1042f9b40cda78111e6db0fbb96037ba2b
> 100755
> --- a/bin/cbuildbot.py
> +++ b/bin/cbuildbot.py
> @@ -47,13 +47,12 @@ def RepoSync(buildroot, rw_checkout=False,
> retries=_DEFAULT_RETRIES):
>   retries -- Number of retries to try before failing on the sync.
>
>   """
> -  # Get the number of processors to use with repo sync.
> -  num_procs = int(RunCommand('grep -c processor /proc/cpuinfo'.split(),
> -                             print_cmd=False, redirect_stdout=True))
> -
>   while retries > 0:
>     try:
> -      RunCommand(['repo', 'sync', '--jobs=%d' % (num_procs)],
> cwd=buildroot)
> +      # The --trace option ensures that repo shows the output from git.
> This
> +      # is needed so that the buildbot can kill us if git is not making
> +      # progress.
> +      RunCommand(['repo', '--trace', 'sync'], cwd=buildroot)
>       if rw_checkout:
>         # Always re-run in case of new git repos or repo sync
>         # failed in a previous run because of a forced Stop Build.
>
>
>

[davidjames, 2010-09-25 18:28:08]

On 2010/09/25 17:34:31, anush wrote:
> LGTM. So is this only temporary? i.e once we get our new git infra in place
> can we switch back to regular "repo sync". Also hopefully it doesn't add too
> much to the build log clutter.

If 'repo' is taking 30 minutes to sync, and there's no output, it's quite
difficult to know if it's just slow or if it's actually hung. By adding --trace,
we get repo to actually print out what is happening, so we can see whether it's
hung or not.

Even with the new infrastructure, it's still possible that git might hang. So I
think --trace is generally a good idea.

If you're concerned with log spam, it might be a good idea to work with a less
verbose version of --trace that will still allow us to detect hangs but wouldn't
spam with as much output.

[sosa, 2010-09-25 21:33:41]

Can't we just add functionality to repo to add functionality that
combines both sync and trace?  I would really prefer to not slow this
down.  I'd honestly rather see it hang every now and have a smaller
timeout.

On Sat, Sep 25, 2010 at 2:28 PM,  <davidjames@chromium.org> wrote:
> On 2010/09/25 17:34:31, anush wrote:
>>
>> LGTM. So is this only temporary? i.e once we get our new git infra in
>> place
>> can we switch back to regular "repo sync". Also hopefully it doesn't add
>> too
>> much to the build log clutter.
>
> If 'repo' is taking 30 minutes to sync, and there's no output, it's quite
> difficult to know if it's just slow or if it's actually hung. By adding
> --trace,
> we get repo to actually print out what is happening, so we can see whether
> it's
> hung or not.
>
> Even with the new infrastructure, it's still possible that git might hang.
> So I
> think --trace is generally a good idea.
>
> If you're concerned with log spam, it might be a good idea to work with a
> less
> verbose version of --trace that will still allow us to detect hangs but
> wouldn't
> spam with as much output.
>
>
> http://codereview.chromium.org/3419024/show
>

[sosa, 2010-09-25 21:40:38]

Another qq, why was this TBR'd?

On Sat, Sep 25, 2010 at 5:33 PM, Chris Sosa <sosa@chromium.org> wrote:
> Can't we just add functionality to repo to add functionality that
> combines both sync and trace?  I would really prefer to not slow this
> down.  I'd honestly rather see it hang every now and have a smaller
> timeout.
>
> On Sat, Sep 25, 2010 at 2:28 PM,  <davidjames@chromium.org> wrote:
>> On 2010/09/25 17:34:31, anush wrote:
>>>
>>> LGTM. So is this only temporary? i.e once we get our new git infra in
>>> place
>>> can we switch back to regular "repo sync". Also hopefully it doesn't add
>>> too
>>> much to the build log clutter.
>>
>> If 'repo' is taking 30 minutes to sync, and there's no output, it's quite
>> difficult to know if it's just slow or if it's actually hung. By adding
>> --trace,
>> we get repo to actually print out what is happening, so we can see whether
>> it's
>> hung or not.
>>
>> Even with the new infrastructure, it's still possible that git might hang.
>> So I
>> think --trace is generally a good idea.
>>
>> If you're concerned with log spam, it might be a good idea to work with a
>> less
>> verbose version of --trace that will still allow us to detect hangs but
>> wouldn't
>> spam with as much output.
>>
>>
>> http://codereview.chromium.org/3419024/show
>>
>

[sosa, 2010-09-25 21:47:02]

Nvm about the TBR comment.  I apparently fail @ reading.

[davidjames, 2010-09-25 22:52:06]

On 2010/09/25 21:33:41, sosa wrote:
> Can't we just add functionality to repo to add functionality that
> combines both sync and trace?  I would really prefer to not slow this
> down.  I'd honestly rather see it hang every now and have a smaller
> timeout.

A bit confused here. Is repo --trace sync slower than repo sync? My
understanding is that it makes little or no difference -- repo --trace sync just
prints more output. I don't think printing more output will necessarily make a
difference in speed. 

Turning off --jobs does slow things down, but it's necessary because repo
doesn't do any error checking when you use --jobs. Without error checking, it's
quite possible you'll get the wrong source code and never know it. Of course,
this shouldn't be too hard to fix upstream, but this work hasn't been done yet.
Upstream bug is http://crosbug.com/6813

I put both the feature of adding --trace and removing --jobs in the same CL
because they both affect the same line of code.

[sosa, 2010-09-26 04:40:51]

LGTM and thanks for filing the upstream bug!

On Sat, Sep 25, 2010 at 6:52 PM,  <davidjames@chromium.org> wrote:
> On 2010/09/25 21:33:41, sosa wrote:
>>
>> Can't we just add functionality to repo to add functionality that
>> combines both sync and trace?  I would really prefer to not slow this
>> down.  I'd honestly rather see it hang every now and have a smaller
>> timeout.
>
> A bit confused here. Is repo --trace sync slower than repo sync? My
> understanding is that it makes little or no difference -- repo --trace sync
> just
> prints more output. I don't think printing more output will necessarily make
> a
> difference in speed.
>
> Turning off --jobs does slow things down, but it's necessary because repo
> doesn't do any error checking when you use --jobs. Without error checking,
> it's
> quite possible you'll get the wrong source code and never know it. Of
> course,
> this shouldn't be too hard to fix upstream, but this work hasn't been done
> yet.
> Upstream bug is http://crosbug.com/6813
>
> I put both the feature of adding --trace and removing --jobs in the same CL
> because they both affect the same line of code.
>
> http://codereview.chromium.org/3419024/show
>