1162522 - perfherder autogenerated summary series are redundant for tests which have only 1 page

Status: RESOLVED FIXED

(Tree Management :: Perfherder, defect)

Description

[Joel Maher ( :jmaher ) (UTC -8)]

the summary is great to roll up results from a series of subtests.  After using compare mode, I see summary results for ts_paint and when viewing the details I load the page and get the same data as there is only 1 data point.

We could reduce our signatures and time to investigate a bit by not generating summaries for tests that have a single data point.

This would have issues in:
* test selection
* alert generation
* compare view move (we have code that does different things based on series.subtestSignatures having data)

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

current tests that I know of are:
ts_paint
sessionrestore
sessionrestore_no_auto_restore
tresize
tpaint
trobocheck

Comment 2

[Avi Halachmi (:avih)]

>  perfherder autogenerated summary series are redundant for tests which have only 1 page

This is not always true. Depends on the algorithm which summarizes the subtest[s] value[s] into the suite summary.

If we have a single subtest and the suite summary algorithm is geomean or average (or sum, etc), then agreed that the suite summary would have the same value as the (single) subtest value.

But for other suite summary algorithms it might not be true.

Treeherder doesn't know and shouldn't care what the suite summary algorithm is, it should just use whatever summary value talos provides (and the appropriate field at that, in case there are several such talos summaries), and it also shouldn't care whether the test has only one subtest or several subtests.

If the talos summary is incorrect for some reason (which I don't think is the case, but it could happen), then it should be fixed at the talos summary side, and not at treeherder.

Comment 3

[William Lachance (:wlach)]

Attachment: PR

Now that bug 1210503 is landed, we can put this in. Most of the changes are pretty simple: the bulk of the commit is a migration script to remove old signatures with one subtest.

Comment 4

[William Lachance (:wlach)]

(In reply to Avi Halachmi (:avih) from comment #2)
> >  perfherder autogenerated summary series are redundant for tests which have only 1 page
> 
> This is not always true. Depends on the algorithm which summarizes the
> subtest[s] value[s] into the suite summary.
> 
> If we have a single subtest and the suite summary algorithm is geomean or
> average (or sum, etc), then agreed that the suite summary would have the
> same value as the (single) subtest value.
> 
> But for other suite summary algorithms it might not be true.
> 
> Treeherder doesn't know and shouldn't care what the suite summary algorithm
> is, it should just use whatever summary value talos provides (and the
> appropriate field at that, in case there are several such talos summaries),
> and it also shouldn't care whether the test has only one subtest or several
> subtests.
> 
> If the talos summary is incorrect for some reason (which I don't think is
> the case, but it could happen), then it should be fixed at the talos summary
> side, and not at treeherder.

This is theoretically true, but in practice a suite summary == the value of the test for all cases where there is only one subtest. I don't see any point in storing redundant data in treeherder for these cases, it only causes confusion and unnecessarily bloats the size of the signature list / stored data.

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8675897 [details] [review]
PR

oh nice, I like the script to convert the database.  here will will also need to fix compare view to not have a 'subtest' link.

Comment 6

[William Lachance (:wlach)]

(In reply to Joel Maher (:jmaher) from comment #5)
> Comment on attachment 8675897 [details] [review]
> PR
> 
> oh nice, I like the script to convert the database.  here will will also
> need to fix compare view to not have a 'subtest' link.

I believe that is already taken care with MikeLing's changes in bug 1210503.

Comment 7

[Treeherder GitHub Bugbot]

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

https://github.com/mozilla/treeherder/commit/06fbcd9b09f06f680af1d9a64190fc1e085c1cb9
Bug 1162522 - Don't summarize talos tests with only one subtest

Comment 8

[Treeherder GitHub Bugbot]

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

https://github.com/mozilla/treeherder/commit/c6e4151bce6dc5d73430976fcff7737bdbe3ef6c
Bug 1162522 - Fix minor problem with migration script

Comment 9

[Joel Maher ( :jmaher ) (UTC -8)]

it appears these are removed now, can we close this?

Comment 10

[William Lachance (:wlach)]

Yep, let's close this!