Very old tasks served for indexes when newer are existing

RESOLVED FIXED in mozilla52

Status

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: gerard-majax, Assigned: asppsa)

Tracking

unspecified
mozilla52

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

2 years ago
mozilla-download pulls mulet from https://index.taskcluster.net/v1/task/buildbot.branches.mozilla-central.linux64-mulet

As of now, this gets us this task: https://tools.taskcluster.net/task-inspector/#Ab13_9_mT7aPAixVZT3CZQ/
Yet, one of today is avaialble: https://tools.taskcluster.net/task-inspector/#CICnxwhYSry3bQjWw_O63Q/ (opt) or https://tools.taskcluster.net/task-inspector/#EWF0H6RFSm2cYEH8ZUR0dw/ (debug)

All those tasks do share several routes, including:
 - index.buildbot.branches.mozilla-central.linux64-mulet
 - index.gecko.v2.mozilla-central.latest.b2g.mulet-dbg or index.gecko.v2.mozilla-central.latest.b2g.mulet-opt

Using those indexes in the Indexed Artifacts Browser always gets us month old tasks: https://tools.taskcluster.net/index/artifacts/#gecko.v2.mozilla-central.latest.b2g/gecko.v2.mozilla-central.latest.b2g.mulet-opt/ yields https://tools.taskcluster.net/task-inspector/#ZnIaXRrpROSmvhOx_-3AJQ/ which is one month old

Comment 1

2 years ago
Jonas, any idea why the newer task would not have been indexed even though it completed successfully and contains the route.
Flags: needinfo?(jopsen)
please see task rank here: https://docs.taskcluster.net/reference/core/index/api-docs

Newer task only overwrite old tasks if they have a higher or equal rank.

As evident here the old task has a high rank:
  https://index.taskcluster.net/v1/task/buildbot.branches.mozilla-central.linux64-mulet

Rank defaults to zero, but can be specified in task.extra.rank = ...

Probably it was set to pushLogId at some point to ensure that we always had the latest push that completed, regardless of what order pushes completed in.
Flags: needinfo?(jopsen)
Note: I suspect anything outside of "gecko.v2" is somewhat declared deprecated, but I could be wrong.
Maybe ask mshal, certainly discuss implications with him before deciding to add task.extra.rank.
(Reporter)

Comment 4

