Slow API endpoint /api/releases

NEW

Status

Release Engineering
Balrog: Backend
P1
normal
2 months ago
a day ago

People

(Reporter: nthomas, Assigned: Collins Abitekaniza)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Reporter)

Description

2 months ago
When I load https://aus4-admin.mozilla.org/releases there's a delay while it looks up the list of releases via https://aus4-admin.mozilla.org/api/releases - about 12 seconds talking to prod, and 4 or so seconds talking to local docker. It may be the continual addition of releases, but it would be good to make sure we're not doing something expensive.

The data returned looks like
{
  "releases": [
    {
      "data_version": 1, 
      "name": "Asyncrendering-2.0", 
      "product": "SystemAddons", 
      "read_only": false, 
      "required_signoffs": {}, 
      "rule_ids": []
    }, 
...
so it could be the signoff and mapping lookup, or overall load on the DB.
This is because of the signoff look-ups - I remember noticing the change when that landed, but I didn't think it was a big enough problem to block on.
(Reporter)

Comment 2

2 months ago
It's not the end of the world, just a bit of a wait. Perhaps we're checking for scheduled changes for each of the 1545 releases, and could use one query and some code instead.
Priority: -- → P3
I see this mentioned almost daily as annoyance now, we should probably do something about it. The quick fix might be to disable signoff look-ups on /releases, which would probably be a worthwhile trade-off.
Priority: P3 → P1
(Reporter)

Comment 4

10 days ago
Here's a request with some light instrumentation:

balrogadmin_1  | 2018-01-08 01:38:08,855 - WARNING - PID: 419 - Request: 139996151769488 - ReleasesAPIView.get#365: Getting release list
balrogadmin_1  | 2018-01-08 01:38:08,912 - WARNING - PID: 419 - Request: 139996151769488 - ReleasesAPIView.get#367: Got list, getting signoff requirements
balrogadmin_1  | 2018-01-08 01:38:16,153 - WARNING - PID: 419 - Request: 139996151769488 - ReleasesAPIView.get#372: Got signoffs, starting serializing
balrogadmin_1  | 2018-01-08 01:38:16,157 - WARNING - PID: 419 - Request: 139996151769488 - auslib.web.common.releases.serialize_releases#48: Done serialising, just need to jsonify now
balrogadmin_1  | [pid: 419|app: 0|req: 6/6] 172.18.0.5 () {54 vars in 1001 bytes} [Mon Jan  8 01:38:08 2018] GET /api/releases => generated 4348 bytes in 7404 msecs (HTTP/1.1 200) 8 headers in 291 bytes (1 switches on core 0)

so definitely a problem with looking for pending signoffs. If I disable those lookups then page load time (for the 256 releases in a local dev db) drops from 7s to ~200ms. Production has 1618 releases and page load is ~20s.

The request is handled by https://github.com/mozilla/balrog/blob/master/auslib/web/admin/views/releases.py#L367, and getPotentialRequiredSignoffs() is at https://github.com/mozilla/balrog/blob/master/auslib/db.py#L1880. It's not set up to use any caching. I wondered if it would be possible to use releases.isMappedTo() to short-circuit DB lookups, but it does uncached DB lookups too. 

So lets go back a few steps and flip this around - is there any reason we can't we look for incomplete scheduled changes for releases, and only then check for pending signoffs for that subset of releases ? With our current usage the subset will be 0 or very small, rather than scale linearly as we add more releases.

'name' is primary key in releases, and IIRC used for matching rows between releases and releases_scheduled_changes, so can the name be modified in a change ? Seems like that would be bad.
(In reply to Nick Thomas [:nthomas] (UTC+13) from comment #4)
> Here's a request with some light instrumentation:
> 
> balrogadmin_1  | 2018-01-08 01:38:08,855 - WARNING - PID: 419 - Request:
> 139996151769488 - ReleasesAPIView.get#365: Getting release list
> balrogadmin_1  | 2018-01-08 01:38:08,912 - WARNING - PID: 419 - Request:
> 139996151769488 - ReleasesAPIView.get#367: Got list, getting signoff
> requirements
> balrogadmin_1  | 2018-01-08 01:38:16,153 - WARNING - PID: 419 - Request:
> 139996151769488 - ReleasesAPIView.get#372: Got signoffs, starting serializing
> balrogadmin_1  | 2018-01-08 01:38:16,157 - WARNING - PID: 419 - Request:
> 139996151769488 - auslib.web.common.releases.serialize_releases#48: Done
> serialising, just need to jsonify now
> balrogadmin_1  | [pid: 419|app: 0|req: 6/6] 172.18.0.5 () {54 vars in 1001
> bytes} [Mon Jan  8 01:38:08 2018] GET /api/releases => generated 4348 bytes
> in 7404 msecs (HTTP/1.1 200) 8 headers in 291 bytes (1 switches on core 0)
> 
> so definitely a problem with looking for pending signoffs. If I disable
> those lookups then page load time (for the 256 releases in a local dev db)
> drops from 7s to ~200ms. Production has 1618 releases and page load is ~20s.
> 
> The request is handled by
> https://github.com/mozilla/balrog/blob/master/auslib/web/admin/views/
> releases.py#L367, and getPotentialRequiredSignoffs() is at
> https://github.com/mozilla/balrog/blob/master/auslib/db.py#L1880. It's not
> set up to use any caching. I wondered if it would be possible to use
> releases.isMappedTo() to short-circuit DB lookups, but it does uncached DB
> lookups too. 
> 
> So lets go back a few steps and flip this around - is there any reason we
> can't we look for incomplete scheduled changes for releases, and only then
> check for pending signoffs for that subset of releases ? With our current
> usage the subset will be 0 or very small, rather than scale linearly as we
> add more releases.

This is a good idea.

> 'name' is primary key in releases, and IIRC used for matching rows between
> releases and releases_scheduled_changes, so can the name be modified in a
> change ? Seems like that would be bad.

You can't change PK columns in a Scheduled Change, so we don't need to worry about this case.
Assignee: nobody → collins.abitekaniza
You need to log in before you can comment on or make changes to this bug.