1189519 - Should be able to filter performance series summary by platform

Status: RESOLVED FIXED

(Tree Management :: Perfherder, defect)

Description

[William Lachance (:wlach)]

We currently load data for *all* platforms in the test chooser in the graphs view. This takes a long time, as the full set of signatures is something like 1MB.

If the API endpoint allowed filtering by platform, we could just download the signatures needed to display the tests for that particular branch/platform combination, which should be much faster (especially if we cache such combinations on the server side of things)

To get started, let's just modify the servers side endpoint: we can do client side changes after we have the capability. Here's an example query:

https://treeherder.mozilla.org/api/project/mozilla-inbound/performance-data/get_performance_series_summary/?interval=86400

We want to be able to filter the query as follows:

https://treeherder.mozilla.org/api/project/mozilla-inbound/performance-data/get_performance_series_summary/?interval=86400&machine_platform=windows7-32

In that case it should only return signatures which have the key value pair: machine_platform==windows7-32.

We should just need to modify two things to make this possible:

* Make the API endpoint aware of the parameter: https://github.com/mozilla/treeherder/blob/master/treeherder/webapp/api/performance_data.py#L42
* Modify the get_performance_series_summary internal method to optionally take the parameter to filter the set of results that it returns https://github.com/mozilla/treeherder/blob/master/treeherder/model/derived/jobs.py#L374

To test this, you'll need to setup a copy of treeherder and ingest some talos data using the procedure described here (ingest a single revision from mozilla-inbound from sometime in the last 3-4 hours for best results):

http://treeherder.readthedocs.org/installation.html#ingesting-a-single-push-at-a-time

Note the comment at the end about setting up a celery work to process async tasks.

Once you've got that set up with some perf data ingested, you should be able to test your changes by going to the API URL directly. For example, to test filtering for the windows7-32 platform, try:

http://local.treeherder.mozilla.org/api/project/mozilla-inbound/performance-data/get_performance_series_summary/?interval=86400&machine_platform=windows7-32

Comment 1

[MikeLing]

Sure Will, I'm interesting in this bug and would like to try my best to fix it :)

Due to I still got some bug on my hand and not been merged, I would like to put it on my bug list and start to work on it after. 

Thank you ;)

Comment 2

[MikeLing]

Attachment: Screenshot.png

Hi Will, I meet some problem when I config the ingesting for a single push.

I want to see if I can get the some result like https://treeherder.mozilla.org/api/project/mozilla-inbound/performance-data/get_performance_series_summary/?interval=86400 . So I run my vagrant environment and use http://local.treeherder.mozilla.org/api/project/mozilla-inbound/performance-data/get_performance_series_summary/?interval=86400 to query the json data. You can see it from my attachment.

I don't know which step I do is wrong. Maybe we could talk on IRC about it, thank you very much! :)

Comment 3

[MikeLing]

Attachment: warningMessage.text

hmm, the problem I meet seems like not about revision number I use and I don't know how to describe it. I past the WARNING message blow. and the commend I use to ingest push is "./manage.py ingest_push mozilla-inbound b080520b4a15"

You can see a lot "WARNING" message in there tells me "skipping ......" and you can also see "Imported 0 running jobs data" at last. 
:wlach could you give me some advice about it?

Comment 4

[MikeLing]

(In reply to MikeLing from comment #3)

Oh, forget to mention in last comment. I can get json use " ./manage.py import_perf_data --time-interval 86400 --filter-props suite:tp5o mozilla-inbound ", just curious about why those warning message will shows up and I can't import data.

Comment 5

[MikeLing]

Attachment: PR for bug 1189519

Comment 6

[William Lachance (:wlach)]

Comment on attachment 8643507 [details]
warningMessage.text

As discussed in the meeting, this is considered normal.

Comment 7

[William Lachance (:wlach)]

Comment on attachment 8643664 [details] [review]
PR for bug 1189519

This mostly looks good! I'd like to see one change outlined in the PR. Did you test it?

Comment 8

[MikeLing]

Comment on attachment 8643664 [details] [review]
PR for bug 1189519

ask review again, with a screenshot of test result ;)

Comment 9

[William Lachance (:wlach)]

Comment on attachment 8643664 [details] [review]
PR for bug 1189519

Ran into one small problem in my testing, which should be relatively easy to fix.

I'll give some instructions on where to go next after this review. :)

Comment 10

[William Lachance (:wlach)]