2 years ago
(In reply to Jonas Finnemann Jensen (:jonasfj) from comment #2)
> please see task rank here:
> https://docs.taskcluster.net/reference/core/index/api-docs
> 
> Newer task only overwrite old tasks if they have a higher or equal rank.
> 
> As evident here the old task has a high rank:
>  
> https://index.taskcluster.net/v1/task/buildbot.branches.mozilla-central.
> linux64-mulet

Thanks! That current rank specifically really looks like a timestamp: 1470696420 (Mon, 08 Aug 2016 22:47:00 GMT)

> 
> Rank defaults to zero, but can be specified in task.extra.rank = ...
> 
> Probably it was set to pushLogId at some point to ensure that we always had
> the latest push that completed, regardless of what order pushes completed in.

I have had a look into the tree, but I could not find anything related to |pushLogId|
(Reporter)

Comment 5

2 years ago
Just found this:
> taskcluster/taskgraph/task/legacy.py:            'rank': params['pushdate'],

That might explain things, but I don't know how this is handled for non legacy tasks (which we are now)
(Reporter)

Comment 6

2 years ago
Dustin just explained me a solution:
> add a |rank| option in https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/transforms/task.py?q=taskcluster%2Ftaskgraph%2Ftransforms%2Ftask.py&redirect_type=direct#87
> "rank: by-tier" is the default
> "rank: 0" forces to 0
> "rank: pushdate" would do what we need

Around https://dxr.mozilla.org/mozilla-central/source/taskcluster/taskgraph/transforms/task.py?q=taskcluster%2Ftaskgraph%2Ftransforms%2Ftask.py&redirect_type=direct#436, we need to change logic a little to take task['index']['rank'] into account

Testing:
> ./mach taskgraph tasks
or
> https://queue.taskcluster.net/v1/task/XXX
with XXX the task id, and check the rank value
(Assignee)

Comment 7

2 years ago
Created attachment 8794601 [details] [diff] [review]
Add support for setting task's index rank

Allows task['index'] to have a 'rank' field which can be one of three values:

- 'by-tier' (default) specifies that the task's rank should be determined according to whether it is in tier 1 or not (the current behaviour);
- 'pushdate' specified that the task's rank should be equal to the int value of the pushdate (similar to what by-tier does with tier 1, but for any tier)
- any int value: specifies a hard-coded rank.

I've tested this by running ./mach taskgraph tasks --json -p parameters.yml with the parameters.yml file from https://tools.taskcluster.net/task-inspector/#J-3whfsORVyKvz9AMdsfPQ/0.
Attachment #8794601 - Flags: review?
Attachment #8794601 - Flags: feedback?(lissyx+mozillians)
(Assignee)

Comment 8

2 years ago
I forgot to add that in testing, I edited taskcluster/ci/b2g-device/kind.yml and added the following:

--- a/taskcluster/ci/b2g-device/kind.yml
+++ b/taskcluster/ci/b2g-device/kind.yml
@@ -36,7 +36,8 @@ job-defaults:
       - mozilla-central
       - integration
       - pine
-
+    index:
+        rank: pushdate

Also tested with integers and by-tier
(Reporter)

Comment 9

2 years ago
(In reply to asppsa from comment #8)
> I forgot to add that in testing, I edited taskcluster/ci/b2g-device/kind.yml
> and added the following:
> 
> --- a/taskcluster/ci/b2g-device/kind.yml
> +++ b/taskcluster/ci/b2g-device/kind.yml
> @@ -36,7 +36,8 @@ job-defaults:
>        - mozilla-central
>        - integration
>        - pine
> -
> +    index:
> +        rank: pushdate
> 
> Also tested with integers and by-tier

Add that to your patch and as :dustin in review
Flags: needinfo?(asppsa)
(Reporter)

Updated

2 years ago
Attachment #8794601 - Flags: review?(dustin)
Attachment #8794601 - Flags: review?
Attachment #8794601 - Flags: feedback?(lissyx+mozillians)
(Assignee)

Comment 11

2 years ago
Created attachment 8794770 [details] [diff] [review]
Use pushdate ranking for Mulet and B2G device builds

This patch uses the pushdate ranking option from the previous patch to give B2G device and Mulet builds timestamp rankings.  I've kept this separate from the other patch out of laziness, but I can make it into a single one if that's preferred.
Flags: needinfo?(asppsa)
Attachment #8794770 - Flags: review?(dustin)
Comment on attachment 8794601 [details] [diff] [review]
Add support for setting task's index rank

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

(conditional r+ on adding comments)

::: taskcluster/taskgraph/transforms/task.py
@@ +104,5 @@
>                  Required('gecko-v2'): basestring,
>              }
>          ),
> +
> +        'rank': Any('by-tier', int, 'pushdate')

Please add some comments here as to what these options mean.
Attachment #8794601 - Flags: review?(dustin) → review+
Comment on attachment 8794770 [details] [diff] [review]
Use pushdate ranking for Mulet and B2G device builds

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

::: taskcluster/ci/build/mulet.yml
@@ +6,4 @@
>              buildbot: linux64-mulet
>              gecko-v1: mulet.dbg
>              gecko-v2: mulet-dbg
> +        rank: pushdate

This and its pair might deserve a comment too:
  # despite being tier-3, these jobs should be inserted into the
  # index ranked by pushdate
or something like that
Attachment #8794770 - Flags: review?(dustin) → review+
(Reporter)

Comment 14

2 years ago
Created attachment 8795609 [details] [diff] [review]
Add support for setting task's index rank

Folding all into one patch and addressing Dustin's review comment
Attachment #8794601 - Attachment is obsolete: true
Attachment #8794770 - Attachment is obsolete: true
Attachment #8795609 - Flags: review+

Comment 15

2 years ago
Pushed by alissy@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8ccca7bb19d
Add support for setting task's index rank r=dustin
(Reporter)

Updated

2 years ago
Assignee: nobody → asppsa
backed out for d-task bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=36598111&repo=mozilla-inbound
Flags: needinfo?(asppsa)

Comment 17

2 years ago
Backout by cbook@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/66eeb1252d8e
Backed out changeset a8ccca7bb19d for breaking Gecko Decision Task opt
(Assignee)

Comment 18

2 years ago
Created attachment 8795626 [details] [diff] [review]
taskcluster-rank.patch

Here's a patch for both changes with comments
Flags: needinfo?(asppsa)
Attachment #8795626 - Flags: review?(dustin)
(Assignee)

Comment 19

2 years ago
I think my latest patch should address the tab issues as well.
(Reporter)

Updated

2 years ago
Attachment #8795609 - Attachment is obsolete: true
Comment on attachment 8795626 [details] [diff] [review]
taskcluster-rank.patch

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

perfect!
Attachment #8795626 - Flags: review?(dustin) → review+

Comment 22

2 years ago
Pushed by alissy@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e36f973cedf
Add support for setting task's index rank r=dustin

Comment 23

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/2e36f973cedf
Status: NEW → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.