731391 - collect 30 row major data points per page, then drop off the first 10

Status: RESOLVED FIXED

(Testing :: Talos, defect)

Description

[Jeff Hammel]

Currently filters don't take an argument except the data array: 

http://hg.mozilla.org/build/talos/file/099171bc6da5/talos/filter.py

We want something like:

def ignore_first(data, number=10):
    if len(data) <= number:
        return data # or raise NotImplementedError
    return data[number:]

I'm not sure how we want to do this in terms of going forward and e.g. https://bugzilla.mozilla.org/show_bug.cgi?id=728442

Comment 1

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

could we make filters like the preferences:
filters:
  ignoreFirst: 10
  median: true

The problem there is we cannot guarantee these filters will end up in order.  If we could assert that this could be solved as mentioned above.

Otherwise, we might need to get more complex.  I would rather keep stuff in the .config file instead of changing values in the code itself.  

I am thinking we could have a list of filters in the config file:

filters: [ignoreFirst, median]

and in the test section we can define custom values:

baseTest:
  ignoreFirst: 1

tp5:
  ignoreFirst: 10


Then in the code, we can look for filter parameters in the test config block.  This could get confusing but it would work.

Comment 2

[Jeff Hammel]

(In reply to Joel Maher (:jmaher) from comment #1)
> could we make filters like the preferences:
> filters:
>   ignoreFirst: 10
>   median: true
> 
> The problem there is we cannot guarantee these filters will end up in order.
> If we could assert that this could be solved as mentioned above.
> 
> Otherwise, we might need to get more complex.  I would rather keep stuff in
> the .config file instead of changing values in the code itself.  
> 
> I am thinking we could have a list of filters in the config file:
> 
> filters: [ignoreFirst, median]
> 
> and in the test section we can define custom values:
> 
> baseTest:
>   ignoreFirst: 1
> 
> tp5:
>   ignoreFirst: 10
> 
> 
> Then in the code, we can look for filter parameters in the test config
> block.  This could get confusing but it would work.

So I'm not 100% sure what we can get away with wrt yml configuration.  In JSON I would have something like

{'filters': [['ignoreFirst', [1]], ['median']]...

or

'filters': [['ignoreFirst', [1]], ['median', []]]

where you basically have a list of 2 item lists where the first item is the filter (function) name and the second item is a list of arguments to the function. In the first case, you use a convenience that functions with no arguments are single-item lists (less explicit, but maybe easier to understand?)

In YAML, I guess we do something like

filters:
- [ignoreFirst, [10]]
- [median]

or

filters:
- ignoreFirst:10
- median
- complexFunction:10,8,13.5

If the latter is legal (which I'm not sure it is) it may require additional parsing

From the command line, this would look like

--filter ignoreFirst:10 --filter medain --filter complexFunction:10,8,13.5

Note a few things:

- you're going to have to parse the numeric values
- this is easy if you only have ints and floats.  If you don't it will be hard
- we will want something we really want here since, at least by the currently ticketed roadmap, graphserver will make use of these too (and once we settle on something, it will not be trivial to change)

Comment 3

[Jeff Hammel]

It is probably also worth adding a filter test as part of this bug

Comment 4

[Jeff Hammel]

so YAML does allow:

# data filters
filters:
 - [foo, [10]]

So maybe we just do that

Comment 5

[Jeff Hammel]

...with --filter foo:10 on the command line (or --filter foo:10,5,7.2 for multiple values)

Comment 6

[Jeff Hammel]

Attachment: allow arbitrary multiple arguments for filters

Comment 7

[Jeff Hammel]

We will also need to be able to configure filters on a per-test basis

Comment 8

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

Comment on attachment 603393 [details] [diff] [review]
allow arbitrary multiple arguments for filters

Review of attachment 603393 [details] [diff] [review]:
-----------------------------------------------------------------

this is great, thanks!

Comment 9

[Jeff Hammel]

pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=f7b4c2002282

Comment 10

[Jeff Hammel]

Somewhat tested on android;  however, I ran into bug 733583 while testing.  I have seen this before, its certainly not new with this patch, but I've never been able to get as good of diagnostics before.

Comment 11

[Mozilla RelEng Bot]

Try run for f7b4c2002282 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=f7b4c2002282
Results (out of 65 total builds):
    success: 61
    failure: 4
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/[e-mail]-f7b4c2002282
 Timed out after 06 hours without completing.

Comment 12

[Jeff Hammel]

There were massive infrastructure failures yesterday so retrying the affected jobs.  FWIW, it doesn't look like any failures from this patch

Comment 13

[Jeff Hammel]

So this is looking green; pushing

Comment 14

[Jeff Hammel]

pushed: http://hg.mozilla.org/build/talos/rev/571ee259dc96

Comment 15

[Jeff Hammel]

There is still the follow up work to do tracked in bug 733514 . leaving open until this is done

Comment 16

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

I think we should close this bug and just track in bug 733514.

Comment 17

[Jeff Hammel]

works for me