1304019 - Very old tasks served for indexes when newer are existing

Status: RESOLVED FIXED

Description

[Alexandre LISSY :gerard-majax]

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

[Greg Arndt [:garndt]]

Jonas, any idea why the newer task would not have been indexed even though it completed successfully and contains the route.

Comment 2

[Jonas Finnemann Jensen (:jonasfj)]

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.

Comment 3

[Jonas Finnemann Jensen (:jonasfj)]

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.

Comment 4

[Alexandre LISSY :gerard-majax]

(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|

Comment 5

[Alexandre LISSY :gerard-majax]

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)

Comment 6

[Alexandre LISSY :gerard-majax]

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

Comment 7

[asppsa]

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.

Comment 8

[asppsa]

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

Comment 9

[Alexandre LISSY :gerard-majax]

(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

Comment 10

[Alexandre LISSY :gerard-majax]

Pushed that to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2b7bef2f3fe6&filter-tier=1&filter-tier=2&filter-tier=3

Comment 11

[asppsa]

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.

Comment 12

[Dustin J. Mitchell [:dustin] pronoun: he]

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.

Comment 13

[Dustin J. Mitchell [:dustin] pronoun: he]

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

Comment 14

[Alexandre LISSY :gerard-majax]

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

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8ccca7bb19d
Add support for setting task's index rank r=dustin

Comment 16

[Carsten Book [:Tomcat]]

backed out for d-task bustage like https://treeherder.mozilla.org/logviewer.html#?job_id=36598111&repo=mozilla-inbound

Comment 17

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/66eeb1252d8e
Backed out changeset a8ccca7bb19d for breaking Gecko Decision Task opt

Comment 18

[asppsa]

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

Here's a patch for both changes with comments

Comment 19

[asppsa]

I think my latest patch should address the tab issues as well.

Comment 20

[Alexandre LISSY :gerard-majax]

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

Comment 21

[Dustin J. Mitchell [:dustin] pronoun: he]

Comment on attachment 8795626 [details] [diff] [review]
taskcluster-rank.patch

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

perfect!

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e36f973cedf
Add support for setting task's index rank r=dustin

Comment 23

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2e36f973cedf