Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates for jenkins-workspace binary temporaries repository #1515

Conversation

joaocgreis
Copy link
Member

This does two things:

  • Increase MaxSartups in sshd_config. This is necessary when many workers running tests try to access the temp repo to download binaries simultaneously. Not sure if there is an optimal value for this. Since the previous value seemed to just barely not be enough, I increased it 10x considering it is possible that more than one job might be started at the same time.
  • Weekly clean-up of the repository. This deletes branches where the last commit is more than one month old. This is not ideal, but since Keeping old binaries to allow resuming fanned jobs #1496 the branches no longer have the number of the job that started them, so looking at Jenkins jobs is no longer possible. This replaces git-clean-temp-repo, which was broken not only because of this but also because unauthenticated access to Jenkins is disabled.

This is necessary when many workers running tests try to access the
temp repo to download binaries simultaneously.
@refack
Copy link
Contributor

refack commented Oct 4, 2018

Question: any reason not to use Jenkins' built-in cron (just so that we don't have configuration in too many places)?
image

@joaocgreis
Copy link
Member Author

@refack was there before, I removed it because the job is broken. Deleting the job and archiving as XML here is the next step there.

fi
done

git prune
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe add a nohup git gc >& ~binary_tmp/git_gc_`date +"%s"`.log ?
That way we won't get an auto gc in the middle of a future fetch

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see below about gc.

git fetch origin +master:master

for b in $(git branch | sed /\*/d); do
if [ -z "$(git log -1 --since='4 weeks ago' -s $b)" ]; then
Copy link
Contributor

@refack refack Nov 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if [ -z "$(git log -1 --since='4 weeks ago' -s $b)" ]; then
if [ -z "$(git log -1 --since='5 days ago' -s $b)" ]; then

IMO 5 days is enough. After five days we should run a rebuild not a resume (since master probably progressed).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agreed, 4 weeks is a long time, any good reason for that @joaocgreis?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find that after a week the jobs are most often no longer in the history anyway. Maybe 7 days?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

7 days sounds reasonable, updated.

joaocgreis added a commit to JaneaSystems/build that referenced this pull request Nov 30, 2018
This job no longer works because Jenkins requires a token.

Replaced by: nodejs#1515
@joaocgreis
Copy link
Member Author

Updated (9b6f4b0), PTAL.

  • Changed the expiry time to 7 days.
  • Changed the script to run daily.
  • Disabled git auto gc, since it can interfere with fetching from test jobs. I don't think there's a need to run the full gc in the script, since repacking does not make sense for this usage and we already do the pruning explicitly. The full git gc takes hours to complete, so I've let it out, but we can add it to the script if there's a need for it.
  • Changed logging to not include bash output and added another du in the beginning.

@rvagg rvagg self-requested a review December 1, 2018 08:52
Copy link
Member

@rvagg rvagg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

joaocgreis added a commit that referenced this pull request Dec 6, 2018
This is necessary when many workers running tests try to access the
temp repo to download binaries simultaneously.

PR-URL: #1515
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
joaocgreis added a commit that referenced this pull request Dec 6, 2018
PR-URL: #1515
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Rod Vagg <rod@vagg.org>
joaocgreis added a commit that referenced this pull request Dec 6, 2018
This job no longer works because Jenkins requires a token.

Replaced by: #1515

PR-URL: #1618
Reviewed-By: Rod Vagg <rod@vagg.org>
@joaocgreis
Copy link
Member Author

Landed in a1957ef...5961359

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants