1492362 - run-task removes MOZ_FETCHES_DIR at the end of the build

Status: RESOLVED FIXED

(Firefox Build System :: Task Configuration, task)

Description

[Mike Hommey [:glandium]]

This is not so much of a problem on normal toolchain builds, because the next build doesn't really care but this is time spent replicating cleanup that the worker is probably going to do anyways, and more importantly, the code is meant to remove the product of extracting fetches, not the source directory! The problem being that the removal code is essentially assuming the directory only contains fetches when, in fact, it also contains the source directory.

The other problem is that a side effect of this is that when you get an interactive task for toolchain jobs, you end up with the source directory gone...

Comment 1

[Tom Prince [:tomprince]]

This was was added to support using run-task on generic-worker's that were not cleaning up between runs.

Comment 2

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

Does that mean it can be reverted now?

Comment 3

[Tom Prince [:tomprince]]

I do not know if there have been any relevant changes to generic-worker to address the original issue. I'd guess that the tasks are using a stable path, which may not be as well supported in generic-worker.

Comment 4

[Pete Moore [:pmoore][:pete]]

I'm guessing no cache is declared in the task?

See "Writable Directory Cache" on https://docs.taskcluster.net/docs/reference/workers/generic-worker/docs/payload

Comment 5

[Mike Hommey [:glandium]]

The cleanup is done by the fetches code when MOZ_FETCHES_DIR is set, which it is set to /builds/worker/workspace/build for those jobs, which is where the source also is. I don't know if the fetches code can be changed to stop doing that, which is essentially the question from comment 2.

Comment 6

[Mike Hommey [:glandium]]

Another problem this causes is that for jobs using fetches, the contents of MOZ_FETCHES_DIR disappears from under you when the build finishes on an interactive task. That's not exactly pleasant.

Comment 7

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to Mike Hommey [:glandium] from comment #6)

Another problem this causes is that for jobs using fetches, the contents of MOZ_FETCHES_DIR disappears from under you when the build finishes on an interactive task. That's not exactly pleasant.

To do a quickish fix for just the interactive task case, would it be possible to make run-task aware of whether it's running in interactive mode, and just not do this cleanup[1] if that's true?

1. https://searchfox.org/mozilla-central/source/taskcluster/scripts/run-task#845-848

Comment 8

[Mike Hommey [:glandium]]

We can't really know if a task is interactive from the task itself, unfortunately. The best approximation is the TASKCLUSTER_INTERACTIVE environment variable, and that's only set if you "Edit as interactive task", which is not supported on Windows. In fact, "interactive tasks" on Windows don't even have the interactive feature enabled.

Comment 9

[Mike Hommey [:glandium]]

With bug 1524637 being closed (by having disabled caches), can we make run-task stop this cleanup?

Comment 10

[Tom Prince [:tomprince]]

Bug 1466660 was the source of this cleanup (this commit. The workers in question are generic-workers, thus the changes in bug 1524637 aren't relevant to the original reason for this code. I think once Bug 1499054 is deployed on the linux talos workers that we can remove this workaround.

Comment 12

[Tom Ritter [:tjr]]

Got bit by this again today; with Bug 1499054 resolved, can we remove it now?

Comment 13

[Tom Prince [:tomprince]]

I don't think that workers have been updated to take advantage of Bug 1499054.

Comment 14

[Andrew Erickson [:aerickson]]

android-hw TC jobs (that run on Bitbar) run in a clean environment (a new docker container) for each job. They shouldn't block disabling cleanup.

Bug 1583936 tracks making the android-hw jobs use g-w multiuser.

Comment 15

[:dhouse]

(In reply to Tom Prince [:tomprince] from comment #13)

I don't think that workers have been updated to take advantage of Bug 1499054.

The testing and switch to multi-user generic-worker on linux hardware is in bug 1585389

Comment 16

[Tom Prince [:tomprince]]

We added the removal of fetches in Bug 1466660. It turns out this is because we have a few tests that use hard-coded directories in a few tests (here and here, rather than the already existing per-task directories. If we update those to use the per-task directories, we don't need to remove the fetches at the end of a task.

Comment 17

[Tom Prince [:tomprince]]

Attachment: Bug 1492362: [taskgraph] Only use absolute paths for checkouts in docker; r?glandium

We define a value run.workdir to point to the defaut directory to use for
all sorts of paths, including checkouts, in the job transform. However, that
directory only makes sense for docker-worker tasks, and relative paths should
be used elsewhere. Adjust the paths on non-docker-worker linux tasks to match
macOS paths.

Comment 18

[Tom Prince [:tomprince]]

Attachment: Bug 1492362: [taskgraph] Stop setting `workdir` on generic-worker tasks; r?glandium

After changing the default checkout path on linux generic-worker tasks to be
relative these settings have no effect.

Comment 19

[Tom Prince [:tomprince]]

Attachment: Bug 1492362: [taskgraph] Don't delete fetches after running a task; r?glandium

In Bug 1466660, we started deleting the fetches after a task had run, to avoid
interference between tasks. It turns out the only tasks this was for were the
source-test-jsshell tasks, which were changed to use an absolute directory in
Bug 1465181. However, since Bug 1568460 we've always used a per-task directory
for fetches, so can remove the work-around of removing fethes.

Comment 20

[Tom Prince [:tomprince]]

Attachment: Bug 1492362: [mozharness] Set reasonable `avds_dir` values;

Comment 21

[Simon Fraser [:sfraser] ⌚️GMT]

(In reply to Tom Prince [:tomprince] from comment #19)

Created attachment 9169350 [details]
Bug 1492362: [taskgraph] Don't delete fetches after running a task; r?glandium

In Bug 1466660, we started deleting the fetches after a task had run, to avoid
interference between tasks. It turns out the only tasks this was for were the
source-test-jsshell tasks, which were changed to use an absolute directory in
Bug 1465181. However, since Bug 1568460 we've always used a per-task directory
for fetches, so can remove the work-around of removing fethes.

ah, so we likely shouldn't use the change in https://bugzilla.mozilla.org/show_bug.cgi?id=1658000 but instead change ccov to not remove the entire fetches directory as well

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a20562dbfece
[taskgraph] Only use absolute paths for checkouts in docker; r=glandium
https://hg.mozilla.org/integration/autoland/rev/ea667ccd4b19
[taskgraph] Stop setting `workdir` on generic-worker tasks; r=glandium
https://hg.mozilla.org/integration/autoland/rev/225a6bfe8cbf
[taskgraph] Don't delete fetches after running a task; r=glandium
https://hg.mozilla.org/integration/autoland/rev/8e6cf79c70f7
[mozharness] Set reasonable `avds_dir` values; r=ahal

Comment 23

[Natalia Csoregi [:nataliaCs]]

Backed out for causing gecko decision task bustage

backout: https://hg.mozilla.org/integration/autoland/rev/4255a87fdd98cca564b89f79350e310d855d6f9d

push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=8e6cf79c70f71bb3a85cf5c6540697fc71bd7ba6&selectedTaskRun=K-MmZWoATr2_I_WubGXahQ.0

failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=313382885&repo=autoland&lineNumber=6233

Comment 24

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/485f333bc3b7
[taskgraph] Only use absolute paths for checkouts in docker; r=glandium
https://hg.mozilla.org/integration/autoland/rev/ae17b683bf71
[taskgraph] Stop setting `workdir` on generic-worker tasks; r=glandium
https://hg.mozilla.org/integration/autoland/rev/89ecd6ee8edd
[taskgraph] Don't delete fetches after running a task; r=glandium
https://hg.mozilla.org/integration/autoland/rev/ac0fdd0c661e
[mozharness] Set reasonable `avds_dir` values; r=ahal

Comment 25

[Narcis Beleuzu [:NarcisB]]

Backed out for android wpt failures.

Backout link: https://hg.mozilla.org/integration/autoland/rev/aed8ee7d70f420c7ff93109dcf9f3f3733bcfff6
Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=313426019&repo=autoland&lineNumber=637

Comment 26

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/99bba94234ca
[taskgraph] Only use absolute paths for checkouts in docker; r=glandium
https://hg.mozilla.org/integration/autoland/rev/b9ba4f0099c6
[taskgraph] Stop setting `workdir` on generic-worker tasks; r=glandium
https://hg.mozilla.org/integration/autoland/rev/673b01c81db2
[taskgraph] Don't delete fetches after running a task; r=glandium
https://hg.mozilla.org/integration/autoland/rev/4b9deff4865e
[mozharness] Set reasonable `avds_dir` values; r=ahal

Comment 27

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/99bba94234ca
https://hg.mozilla.org/mozilla-central/rev/b9ba4f0099c6
https://hg.mozilla.org/mozilla-central/rev/673b01c81db2
https://hg.mozilla.org/mozilla-central/rev/4b9deff4865e