So, the next step is to add an endpoint to get a list of platforms that are valid for this set of performance signatures (we need that to be able to populate the list in the test chooser, since we'll no longer be downloading everything at once). I'd recommend having an endpoint here like:

/api/project/mozilla-inbound/performance-data/platforms/

This should return a simple list of all the possible values for the machine_platform key. i.e.:

[ "linux32", "linux64", "windows7-32", ... ]

You can follow the example of the existing endpoint that you modified to add a new one: basically add the api method in webapp/api/performance_data.py, then something to process the list of signatures in `jobs.py`.
Not sure if that's enough info to get you started, let me know if you need more guidance. :)

After this is done, we can update the client side code to take advantage of your new web api methods.

Comment 11

[MikeLing]

(In reply to William Lachance (:wlach) from comment #10)
> So, the next step is to add an endpoint to get a list of platforms that are
> valid for this set of performance signatures (we need that to be able to
> populate the list in the test chooser, since we'll no longer be downloading
> everything at once). I'd recommend having an endpoint here like:
> 
> /api/project/mozilla-inbound/performance-data/platforms/

Hm, which kind of proc file I could use to query the platforms list? Maybe I should custom a proc file named "get_platforms" and put it into here? (https://github.com/mozilla/treeherder/blob/21ca58c3d2b0c6823529e1401802efced345bf8d/treeherder/model/sql/jobs.json)

Comment 12

[William Lachance (:wlach)]

(In reply to MikeLing from comment #11)
> (In reply to William Lachance (:wlach) from comment #10)
> > So, the next step is to add an endpoint to get a list of platforms that are
> > valid for this set of performance signatures (we need that to be able to
> > populate the list in the test chooser, since we'll no longer be downloading
> > everything at once). I'd recommend having an endpoint here like:
> > 
> > /api/project/mozilla-inbound/performance-data/platforms/
> 
> Hm, which kind of proc file I could use to query the platforms list? Maybe I
> should custom a proc file named "get_platforms" and put it into here?
> (https://github.com/mozilla/treeherder/blob/
> 21ca58c3d2b0c6823529e1401802efced345bf8d/treeherder/model/sql/jobs.json)

I think we can just the existing `jobs.selects.get_perf_series_properties` proc (from the method you just modified) and search for the particular keys we're interested in. I guess you'll need to pass in an interval seconds parameter here, to know which ones are valid. When I change the way we use the database, I'm going to be removing all that code: may as well make my job a little less difficult.

Comment 13

[MikeLing]

(In reply to William Lachance (:wlach) from comment #12)
> (In reply to MikeLing from comment #11)

It tells me 

> "detail": "'JobsModel' object has no attribute 'get_plaforms'"

But I'm sure that I had add a get_platforms function in job.py, should I register it in somewhere else?

Comment 14

[MikeLing]

you can see it in here(haven't finish it yet) 

https://github.com/mozilla/treeherder/commit/b98fd0520bd27e9f478380c6d39541a59af00f3c#diff-a03f6e7c1747a7ef236e211422cc7c5eR443

Comment 15

[MikeLing]

Attachment: PR for add get platforms endpoint

Comment 16

[William Lachance (:wlach)]

Comment on attachment 8643664 [details] [review]
PR for bug 1189519

Just one issue with variable names.

Comment 17

[William Lachance (:wlach)]

Comment on attachment 8645288 [details] [review]
PR for add get platforms endpoint

Looks good, just a few adjustments needed (see PR)

Comment 18

[Treeherder GitHub Bugbot]

Commits pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/a2ebf02bd4563e31638bb73d4efb47bed419b2e8
Bug 1189519 - Filter performance series summary by platform

https://github.com/mozilla/treeherder/commit/9deb03620418e82df7cd8ed0c3789e6428b0125e
Merge pull request #833 from MikeLing/bugfix-1189519

Bug 1189519 - Filter performance series summary by platform

Comment 19

[William Lachance (:wlach)]

Comment on attachment 8643664 [details] [review]
PR for bug 1189519

Looks great! Merged.

Comment 20

[William Lachance (:wlach)]

Comment on attachment 8645288 [details] [review]
PR for add get platforms endpoint

I think there was some minor confusion on what exactly needed to be changed here. Still almost there though!

Comment 21

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/3418d2e5ccf1f2d82334dc276e9233c2c11a77d0
Merge pull request #848 from MikeLing/add_get_platform_endpoint

Bug 1189519 - Add get platforms endpoint

Comment 22

[William Lachance (:wlach)]

Comment on attachment 8645288 [details] [review]
PR for add get platforms endpoint

Looks good now, thanks!

Comment 23

[William Lachance (:wlach)]

I closed this too early, we still need to add the UI logic to filter the list of signatures by platform. Basically loading signatures should now be a two step process inside the UI when the performance modal is first loaded:

(1) Get the list of platforms associated with the branch.
(2) For the platform selected, filter the list of signatures we allow to add by the selected platform.

Let me know if you're still interested in working on this :mikeling-- I think it should actually be less complicated than the work you most recently did for bug 1134780.

Comment 24

[MikeLing]

(In reply to William Lachance (:wlach) from comment #23)

> Let me know if you're still interested in working on this :mikeling-- I
> think it should actually be less complicated than the work you most recently
> did for bug 1134780.

Sure! I'd happy to keep working on it untill it's perfect to closed.

I think the (1) has been achieved in last PR which is get platform list. So the next step I should do is filter a list of signatures associated with platform we select, Isn't it? Please correct me if I was wrong :)

Comment 25

[MikeLing]

Attachment: patch for bug 1189519

Hmm, I wrote some code for it but can't test locally. Actually, I meet some problem, when I try to ingest data on locally like few weeks ago. You can see the error blow.

>vagrant ~/treeherder $ ./manage.py import_perf_data --time-interval 86400 >--filter-props suite:tp5o mozilla-inbound
>
>Exception in thread Thread-1:
>
>Traceback (most recent call last):
>
>  File "/usr/lib/python2.7/threading.py", line 810, in __bootstrap_inner
>
>    self.run()
>
>  File "/usr/lib/python2.7/threading.py", line 763, in run
>
>    self.__target(*self.__args, **self.__kwargs)
>
>  File "/home/vagrant/venv/local/lib/python2.7/site-packages/concurrent/futures/process.py", line 208, in _queue_management_worker
>
>    result_item = result_queue.get(block=True)
>
>  File "/usr/lib/python2.7/multiprocessing/queues.py", line 117, in get
>
>    res = self._recv()
>
>TypeError: ('__init__() takes at least 2 arguments (1 given)', <class 'treeherder.model.derived.base.DatasetNotFoundError'>, ())

And the other method on treeherder document doesn't work, too.

>vagrant ~/treeherder $ ./manage.py ingest_push mozilla-inbound 507a508aea70
>
>MySQL.MySQL debug message:
>
>host:localhost db:treeherder host_type:read_host >proc:reference.selects.get_all_repository_info
>
>Executing SQL:SELECT * FROM `repository` WHERE `active_status` = 'active'
>
>Execution Time:2.0289e-04 sec
>
>CommandError: No project found named 'mozilla-inbound'

The weirdest part is I can run treeherder locally without problem and can see moziila-inbound data on treeherder UI, but it just tell me no project named 'mozilla-inbound' and other project.

Comment 26

[MikeLing]

Oh! I'm very sorry about the format of last comment, I should double check before I commit it. 

If there are anything I haven't explain clearly or hard to know from last comment, please tell me on IRC or email me. I wrote some notes for that, so I can reproduce the problem immediately if you want :) 

Thank you!

Comment 27

[MikeLing]

Comment on attachment 8651339 [details]
patch for bug 1189519

https://github.com/MikeLing/treeherder/commit/6b64f8fe4defd272440371cb9c1d09cc95beda94

Comment 28

[MikeLing]

Attachment: PR for get signature list filted by platform name

Hope this PR can fit your mind :)

Comment 29

[William Lachance (:wlach)]

Comment on attachment 8652788 [details] [review]
PR for get signature list filted by platform name

This actually isn't necessary. :) You already implemented the server-side filtering by platform by adding a parameter to an existing method:

https://github.com/mozilla/treeherder/blob/master/treeherder/webapp/api/performance_data.py#L40

The only remaining pieces for this bug are the UI portions I described just above.

Comment 30

[MikeLing]

(In reply to William Lachance (:wlach) from comment #29)
> Comment on attachment 8652788 [details] [review]
> PR for get signature list filted by platform name
> 
> This actually isn't necessary. :) You already implemented the server-side
> filtering by platform by adding a parameter to an existing method:
> 
> https://github.com/mozilla/treeherder/blob/master/treeherder/webapp/api/
> performance_data.py#L40
> 
> The only remaining pieces for this bug are the UI portions I described just
> above.

Oh, sorry about that. I misinterpret this issue from begining. Please close this pr. sorry for this mistake
: (

Comment 31

[MikeLing]

Comment on attachment 8652788 [details] [review]
PR for get signature list filted by platform name

Please tell me if doesn't fit your mind, then I will ask review if it's good to go :)

Comment 32

[MikeLing]

Comment on attachment 8652788 [details] [review]
PR for get signature list filted by platform name

Think it's good to go now :)

Comment 33

[William Lachance (:wlach)]

Comment on attachment 8652788 [details] [review]
PR for get signature list filted by platform name

There are a few problems, but this is definitely on the right track.

Comment 34

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/f2c918ec9cff59254b4b32b7ccce77fce05674ef
Bug 1189519 - Add UI logic to filter series data by platform

Comment 35

[William Lachance (:wlach)]

Comment on attachment 8652788 [details] [review]
PR for get signature list filted by platform name

Works great now, thanks!