Log build_resources.json to influxdb

RESOLVED FIXED

Status

Release Engineering
Mozharness
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: mshal, Assigned: mshal)

Tracking

unspecified

Firefox Tracking Flags

(firefox40 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

3 years ago
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.
(Assignee)

Updated

3 years ago
Assignee: nobody → mshal
(Assignee)

Comment 1

3 years ago
Created attachment 8600448 [details] [diff] [review]
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.
Attachment #8600448 - Flags: review?(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?
Attachment #8600448 - Flags: review?(rail) → review+
(Assignee)

Comment 3

3 years ago
(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.
(Assignee)

Comment 4

3 years ago
Created attachment 8601024 [details] [diff] [review]
mach-influx

Patch updated with comments, os.path.exists removed.
Attachment #8600448 - Attachment is obsolete: true
Attachment #8601024 - Flags: review+
(Assignee)

Comment 5

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

https://hg.mozilla.org/build/mozharness/rev/fa8fcd399835
Attachment #8601024 - Flags: checked-in+

Comment 6

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/2dd6c477f5e6
(Assignee)

Comment 7

3 years ago
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'
Attachment #8601024 - Flags: checked-in+

Comment 8

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/f4d65249ffb8
https://hg.mozilla.org/mozilla-central/rev/2dd6c477f5e6
https://hg.mozilla.org/mozilla-central/rev/f4d65249ffb8
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
(Assignee)

Updated

3 years ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 10

3 years ago
Created attachment 8601498 [details] [diff] [review]
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.
Attachment #8601498 - Flags: review?(rail)
Attachment #8601498 - Flags: review?(rail) → review+

Comment 11

3 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/a0c4366798b4
(Assignee)

Comment 12

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

mozharness bump - https://hg.mozilla.org/integration/mozilla-inbound/rev/a0c4366798b4
https://hg.mozilla.org/mozilla-central/rev/a0c4366798b4
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.