1405570 - Download of the clang tidy artifact is failing

Status: RESOLVED FIXED

(Firefox Build System :: Task Configuration, task)

Description

[Sylvestre Ledru [:Sylvestre]]

$ ./mach static-analysis install
 0:01.75 Could not find artifacts for a toolchain build named `linux64-clang-tidy`

Comment 1

[Mike Hommey [:glandium]]

The url that it tries to download is correct, but ends in a 404.

So, if I take a look at a random decision task on central (the current tip, for instance), here is what I can see in the logs:
https://public-artifacts.taskcluster.net/Z_rg348JT8qtcGwuOTQtWQ/0/public/logs/live_backing.log

[task 2017-10-03T21:39:01.151910Z] Generating tasks for toolchain linux64-clang-tidy
(...)
[task 2017-10-03T21:40:03.415808Z] "GET /index/v1/task/gecko.cache.level-3.toolchains.v1.linux64-clang-tidy.74ca61387bd2c45cf3e85c31818c049723d7977ff7d05ab07e66395f79ce0c3e HTTP/1.1" 404 454

So with this, we'd expect a job to be created for that task, right? Nope, it's not in https://queue.taskcluster.net/v1/task/Z_rg348JT8qtcGwuOTQtWQ/runs/0/artifacts/public/task-graph.json

I suspect this is fallout from bug 1383880. Since those jobs have nothing depending on them, they're probably optimized out for good.

In the meanwhile, I've triggered the jobs manually, once they're finished building, it should work. https://queue.taskcluster.net/v1/task/f8hGhf9MQT2NNJ6fiQFVGg/runs/0/artifacts/public/to-run.json

But any change that makes that index url above to change will break it again.

Comment 2

[Jan Keromnes [:janx]]

Note: I did some bisecting, and https://github.com/mozilla/gecko-dev/commit/938bafc956f41385a32d8c81340d6696477da9ac is the first commit that broke `./mach artifact toolchain --from-build linux64-clang-tidy`.

Comment 3

[Mike Hommey [:glandium]]

