Optimize Bugzilla->active_custom_fields() for CPU and memory usage

RESOLVED FIXED

Status

()

P1
major
RESOLVED FIXED
2 months ago
18 days ago

People

(Reporter: jdm, Assigned: dylan)

Tracking

({leave-open})

Production
leave-open

Details

Attachments

(5 attachments)

(Reporter)

Description

2 months ago
When performing the following query, I am receiving consistent 502 Bad Gateway responses.

https://bugzilla.mozilla.org/rest/bug?username=josh%40joshmatthews.net&status=RESOLVED&apikey=[secret]&target_milestone=Firefox+45&target_milestone=mozilla45&include_fields=id%2Cassigned_to

This query worked fine six weeks ago; it's one of many that I run when preparing the reports of new contributors to Firefox releases.
(Reporter)

Comment 1

a month ago

More specifically, I receive the following behaviour consistently when running my script that fetches data for releases 10 -> 65:
10->16: 200 OK
17: 502 Bad Gateway

This is preventing me from creating the reports that link to from the Firefox release notes.

(Assignee)

Updated

a month ago
Assignee: nobody → dylan
(Assignee)

Comment 3

a month ago

Something about the bad queries is using a huge amount of memory.
Worse, the allocations are coming from the mysqlclient C code.

I'm working out a fix.

(Assignee)

Comment 4

a month ago

I have found where the memory grows -- it's not in the mysqlclient after all, but is related to custom fields.

(Assignee)

Comment 5

a month ago

Each call to Bugzilla->active_custom_fields() uses between 1600 kb to 1800 kb of memory per call. This memory is not leaked -- it is returned when the request cache is cleaned up, but it grows to the number of bugs returned in the request.

Further investigation shows it isn't active_custom_fields() itself, but rather the hook that adds tracking flags.

Immediate-ish remediation for this would be bumping the max memory limit up a bit -- 3G instead of 2G may be sufficient.
Quick remediation would passing skip_extensions to Bugzilla->active_custom_fields() when include_fields doesn't include any tracking flags.

The most correction remediation would be making Tracking Flags extension not cache so aggressively.

Severity: normal → major
Component: API → Extensions: TrackingFlags
Priority: -- → P1
(Assignee)

Comment 6

a month ago

josh:

Do you have an immediate need for those APIs to work? If that is the case I can direct ops to drastically increase the memory limits until I can get a code fix out.

Flags: needinfo?(josh)
(Assignee)

Comment 7

a month ago

dkl: Any idea how we could leverage filter_wants() to tell if the user actually wants tracking flags, without first loading the list of tracking flags? Perhaps a regex?

Flags: needinfo?(dkl)
(Reporter)

Comment 8

a month ago

I will need the APIs to work in two weeks, but I can wait until then.

Flags: needinfo?(josh)
(Assignee)

Comment 9

a month ago

Ok, it will definitely work by then. I hope it works by monday.

(Assignee)

Updated

a month ago
Summary: Rest API queries are yielding 502 Bad Gateway → Tracking flags active_custom_fields() hook allocates 1700kb per call leading to OOM errors for some APIs
(Assignee)

Comment 10

a month ago

Created attachment 9037361 [details]
debug.pl

script I used for collecting information.

(Assignee)

Comment 11

a month ago

A few runs of the attached script:

  DB<7> p main::cache_size()
filter_wants = 68
Bugzilla::Component,9 = 545
Bugzilla::User,302720 = 665
Bugzilla::Bug,_,_ = 2003
  DB<8> c
Bugzilla::Extension::TrackingFlags::active_custom_fields(/app/extensions/TrackingFlags/Extension.pm:291):
291:      my ($self, $args) = @_;
  DB<8> p main::cache_size()
Bugzilla::Component,149 = 662
Bugzilla::User,269762 = 665
Bugzilla::Bug,_,_ = 2120

it seems like this might be the problem

(Assignee)

Comment 12

a month ago

Here's some more debugging info, counting the number and size of objects in memory over time.
https://gist.github.com/dylanwh/ae8c37963cc146643e24ac0eb95c10e9

Need to think on this some more.

(Assignee)

Comment 13

a month ago

Well, there's the problem. We have 37,053 Bugzilla::Extension::TrackingFlags::Flag::Value objects allocated after the a few requests, and there are only 3,861 in the database.

(Assignee)

Comment 14

29 days ago

Created attachment 9037648 [details] [review]
Skip preloading tracking flags

(Assignee)

Comment 15

29 days ago

Created attachment 9037704 [details] [review]
Store the list of all tracking flags in memcached.

(Reporter)

Comment 16

26 days ago

Is there a chance this will be fixed this week? The next release is on the 28th.

Flags: needinfo?(dylan)
(Assignee)

Comment 17

26 days ago

Absolutely, the two patches fix everything

Flags: needinfo?(dylan)
(Assignee)

Comment 18

25 days ago

Created attachment 9038289 [details] [review]
Add object that encapsulates the include/exclude logic

(Assignee)

Updated

25 days ago
Attachment #9037704 - Attachment description: Ignore tracking flags if they're excluded from the results anyway → Store the list of all tracking flags in memcached.
(Assignee)

Updated

25 days ago
Summary: Tracking flags active_custom_fields() hook allocates 1700kb per call leading to OOM errors for some APIs → Optimize Bugzilla->active_custom_fields() for CPU and memory usage
(Assignee)

Updated

25 days ago
Keywords: leave-open
(Assignee)

Updated

25 days ago
Version: Staging → Production
Flags: needinfo?(dkl)
(Assignee)

Comment 19

19 days ago

Created attachment 9039667 [details] [review]
Make Bugzilla->active_custom_wants() 10x faster when using specific include_fields

(Assignee)

Updated

18 days ago
Status: NEW → RESOLVED
Last Resolved: 18 days ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.