Reduce frequency of polling in remoteautomation.py

RESOLVED FIXED in mozilla25

Status

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: dminor, Unassigned)

Tracking

Trunk
mozilla25
ARM
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

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.
Blocks: 807230
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.
Depends on: 881879
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.
Attachment #763523 - Flags: feedback?(jmaher)
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.
Attachment #763523 - Flags: feedback?(jmaher) → feedback+
I have a try run for the patch here: https://tbpl.mozilla.org/?tree=Try&rev=1451f337a213
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.
Attachment #763523 - Attachment is obsolete: true
Attachment #768404 - Flags: review?(jmaher)
Attachment #768404 - Flags: feedback?(gbrown)
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!
Attachment #768404 - Flags: review?(jmaher) → review+
Attachment #768404 - Flags: feedback?(gbrown) → feedback+
https://hg.mozilla.org/mozilla-central/rev/2b2fea402b9c
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.