Keeping old binaries to allow resuming fanned jobs

[joaocgreis]

Currently, we delete the temporary branch at the end of the fanned jobs. This means that when a fanned job is resumed, if the compile job succeeded only the test job will run again, and will not be able to find the temporary branch from last run.

I'd like to stop deleting temporary branches at the end of the runs, but this change is not urgent so it should be discussed with @nodejs/build first. Also, it would be good to do this when we have people available in the next few days to put out any fire.

At first, this sounds like the size of the binary repo could become a problem. However, git keeps references around long after the branches are deleted, so this might have a very small impact. Note that we have a job that runs daily and deletes branches for which the Jenkins job is no longer accessible, so branches will still get deleted after about one week.

Emergency cleanup of the binary repo

ssh jenkins-workspace-1
su binary_tmp -s /bin/bash
cd ~/binary_tmp.git
du -sh .
git prune
du -sh .

[Trott]

this change is not urgent

If we can get this working before Code + Learn, that could be immensely helpful.

[joaocgreis]

I've changed the jobs, will monitor the temporary repo.

For now the changes were minimal (changed the delete parameter to false). If all goes well, in a couple of months will remove the parameter and the build step.

[joyeecheung]

https://ci.nodejs.org/job/node-test-commit-windows-fanned/20827/

https://ci.nodejs.org/job/node-test-binary-windows/COMPILED_BY=vs2017-x86,RUNNER=win2012r2,RUN_SUBSET=1/20072/console

10:25:29 c:\workspace\node-test-binary-windows>git checkout -f refs/remotes/jenkins_tmp/jenkins-node-test-commit-windows-fanned-20827-bin-win-vs2017-x86 
10:25:29 error: pathspec 'refs/remotes/jenkins_tmp/jenkins-node-test-commit-windows-fanned-20827-bin-win-vs2017-x86' did not match any file(s) known to git.

This job started 6hrs ago. Looks like it did not resume successfully

[refack]

It did not work because the test branch was named using:

BUILD_TAG
String of "jenkins-${JOB_NAME}-${BUILD_NUMBER}"

Which changes in the resumed job.

I've changed in to ${JOB_NAME}-${GIT_COMMIT}
https://ci.nodejs.org/job/node-test-commit-windows-fanned/jobConfigHistory/showDiffFiles?timestamp1=2018-09-16_23-11-57&timestamp2=2018-09-18_17-21-11

Let's see if that works

[refack]

I've changed in to ${JOB_NAME}-${GIT_COMMIT}

I reverted that because now node-test-commit-windows-fanned is not a git job...

[joaocgreis]

@refack not a git job, but it may receive GIT_COMMIT as a parameter. Here's another try: https://ci.nodejs.org/job/node-test-commit-windows-fanned/jobConfigHistory/showDiffFiles?timestamp1=2018-09-18_22-40-44&timestamp2=2018-09-18_22-44-06

[joaocgreis]

Resuming at the level of node-test-commit-windows-fanned works, but does not at any level above. This is because the REBASE_ONTO parameter gets updated.

(My test run yesterday: https://ci.nodejs.org/job/node-test-commit-windows-fanned/20840/ and https://ci.nodejs.org/job/node-test-commit-windows-fanned/20841/ , @Trott failed run now: https://ci.nodejs.org/job/node-test-pull-request/17320/ and https://ci.nodejs.org/job/node-test-pull-request/17327/ .)

I'm not sure whether we should ignore the REBASE_ONTO parameter for this. The fanned jobs will never rebase when resumed while others do and if there is any issue with the job understanding it will be even more confusing. There will be interference if multiple jobs for the same commit happen to run simultaneously. Without a better solution, I'm leaning towards giving this a try. What do you think?

[joaocgreis]

Let's try it: https://ci.nodejs.org/job/node-test-commit-windows-fanned/jobConfigHistory/showDiffFiles?timestamp1=2018-09-18_22-44-06&timestamp2=2018-09-20_23-08-36 . This might cause subtle issues in CI, if the same commit has more than one job running simultaneously. Since the branch name includes the commit SHA, it should be safe, but we might end up testing outdated rebases and that can be confusing.

[refack]

Resuming at the level of node-test-commit-windows-fanned works, but does not at any level above. This is because the REBASE_ONTO parameter gets updated.

I think that is a bug. We should strive to have "resume" use the exact same parameters (in this case a SHA and not a ref for the rebase point). I'll look at that...

[refack]

This might cause subtle issues in CI, if the same commit has more than one job running simultaneously.

IMHO we can consider skipping compilation job altogether, if we find the binary branch...

[joaocgreis]

I think that is a bug. We should strive to have "resume" use the exact same parameters (in this case a SHA and not a ref for the rebase point). I'll look at that...

I don't think that's going to be possible (but would be great to find a way!). The only thing you have is "rebase against the base branch", so you have to find out what the "base branch" is somewhere. Resuming at that level will calculate the SHA again... unless we find a way to know the job is a resume.

IMHO we can consider skipping compilation job altogether, if we find the binary branch...

For new builds or rebuilds, it should always rebase against the latest and compile again. It would be nice to have the resume feature working well, but it's not worth interfering with fresh builds because of it.

I've changed the compile job to delete the binary branch if found, before starting to compile. This is to make sure that if the compile job fails the test job doesn't simply pick an old branch and tests that.

If the current approach has issues, we could try to propagate a branch name from the top, from the test-PR job. Then, allow resuming only on the test-commit level and below. But this is one more bit of complexity, so I'd avoid it if possible.

[joaocgreis]

Changed the ARM jobs: https://ci.nodejs.org/view/All/job/node-test-commit-arm-fanned/jobConfigHistory/showDiffFiles?timestamp1=2018-10-10_23-08-16&timestamp2=2018-10-11_00-03-21 and https://ci.nodejs.org/job/node-cross-compile/jobConfigHistory/showDiffFiles?timestamp1=2018-10-10_23-38-49&timestamp2=2018-10-10_23-46-21

[refack]

needed two more changes

• Stop deleting temp branch on fail - https://ci.nodejs.org/job/node-test-commit-arm-fanned/jobConfigHistory/showDiffFiles?timestamp1=2018-10-11_00-03-21&timestamp2=2018-10-11_19-00-51
• Propagate GIT_COMMIT - https://ci.nodejs.org/job/node-test-commit/jobConfigHistory/showDiffFiles?timestamp1=2018-09-26_12-59-09&timestamp2=2018-10-11_19-11-38

[refack]

@joaocgreis PTAL
I've chanched two values in the config https://ci.nodejs.org/job/node-test-commit-windows-fanned/jobConfigHistory/showDiffFiles?timestamp1=2018-10-30_19-03-30&timestamp2=2018-11-17_11-26-47

1. Don't try to test the binaries if the compilation step failed
2. Delete the temp branch if build is stable (i.e. ✔️ )

[joaocgreis]

@refack LGTM, thanks!

I think we're all done here. #1515 is still open but we can discuss there.