Closed Bug 1159838 Opened 9 years ago Closed 9 years ago

Log build_resources.json to influxdb

Categories

(Release Engineering :: Applications: MozharnessCore, defect)

defect
Not set
normal

Tracking

(firefox40 fixed)

RESOLVED FIXED
Tracking Status
firefox40 --- fixed

People

(Reporter: mshal, Assigned: mshal)

References

Details

Attachments

(2 files, 1 obsolete file)

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: nobody → mshal
Attached patch mach-influx (obsolete) — Splinter Review
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+
(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.
Attached patch mach-influxSplinter Review
Patch updated with comments, os.path.exists removed.
Attachment #8600448 - Attachment is obsolete: true
Attachment #8601024 - Flags: review+
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+
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached patch objdirSplinter Review
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+
https://hg.mozilla.org/mozilla-central/rev/a0c4366798b4
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.