881897 - Reduce frequency of polling in remoteautomation.py

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Dan Minor [:dminor]]

Reducing the frequency of polling the current foreground activity and retrieving the log file in remoteautomation.py would reduce network load and might help reduce how often bug 807230 shows up in production.

Comment 1

[Dan Minor [:dminor]]

Once a fix for bug 881879 is landed, we can also try pulling only new portions of the log file instead of the whole file each time.

Comment 2

[Dan Minor [:dminor]]

Attachment: Patch to reduce frequency of polling and only retrieve latest portion of log file.

I'm looking for a bit of feedback on this approach before I subject this to more extensive testing. Seems to work fine in the limited testing I've done so far. Thanks.

Comment 3

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 763523 [details] [diff] [review]
Patch to reduce frequency of polling and only retrieve latest portion of log file.

Review of attachment 763523 [details] [diff] [review]:
-----------------------------------------------------------------

this seems small, yet major...all in the right ways.

::: build/mobile/remoteautomation.py
@@ +265,5 @@
>                  the new log entries since the last call (as a multi-line string).
>              """
>              if self.dm.fileExists(self.proc):
>                  try:
> +                    newLogContent = self.dm.pullFile(self.proc, self.stdoutlen)

I assume self.stdoutlen is the len of what we read last time, and that the api for pullFile will use that as a starting point and read until the end?

@@ +298,5 @@
> +                # retrieve log updates every 60 seconds
> +                if timer % 60 == 0: 
> +                    t = self.stdout
> +                    if t != '':
> +                        print(t)

my concern with this is we might only print out stdout from the most recent pull.  how do we mark the last bit of stdout that we displayed so we can print it all out.  I suspect this will work just fine.

also we usually don't do print(..).  I think it is a style thing and maybe I didn't get the memo that we are changing our style.

Comment 4

[Dan Minor [:dminor]]

I have a try run for the patch here: https://tbpl.mozilla.org/?tree=Try&rev=1451f337a213

Comment 5

[Dan Minor [:dminor]]

Attachment: Patch to reduce polling frequency and only retrieve latest portion of log file.

If I properly understand the concern with only printing stdout from the last pull, this patch does not change how this worked before. As long as no one else accesses self.stdout, there will be no gaps in the output. This isn't great, but it is how things worked before.

Comment 6

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 768404 [details] [diff] [review]
Patch to reduce polling frequency and only retrieve latest portion of log file.

Review of attachment 768404 [details] [diff] [review]:
-----------------------------------------------------------------

this is great stuff!

Comment 7

[Dan Minor [:dminor]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2b2fea402b9c

Comment 8

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/2b2fea402b9c