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

Use bootsnap to improve startup time. #2626

Merged
merged 2 commits into from
Dec 13, 2022
Merged

Use bootsnap to improve startup time. #2626

merged 2 commits into from
Dec 13, 2022

Conversation

sengi
Copy link
Contributor

@sengi sengi commented Dec 8, 2022

We seem to be using bootsnap on most of the publisher apps but not on the frontends.

We don't care much about startup time per se, but reducing the CPU spike on pod startup is helpful for capacity planning and helps to keep the overall performance of the cluster more predictable.

Tested: works on the integration k8s cluster. Benchmarking locally with docker run showed >40% improvement in startup time and savings of roughly 2 core-seconds per pod startup. Regression tested on EC2 by deploying to the integration environment, checking that smoke tests still pass there and checking there were no errors/warnings in app logs.

@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2626 December 8, 2022 17:48 Inactive
@sengi sengi force-pushed the sengi/dockerfile-cleanup branch from b072719 to 071ec26 Compare December 9, 2022 10:11
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2626 December 9, 2022 10:11 Inactive
@sengi sengi marked this pull request as ready for review December 9, 2022 11:35
Dockerfile Show resolved Hide resolved
@sengi
Copy link
Contributor Author

sengi commented Dec 9, 2022

Thanks! Since it's Friday afternoon and this thing's push-on-green I'll wait until Monday before merging :D

- Add .dockerignore. docker build no longer fails when there are build
  artefacts in the source directory.
- Use `$APP_HOME` from the base image instead of hardcoding `/app`
  throughout.
- Remove unnecessary and misleading definition of `PORT` env var.
- Re-enable bootsnap.
- Simplify the command, now that we no longer need to run `bundle exec`
  explicitly.
@sengi sengi force-pushed the sengi/dockerfile-cleanup branch from 071ec26 to c5d5971 Compare December 13, 2022 15:11
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-2626 December 13, 2022 15:11 Inactive
@sengi sengi merged commit 9abb78b into main Dec 13, 2022
@sengi sengi deleted the sengi/dockerfile-cleanup branch December 13, 2022 15:17
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