Closed Bug 1378451 Opened 6 years ago Closed 6 years ago

Taskcluster/buildbot configs for new talos perf-reftest-singletons suite

Categories

(Firefox Build System :: Task Configuration, task)

Product:
Firefox Build System
Component:
Task Configuration
Type:
task
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED
Milestone:
mozilla56

People

(Reporter: rwood, Assigned: rwood)

References

Details

(Whiteboard: [PI:July])

Attachments

(3 files, 3 obsolete files)

bug1378451-take3.patch
3.96 KB, patch
catlee
: review+
Details | Diff | Splinter Review
fixconfigs.patch
1.25 KB, patch
catlee
: review+
Details | Diff | Splinter Review
run the 'ps' job on all builds
2.82 KB, patch
rwood
: review+
Details | Diff | Splinter Review 
Bug 1378139 lands a new talos 'perf-reftest-singletons' test suite. We need to add taskcluster and buildbot configs so this new suite will run (on stylo only).
could we just add this in talos.json for the 'p' job?
Hmm, problem is we only want this to run on stylo, that's why I was thinking a separate treeherder job group was needed
* job symbol (not group)(In reply to Robert Wood [:rwood] from comment #2)
> Hmm, problem is we only want this to run on stylo, that's why I was thinking
> a separate treeherder job group was needed

* job symbol (not group)
yeah, then we will need talos.json changes now; then builbot changes (and the reconfig), then finally taskcluster changes.  could we also detect the build/config type and inside of talos skip the test if it is != stylo ?
Whiteboard: [PI:July]
(In reply to Joel Maher ( :jmaher) (afk until july 12) from comment #4)
> yeah, then we will need talos.json changes now; then builbot changes (and
> the reconfig), then finally taskcluster changes.  could we also detect the
> build/config type and inside of talos skip the test if it is != stylo ?

That would cool if possible, and have a test flag 'requires_stylo=True' or something. Then there wouldn't be any config changes required at all, like you say just add it as an additional test in the 'perf-reftest' suite / 'p' job.
Update: because of SETA I prefer to keep this job totally separate.
Attached patch bug1378451-bb.patch (obsolete) — — Splinter Review 

        Attachment #8884307 -
        Flags: review?(catlee)

    
  
Depends on: 1379164

    
    

    
  
Comment on attachment 8884307 [details] [diff] [review]
bug1378451-bb.patch

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

::: mozilla-tests/config.py
@@ +2750,5 @@
>      branch['perf-reftest_tests'] = (0, False, {}, ALL_TALOS_PLATFORMS)
>  
> +# Enable talos perf-reftest-singeltons on 56+
> +for name, branch in items_at_least(BRANCHES, 'gecko_version', 56):
> +    if branch.get('enable_talos') is False:

this should be re-written as

    if not branch.get('enable_talos'):

If enable_talos is not set for a branch, branch.get('enable_talos') will return None, and your conditional will fail.

        Attachment #8884307 -
        Flags: review?(catlee) → review-

    
    

    
  
Comment on attachment 8884297 [details]
Bug 1378451 - Taskcluster configs for new perf-reftest-singleton talos suite for stylo;

https://reviewboard.mozilla.org/r/155240/#review161176

        Attachment #8884297 -
        Flags: review?(wcosta) → review+

    
    

    
  

      
      
      
      

        Attached patch
          
          
        bug1378451-take2.patch (obsolete)
        — 
          — Splinter Review
      

    


  
Thanks Chris!

        Attachment #8884307 -
        Attachment is obsolete: true

        Attachment #8885266 -
        Flags: review?(catlee)

    
    

    
  


  
Third time's a charm! :) Thanks Chris

        Attachment #8885266 -
        Attachment is obsolete: true

        Attachment #8885266 -
        Flags: review?(catlee)

        Attachment #8885283 -
        Flags: review?(catlee)

        Attachment #8885283 -
        Flags: review?(catlee) → review+

    
    

    
  
Pushed by rwood@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/feaa8a920f06
Taskcluster configs for new perf-reftest-singleton talos suite for stylo; r=wcosta

    
    

    
  
Thanks :kwierso.

From IRC (thanks :wcosta):
wcosta> rwood: I think you have to add "default: 'built-projects' in run-on-projects: by-test-platform
Flags: needinfo?(rwood)

    
    

    
      
  

    
    

    
      
  

    
    

    
  
Comment on attachment 8884297 [details]
Bug 1378451 - Taskcluster configs for new perf-reftest-singleton talos suite for stylo;

That fixes it, thanks :wcosta. I also had to correct the suite name as this one is on e10s only. Set r? for the latest update in mozreview (here it is on try):

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f6fb94ccf1f264c19514b7e6a81063bc440afe02

Shouldn't the new job (talos 'ps') be showing up here on the try run?

        Attachment #8884297 -
        Flags: review+ → review?(wcosta)

    
    

    
      
  

    
    

    
      
  

    
    

    
  
Comment on attachment 8884297 [details]
Bug 1378451 - Taskcluster configs for new perf-reftest-singleton talos suite for stylo;

I can't seem to get the new 'ps' job to appear in T-e10s for stylo, latest try run:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8168e643f828ab6dc32dae43817efd1dc529d734

        Attachment #8884297 -
        Flags: review?(wcosta)

    
    

    
  
(In reply to Robert Wood [:rwood] from comment #22)
> Comment on attachment 8884297 [details]
> Bug 1378451 - Taskcluster configs for new perf-reftest-singleton talos suite
> for stylo;
> 
> I can't seem to get the new 'ps' job to appear in T-e10s for stylo, latest
> try run:
> 
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=8168e643f828ab6dc32dae43817efd1dc529d734

It runs on buildbot-bridge, so, must be something to be done on buildbot side. 303 :catlee
Flags: needinfo?(catlee)

    
    

    
  
Your patch needs to be merged to the production branch and deployed. Ping me / buildduty tomorrow and we can get that done for you.
Flags: needinfo?(catlee)

    
    

    
  
Comment on attachment 8884297 [details]
Bug 1378451 - Taskcluster configs for new perf-reftest-singleton talos suite for stylo;

https://reviewboard.mozilla.org/r/155240/#review161934

        Attachment #8884297 -
        Flags: review?(wcosta) → review+

    
    

    
  
(In reply to Chris AtLee [:catlee] from comment #24)
> Your patch needs to be merged to the production branch and deployed. Ping me
> / buildduty tomorrow and we can get that done for you.

Ok thank guys

    
    

    
  

      
      
      
      

        Attached patch
          
          
        fixconfigs.patch
        — 
          — Splinter Review
      

    


  
Updating patch to use previous syntax, as per:

https://bugzilla.mozilla.org/show_bug.cgi?id=1380721#c6

        Attachment #8886577 -
        Flags: review?(catlee)

    
    

    
  
Comment on attachment 8886577 [details] [diff] [review]
fixconfigs.patch

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

sorry for misleading you earlier :(

I'm still a bit confused, but this seems safer given the previous landing.

        Attachment #8886577 -
        Flags: review?(catlee) → review+

    
    

    
  
(In reply to Chris AtLee [:catlee] from comment #29)
> Comment on attachment 8886577 [details] [diff] [review]
> fixconfigs.patch
> 
> Review of attachment 8886577 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> sorry for misleading you earlier :(
> 
> I'm still a bit confused, but this seems safer given the previous landing.

Np, I thought your original suggestion made sense also. I find the configs really confusing. Thanks for the review! Landed:

https://hg.mozilla.org/build/buildbot-configs/rev/18fbf8b7493bcbfb40c75cfb2514629ba22097ce

    
    

    
      
  

    
    

    
  
Alright this is still not working. Two issues here:

1) The decision task is generating taskgraphs [see A] for the new talos 'perf-reftest-singeltons-e10s' suite, but for regular linux as well as stylo. Supposed to only be stylo but it is creating:

"test-linux64-stylo/opt-talos-perf-reftest-singletons-e10s"
"test-linux64-stylo-sequential/opt-talos-perf-reftest-singletons-e10s"
"test-linux64/opt-talos-perf-reftest-singletons-e10s"

[A] https://public-artifacts.taskcluster.net/F4Am_vm0TRuTW4TtXo5_IQ/0/public/target-tasks.json

:wcosta, any idea why it is generating a task for "test-linux64/opt-talos-perf-reftest-singletons-e10s" when it should only be stylo builds only?

.....

2) The test still isn't running, or if it is running, it's not showing up on treeherder. There should be a 'ps' symbol added along with all the other talos jobs [see B]


[B] https://treeherder.mozilla.org/#/jobs?repo=try&revision=6189d2d7ca43d74ebc55f3fddcc5ea86a914be04

I looked over the taskgraphs created and they look to be the same as the graphs created for 'perf-reftest' so I don't believe this is a taskcluster issue. I think this is a buildbot issue -or- a treeherder etl data issue when mapping jobs to symbols. I'll revisit that code next.
Flags: needinfo?(wcosta)

    
    

    
  
(In reply to Robert Wood [:rwood] from comment #33)
> Alright this is still not working. Two issues here:
> 
> 1) The decision task is generating taskgraphs [see A] for the new talos
> 'perf-reftest-singeltons-e10s' suite, but for regular linux as well as
> stylo. Supposed to only be stylo but it is creating:
> 
> "test-linux64-stylo/opt-talos-perf-reftest-singletons-e10s"
> "test-linux64-stylo-sequential/opt-talos-perf-reftest-singletons-e10s"
> "test-linux64/opt-talos-perf-reftest-singletons-e10s"
> 
> [A]
> https://public-artifacts.taskcluster.net/F4Am_vm0TRuTW4TtXo5_IQ/0/public/
> target-tasks.json
> 
> :wcosta, any idea why it is generating a task for
> "test-linux64/opt-talos-perf-reftest-singletons-e10s" when it should only be
> stylo builds only?
> 

style and linux64 share the same talos test set [1]. I suggest creating a new test set which contains stylo tests that don't run on linux, and add it to linux64-stylo.

> .....
> 
> 2) The test still isn't running, or if it is running, it's not showing up on
> treeherder. There should be a 'ps' symbol added along with all the other
> talos jobs [see B]
> 
> 
> [B]
> https://treeherder.mozilla.org/#/
> jobs?repo=try&revision=6189d2d7ca43d74ebc55f3fddcc5ea86a914be04
> 
> I looked over the taskgraphs created and they look to be the same as the
> graphs created for 'perf-reftest' so I don't believe this is a taskcluster
> issue. I think this is a buildbot issue -or- a treeherder etl data issue
> when mapping jobs to symbols. I'll revisit that code next.

Yeah, I believe this is a config in buildbot side. 303 :kmoir

[1] https://dxr.mozilla.org/mozilla-central/source/taskcluster/ci/test/test-platforms.yml#51,97
Flags: needinfo?(wcosta) → needinfo?(kmoir)

    
    

    
  
If you are adding jobs on taskcluster that run on hardware via buildbot bridge, the associated builders need to be added on the buildbot side so that when they are triggered via taskcluster, there are builders for them to run on buildbot.  See bug 1338871 for an example.
Flags: needinfo?(kmoir)

    
    

    
  
:bholley, do you want the perf-reftest-singleton tests to only run on stylo? Or is it fine to run on stylo and also linux64? If we're going to be turning off stylo builds down the road maybe we should add the perf-reftest-singleton test to linux 64 now also.
Flags: needinfo?(bobbyholley)

    
    

    
  
(In reply to Kim Moir [:kmoir] ET from comment #35)
> If you are adding jobs on taskcluster that run on hardware via buildbot
> bridge, the associated builders need to be added on the buildbot side so
> that when they are triggered via taskcluster, there are builders for them to
> run on buildbot.  See bug 1338871 for an example.

Thanks :kmoir. Are the buildbot configs that landed in comment 27 and comment 30 not sufficient?
Flags: needinfo?(kmoir)

    
    

    
  
(In reply to Robert Wood [:rwood] from comment #36)
> :bholley, do you want the perf-reftest-singleton tests to only run on stylo?
> Or is it fine to run on stylo and also linux64? If we're going to be turning
> off stylo builds down the road maybe we should add the
> perf-reftest-singleton test to linux 64 now also.

We do intend to stop special _builds_ for Stylo soon (bug 1374748), though for now there will still be a separate Stylo test platform, so that we continue gathering test and talos results from both Gecko and Stylo modes.

So I think running it also for linux64 sounds okay, since I believe we'll want that eventually anyway.

    
    

    
  
The names of the builders are on the buildbot side

https://tools.taskcluster.net/groups/fSE21Jy9RleEQivr1VjtEw

It looks like they aren't being scheduled on the tc side
https://tools.taskcluster.net/groups/fSE21Jy9RleEQivr1VjtEw

I would recommend looking at m-c (stylo talos just run on m-c iirc) and generating a target graph to see if they are included
Flags: needinfo?(kmoir)

    
    

    
  
(In reply to Robert Wood [:rwood] from comment #36)
> :bholley, do you want the perf-reftest-singleton tests to only run on stylo?
> Or is it fine to run on stylo and also linux64? If we're going to be turning
> off stylo builds down the road maybe we should add the
> perf-reftest-singleton test to linux 64 now also.

Yes, that sounds like the right thing to me. Running it on all platforms would be ideal really, since it's cheap. Is this harness compatible with mac/windows?
Flags: needinfo?(bobbyholley)

    
    

    
  
(In reply to Bobby Holley (:bholley) (busy with Stylo) from comment #40)
> 
> Yes, that sounds like the right thing to me. Running it on all platforms
> would be ideal really, since it's cheap. Is this harness compatible with
> mac/windows?

I agree, and yes re: mac/windows. I notice that the 'ps' job is showing up on Windows, hopefully with :jmaher's patch it will appear on the other platforms.

    
    

    
  
Comment on attachment 8884297 [details]
Bug 1378451 - Taskcluster configs for new perf-reftest-singleton talos suite for stylo;

I totally thought I landed this, hah. :jmaher has a replacement patch below

        Attachment #8884297 -
        Attachment is obsolete: true

        Attachment #8884297 -
        Flags: review+

    
    

    
  
Comment on attachment 8888486 [details] [diff] [review]
run the 'ps' job on all builds

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

One issue I believe

::: taskcluster/ci/test/tests.yml
@@ +1453,5 @@
> +    description: "Talos perf-reftest singletons"
> +    suite: talos
> +    try-name: perf-reftest-singletons
> +    treeherder-symbol: tc-T(ps)
> +    run-on-projects: ['mozilla-central', 'try']

I think you need to include by-test-platform, and include a default. I didn't have that and it broke the decision task. Does it work on try as/is? If not, you probably need to add something like this:

http://searchfox.org/mozilla-central/rev/3a3af33f513071ea829debdfbc628caebcdf6996/taskcluster/ci/test/tests.yml#1252

        Attachment #8888486 -
        Flags: review?(rwood) → review-

    
    

    
  
Pushed by jmaher@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/80ba5ccded4c
Taskcluster configs for new perf-reftest-singleton talos suite for stylo; r=wcosta

    
    

    
  
Comment on attachment 8888486 [details] [diff] [review]
run the 'ps' job on all builds

R+ with your 'virtualization: hardware' fix

        Attachment #8888486 -
        Flags: review- → review+

    
    

    
  
https://hg.mozilla.org/mozilla-central/rev/80ba5ccded4c
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56

    
  
Product: TaskCluster → Firefox Build System

          You need to log in
          before you can comment on or make changes to this bug.