Android perf tests: kill adbd on the device after running a test.

Android perf tests: kill adbd on the device after running a test.

There's some data indicating that adbd on the device sometimes
spontaneously restores its connection.
After running a test, let's kill adbd and wait for bit.

This is a speculative change to try to make the bots healthier.

BUG=

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=226762

[bulach, 2013-10-01 18:44:43]

this pushes armand's idea of just waiting at the end of the test to something a
bit more brutal :)

I'm not sure if this would help or make things worse, but I think it's worth
trying and collecting data across the bots, then revert if needed.. ptal!

[craigdh, 2013-10-01 18:56:44]

https://codereview.chromium.org/25574002/diff/1/build/android/pylib/perf/test...
File build/android/pylib/perf/test_runner.py (right):

https://codereview.chromium.org/25574002/diff/1/build/android/pylib/perf/test...
build/android/pylib/perf/test_runner.py:165: adb =
android_commands.AndroidCommands(self.device)
An adb object should already exists for this device. Normally that is reused
instead of creating a new one.

https://codereview.chromium.org/25574002/diff/1/build/android/pylib/perf/test...
build/android/pylib/perf/test_runner.py:168: adb.RunShellCommandWithSU('kill %s'
% ' '.join(adb_pids))
Instead of putting all this here, let's add
android_commands.AndroidCommands.KillAdbd()

[navabi, 2013-10-01 22:55:10]

I like trying this out. Though, I should note that while my tests show that
sleeping between tests means that the tests start out with the device available,
it is still the case that most of the time (i.e. ~75%) the device still goes
offline while running tests.

I am trying to figure out exactly where that happens, but in the meantime, I
think this is a good change. Even giving each step its own 25% chance to
complete would be better than what we have now.

https://codereview.chromium.org/25574002/diff/1/build/android/pylib/perf/test...
File build/android/pylib/perf/test_runner.py (right):

https://codereview.chromium.org/25574002/diff/1/build/android/pylib/perf/test...
build/android/pylib/perf/test_runner.py:165: adb =
android_commands.AndroidCommands(self.device)
On 2013/10/01 18:56:44, craigdh wrote:
> An adb object should already exists for this device. Normally that is reused
> instead of creating a new one.

I don't understand. Where is the reference to that adb object? We do not have a
reference to an adb object here. Are you suggesting it be passed in to
TestRunner?

https://codereview.chromium.org/25574002/diff/1/build/android/pylib/perf/test...
build/android/pylib/perf/test_runner.py:168: adb.RunShellCommandWithSU('kill %s'
% ' '.join(adb_pids))
On 2013/10/01 18:56:44, craigdh wrote:
> Instead of putting all this here, let's add
> android_commands.AndroidCommands.KillAdbd()

+1

[bulach, 2013-10-02 08:53:43]

thanks both! comments addressed, ptal..

https://codereview.chromium.org/25574002/diff/1/build/android/pylib/perf/test...
File build/android/pylib/perf/test_runner.py (right):

https://codereview.chromium.org/25574002/diff/1/build/android/pylib/perf/test...
build/android/pylib/perf/test_runner.py:165: adb =
android_commands.AndroidCommands(self.device)
On 2013/10/01 22:55:10, navabi wrote:
> On 2013/10/01 18:56:44, craigdh wrote:
> > An adb object should already exists for this device. Normally that is reused
> > instead of creating a new one.
> 
> I don't understand. Where is the reference to that adb object? We do not have
a
> reference to an adb object here. Are you suggesting it be passed in to
> TestRunner?

I think base_test_runner.BaseTestRunner has already an adb member available,
using that instead.

https://codereview.chromium.org/25574002/diff/1/build/android/pylib/perf/test...
build/android/pylib/perf/test_runner.py:168: adb.RunShellCommandWithSU('kill %s'
% ' '.join(adb_pids))
On 2013/10/01 22:55:10, navabi wrote:
> On 2013/10/01 18:56:44, craigdh wrote:
> > Instead of putting all this here, let's add
> > android_commands.AndroidCommands.KillAdbd()
> 
> +1

Done.

[navabi, 2013-10-02 17:28:10]

On 2013/10/02 08:53:43, bulach wrote:
> thanks both! comments addressed, ptal..
> 
>
https://codereview.chromium.org/25574002/diff/1/build/android/pylib/perf/test...
> File build/android/pylib/perf/test_runner.py (right):
> 
>
https://codereview.chromium.org/25574002/diff/1/build/android/pylib/perf/test...
> build/android/pylib/perf/test_runner.py:165: adb =
> android_commands.AndroidCommands(self.device)
> On 2013/10/01 22:55:10, navabi wrote:
> > On 2013/10/01 18:56:44, craigdh wrote:
> > > An adb object should already exists for this device. Normally that is
reused
> > > instead of creating a new one.
> > 
> > I don't understand. Where is the reference to that adb object? We do not
have
> a
> > reference to an adb object here. Are you suggesting it be passed in to
> > TestRunner?
> 
> I think base_test_runner.BaseTestRunner has already an adb member available,
> using that instead.
> 
>
https://codereview.chromium.org/25574002/diff/1/build/android/pylib/perf/test...
> build/android/pylib/perf/test_runner.py:168: adb.RunShellCommandWithSU('kill
%s'
> % ' '.join(adb_pids))
> On 2013/10/01 22:55:10, navabi wrote:
> > On 2013/10/01 18:56:44, craigdh wrote:
> > > Instead of putting all this here, let's add
> > > android_commands.AndroidCommands.KillAdbd()
> > 
> > +1
> 
> Done.

lgtm

[craigdh, 2013-10-02 17:36:53]

It will be interesting to see if this helps... lgtm w/ comment.

https://codereview.chromium.org/25574002/diff/6001/build/android/pylib/androi...
File build/android/pylib/android_commands.py (right):

https://codereview.chromium.org/25574002/diff/6001/build/android/pylib/androi...
build/android/pylib/android_commands.py:470: except Exception as e:
Let's not eat the exception here. It's fine if you want to catch and ignore it
in the perf test but in general I think we should allow exceptions to bubble up
from android_commands to avoid silent failures.

[bulach, 2013-10-03 08:26:13]

thanks all! landing this, keep your fingers crossed :)

https://codereview.chromium.org/25574002/diff/6001/build/android/pylib/androi...
File build/android/pylib/android_commands.py (right):

https://codereview.chromium.org/25574002/diff/6001/build/android/pylib/androi...
build/android/pylib/android_commands.py:470: except Exception as e:
On 2013/10/02 17:36:53, craigdh wrote:
> Let's not eat the exception here. It's fine if you want to catch and ignore it
> in the perf test but in general I think we should allow exceptions to bubble
up
> from android_commands to avoid silent failures.

good point! agreed, moved the try..except up to the perf test..

[commit-bot: I haz the power, 2013-10-03 08:26:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bulach@chromium.org/25574002/13001

[commit-bot: I haz the power, 2013-10-03 15:24:39]

Change committed as 226762

[bulach, 2013-10-04 12:17:08]

turns out this made the bots flakier and ross reverted here:
http://src.chromium.org/viewvc/chrome?view=rev&revision=226980

it looks like it was due to the forwarder, so here's another try:
https://codereview.chromium.org/25982005/