(In reply to Jan Keromnes [:janx] from comment #2)
> Note: I did some bisecting, and
> https://github.com/mozilla/gecko-dev/commit/
> 938bafc956f41385a32d8c81340d6696477da9ac is the first commit that broke
> `./mach artifact toolchain --from-build linux64-clang-tidy`.

Kind of yes, kind of no. It just happens that that commit changed the hash in the artifact url by touching the toolchain.py file. And since it's only one commit in a push of several, that particular commit hasn't been built individually. Had it been, it wouldn't have been broken. The problem is that some other commit on top of that one made the tasks for the clang-tidy task never happen.

The one that actually broke stuff is probably 400f6f88ad15586926b464d32f7fc13be78d97f9, but that one actually breaks the mach artifact toolchain code in another way, that isn't fixed until fc50c56e23509208fa92727d664dcc5ab20855ed (both those commits being in the same push)

Comment 4

[Mike Hommey [:glandium]]

(In reply to Mike Hommey [:glandium] from comment #1)
> Nope, it's not in
> https://queue.taskcluster.net/v1/task/Z_rg348JT8qtcGwuOTQtWQ/runs/0/
> artifacts/public/task-graph.json

But it *is* in https://queue.taskcluster.net/v1/task/Z_rg348JT8qtcGwuOTQtWQ/runs/0/artifacts/public/target-tasks.json
so it's stripped out somewhere in between.

Comment 5

[Sylvestre Ledru [:Sylvestre]]

This is now fixed. Thanks.

Comment 6

[Mike Hommey [:glandium]]

Attachment: Bug 1405570 - Never remove tasks with an index-search optimization.

Review commit: https://reviewboard.mozilla.org/r/186586/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/186586/

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Happening again:
 1:53.92 Could not find artifacts for a toolchain build named `linux64-clang-tidy`

Comment 8

[Mike Hommey [:glandium]]

WFM on central. Ensure you don't have spurious files in build/clang-plugin (like a .pyc file).

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Actually, it is because I have local pending changes for bug 1406310 (thanks Andi)
This is pretty fragile and cryptic. is there a way to avoid this behavior?

Comment 10

[Sylvestre Ledru [:Sylvestre]]

Indeed, without the local patch, it is working. 
I reported bug 1406346 to improve that.

Comment 11

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8915397 [details]
Bug 1405570 - Never remove tasks with an index-search optimization.

https://reviewboard.mozilla.org/r/186586/#review192792

::: taskcluster/taskgraph/optimize.py:302
(Diff revision 1)
>      def should_remove_task(self, task, params, index_paths):
> -        "If this task has no dependencies, don't run it.."
> -        return True
> +        # If the task has artifacts existing for the given index_paths, it will
> +        # be replaced below. Otherwise, this means the artifacts have expired
> +        # and we need to create new ones, or some changes altered the
> +        # index_paths and we need new artifacts. In both cases the task needs
> +        # to run.

This will apply to all IndexSearch tasks, and will mean that they always run regardless of whether they are required, on all trees.  For example, adding a new directory to `taskcluster/docker` will cause that docker image to be built, even if it is unused.

And I think that's what you want.  I don't quite understand *why* you want it, though: why should a toolchain that has not yet been used in automation be available for use on the command line?

One final note: the logic in this comment isn't quite complete: `should_replace_task` is only called for tasks with no un-optimized dependencies.  In this event that's what you want, but please update the comment accordingly.

Comment 12

[Mike Hommey [:glandium]]

(In reply to Dustin J. Mitchell [:dustin] from comment #11)
> And I think that's what you want.  I don't quite understand *why* you want
> it, though: why should a toolchain that has not yet been used in automation
> be available for use on the command line?

We have toolchains where the only consumers are end users, not automation.

Comment 13

[Mike Hommey [:glandium]]

Comment on attachment 8915397 [details]
Bug 1405570 - Never remove tasks with an index-search optimization.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/186586/diff/1-2/

Comment 14

[Dustin J. Mitchell [:dustin] (he/him)]

(In reply to Mike Hommey [:glandium] from comment #12)
> We have toolchains where the only consumers are end users, not automation.

Ah, OK.  Those should be included in the target task set, then.

Comment 15

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8915397 [details]
Bug 1405570 - Never remove tasks with an index-search optimization.

https://reviewboard.mozilla.org/r/186586/#review193696

Comment 16

[Jonas Finnemann Jensen (:jonasfj)]

Comment on attachment 8915397 [details]
Bug 1405570 - Never remove tasks with an index-search optimization.

https://reviewboard.mozilla.org/r/186586/#review193762

::: taskcluster/taskgraph/optimize.py:298
(Diff revision 2)
>  class IndexSearch(OptimizationStrategy):
> -    def should_remove_task(self, task, params, index_paths):
> -        "If this task has no dependencies, don't run it.."
> -        return True
>  
> +    # A task with no un-optimized dependencies will be replaced if artifacts

"no un-optimized dependencies" => "fully optimized dependencies"

Comment 17

[Dustin J. Mitchell [:dustin] (he/him)]

Comment on attachment 8915397 [details]
Bug 1405570 - Never remove tasks with an index-search optimization.

https://reviewboard.mozilla.org/r/186586/#review193786

After discussion in irc and some cogitation, this is correct.  The default for `should_remove_task` is False, and in cases where indexed tasks are in the target task graph but nothing depends on them, we usually will want to run them: they produce artifacts which may be useful outside automation or may save time in a subsequent push.  So we should never remove such tasks entirely, but should continue to replace them when a suitable existing task exists.

::: taskcluster/taskgraph/optimize.py:298
(Diff revision 2)
>  class IndexSearch(OptimizationStrategy):
> -    def should_remove_task(self, task, params, index_paths):
> -        "If this task has no dependencies, don't run it.."
> -        return True
>  
> +    # A task with no un-optimized dependencies will be replaced if artifacts

"with no dependencies remaining after optimization"

Comment 18

[Mike Hommey [:glandium]]

Comment on attachment 8915397 [details]
Bug 1405570 - Never remove tasks with an index-search optimization.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/186586/diff/2-3/

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5b8fb23f16af
Never remove tasks with an index-search optimization. r=dustin

Comment 20

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/5b8fb23f16af