Closed Bug 1140612 Opened 9 years ago Closed 9 years ago

buildapi reporter should use request_id properties if set

Categories

(Release Engineering :: General, defect)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: catlee, Assigned: catlee)

Details

Attachments

(1 file)

One of the frequent queries we see from buildapi is this code:
http://hg.mozilla.org/build/buildapi/file/7045b03ccdaf/buildapi/scripts/reporter.py#l58

It's trying to find the request ids in the schedulerdb for a given build in the statusdb.

Luckily we now have the request ids set as properties from postrun.py, so we should use those instead of trying to reconstruct things after the fact. This may also explain why we sometimes have differences between various buildapi/selfserve endpoints when reporting request ids for jobs.
Attachment #8574182 - Flags: review?(nthomas)
Comment on attachment 8574182 [details] [diff] [review]
request_ids-buildapi.patch

r+, assuming the properties have been properly cast when they're read back in from the statusdb into props (aka not a string).
Attachment #8574182 - Flags: review?(nthomas) → review+
Comment on attachment 8574182 [details] [diff] [review]
request_ids-buildapi.patch

Committed as https://hg.mozilla.org/build/buildapi/rev/a8fbc0f17848

Some minor changes:
- bumped version
- use min(props['request_times'].values()) instead of min(props['request_times']) to get the actual request time rather than request id (key of the dict).
Attachment #8574182 - Flags: checked-in+
deployed. looks to be working
Assignee: nobody → catlee
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Hi catlee,
Thanks for fixing this.

(In reply to Chris AtLee [:catlee] from comment #0)
> This may also explain why we sometimes have differences between various
> buildapi/selfserve endpoints when reporting request ids for jobs.

Is this comment related to that email I sent you last week?

I landed last week a fix which looks at the request_ids from properties instead of the request_ids key:
https://github.com/armenzg/mozilla_ci_tools/commit/87c32cb126f0ccf4f606c45c7461f84cff881204#diff-56ae75e3365cc1d018a3686e48e3bd8fL69

Does it look good to you? (You can ignore everything else on that patch; only the first block matters)
(In reply to Armen Zambrano - Automation & Tools Engineer (:armenzg) from comment #5)
> Hi catlee,
> Thanks for fixing this.
> 
> (In reply to Chris AtLee [:catlee] from comment #0)
> > This may also explain why we sometimes have differences between various
> > buildapi/selfserve endpoints when reporting request ids for jobs.
> 
> Is this comment related to that email I sent you last week?

Yes

> I landed last week a fix which looks at the request_ids from properties
> instead of the request_ids key:
> https://github.com/armenzg/mozilla_ci_tools/commit/
> 87c32cb126f0ccf4f606c45c7461f84cff881204#diff-
> 56ae75e3365cc1d018a3686e48e3bd8fL69
> 
> Does it look good to you? (You can ignore everything else on that patch;
> only the first block matters)

Yeah, that looks fine as well. That's basically what this reporting patch is doing.
Few questions :-)

1) Is the buildapi reporter used as part of builds-{pending,running,4hr}.js generation? ie will the commits here fix some of the inconsistencies I've seen in them (will file a bug shortly).
2) Are we sure using min() is the right thing to do? In Treeherder we found that the last element in the list (not the smallest) was the correct request_id (at least more of the time than anything else).

Context: us trying to fix bug 1093743.
Flags: needinfo?(catlee)
Just filed bug 1141603.
(In reply to Ed Morley [:edmorley] from comment #7)
> Few questions :-)
> 
> 1) Is the buildapi reporter used as part of builds-{pending,running,4hr}.js
> generation? ie will the commits here fix some of the inconsistencies I've
> seen in them (will file a bug shortly).

This particular piece is used for builds-4hr.js. pending/running are generated differently.

> 2) Are we sure using min() is the right thing to do? In Treeherder we found
> that the last element in the list (not the smallest) was the correct
> request_id (at least more of the time than anything else).

I did this to match the existing format. All the request times are already present as build properties if you dig down into the structure.
Flags: needinfo?(catlee)
Component: General Automation → General
You need to log in before you can comment on or make changes to this bug.