1159838 - Log build_resources.json to influxdb

Status: RESOLVED FIXED

(Release Engineering :: Applications: MozharnessCore, defect)

Description

[Michael Shal [:mshal]]

mach currently logs a bunch of statistics for each tier, like runtime and cpu/io stats. We should record these to influxdb from mozharness like we do for the action info.

Comment 1

[Michael Shal [:mshal]]

Attachment: mach-influx

This posts the overall and individual tier data from build_resources.json to influxdb after the build step in mozharness completes. I'm skipping the once-per-second resource utilization data for now, since it would be a ton of data and probably not very useful for us.

Comment 2

[Rail Aliiev [:rail]]

Comment on attachment 8600448 [details] [diff] [review]
mach-influx

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

::: mozharness/base/python.py
@@ +566,5 @@
> +            self.query_abs_dirs()['abs_obj_dir'], '.mozbuild', 'build_resources.json'
> +        )
> +        if os.path.exists(self.res_props):
> +            self.info("Removing previous resource usage property file: %s" % self.res_props)
> +            self.rmtree(self.res_props)

IMHO, os.path.exists is useless because it may cause race conditions. I'd rather try/except self.rmtree. Not a show stopper though.

@@ +663,5 @@
> +            res.get('duration'),
> +            res.get('cpu_percent'),
> +        ]
> +        data.extend(res.get('io', [None] * iolen))
> +        data.extend(res.get('cpu_times', [None] * cpulen))

Can you add comments, why we need the 2 lines above?

Comment 3

[Michael Shal [:mshal]]

(In reply to Rail Aliiev [:rail] from comment #2)
> ::: mozharness/base/python.py
> @@ +566,5 @@
> > +            self.query_abs_dirs()['abs_obj_dir'], '.mozbuild', 'build_resources.json'
> > +        )
> > +        if os.path.exists(self.res_props):
> > +            self.info("Removing previous resource usage property file: %s" % self.res_props)
> > +            self.rmtree(self.res_props)
> 
> IMHO, os.path.exists is useless because it may cause race conditions. I'd
> rather try/except self.rmtree. Not a show stopper though.

Good point - I've just removed the os.path.exists & self.info calls. Though it turns out our self.rmtree does an os.path.exists internally :/

> 
> @@ +663,5 @@
> > +            res.get('duration'),
> > +            res.get('cpu_percent'),
> > +        ]
> > +        data.extend(res.get('io', [None] * iolen))
> > +        data.extend(res.get('cpu_times', [None] * cpulen))
> 
> Can you add comments, why we need the 2 lines above?

I added comments here and where the corresponding columns are extended.

Comment 4

[Michael Shal [:mshal]]

Attachment: mach-influx

Patch updated with comments, os.path.exists removed.

Comment 5

[Michael Shal [:mshal]]

Comment on attachment 8601024 [details] [diff] [review]
mach-influx

https://hg.mozilla.org/build/mozharness/rev/fa8fcd399835

Comment 6

[Pulsebot]

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

Comment 7

[Michael Shal [:mshal]]

Comment on attachment 8601024 [details] [diff] [review]
mach-influx

Backed out: https://hg.mozilla.org/build/mozharness/rev/7a6786489cf8

Some TC B2G jobs at least are complaining about abs_obj_dir:


File "/home/worker/workspace/gecko/testing/taskcluster/scripts/builder/mozharness/mozharness/base/script.py", line 1270, in run
method()
File "/home/worker/workspace/gecko/testing/taskcluster/scripts/builder/mozharness/mozharness/base/python.py", line 566, in influxdb_recording_init
self.query_abs_dirs()['abs_obj_dir'], '.mozbuild', 'build_resources.json'
KeyError: 'abs_obj_dir'

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/f4d65249ffb8

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2dd6c477f5e6
https://hg.mozilla.org/mozilla-central/rev/f4d65249ffb8

Comment 10

[Michael Shal [:mshal]]

Attachment: objdir

Followup patch to define abs_obj_dir for b2g_build.py scripts. Here's the try before/after:

before: https://treeherder.mozilla.org/#/jobs?repo=try&revision=89d0f2aacca0
after: https://treeherder.mozilla.org/#/jobs?repo=try&revision=09588e689029

Only B2G ICS Emulator fails with the error because the rest of the B2G jobs are running on TC, and bug 1161174 means they aren't using the mozharness.json rev from the try push.

Also, I know this just adds to the mess of objdir things in mozharness. so I filed bug 1161556 for that.

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a0c4366798b4

Comment 12

[Michael Shal [:mshal]]

(squashed both patches) https://hg.mozilla.org/build/mozharness/rev/f745f6eaaeea

mozharness bump - https://hg.mozilla.org/integration/mozilla-inbound/rev/a0c4366798b4

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/a0c4366798b4