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

Improve test containers startup speed #768

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jfbenckhuijsen
Copy link
Contributor

Currently emulators are downloaded on every startup, increasing startup time.

Lets try to avoid that and make the emulator startup way faster

@jfbenckhuijsen jfbenckhuijsen changed the title Try to fix only downloading emulators once Improve test containers startup speed Feb 10, 2025
@jfbenckhuijsen
Copy link
Contributor Author

@loicmathieu This PR implements some aggressive caching for the docker image which is build. I've seen (at least locally) that the docker image is not reused always using layer-caching, resulting in the emulators being downloaded for every test run, which is a huge waste of time (the combined emulators are about 300mb in size).

Also, there was a bug which caused the emulators te be re-downloaded when running the container (effectively downloading 600mb for every test, running on limited data now, so I just noticed it). This is also now fixed, so the emulators are only downloaded once and cached until the user decides the remove the base image.

…h is only rebuild when the firebase-version changes.
@jfbenckhuijsen jfbenckhuijsen force-pushed the feature/improve-startup-time branch from 8770cf5 to 16fbae3 Compare February 10, 2025 21:31
@jfbenckhuijsen jfbenckhuijsen marked this pull request as ready for review February 10, 2025 21:31
@jfbenckhuijsen jfbenckhuijsen requested a review from a team as a code owner February 10, 2025 21:31
Copy link
Collaborator

@loicmathieu loicmathieu left a comment

Choose a reason for hiding this comment

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

I'm not sure I fully understand this issue it tries to fix and the caveats it introduce.
Can you give me more context?

docs/modules/ROOT/pages/firebase-devservices.adoc Outdated Show resolved Hide resolved
@jfbenckhuijsen
Copy link
Contributor Author

The firebase-tools automatically download the individual emulators (firestore, pubsub, etc) when they are used. These emulators are quite large (>40mb, 120mb for the pubsub). These downloads cause a slowdown in the execution of the test-container on startup of quarkus. There's actually 2 issues here.

First there is a bug in the way the image is currently created which causes the firebase emulators to be downloaded twice. Once when the image Is created and once when the image is run.

Second, normally you would want to use the Docker layering setup to ensure that layers are reused when the image is rebuild (on every startup). However what I saw is that when the image is rebuild, for the download steps the layer caching from docker is not used. The second part of the fix builds an intermediate image which is then reused over and over again, ensuring we don't re-download the emulators (which cause the slowdown of building the image).

@loicmathieu
Copy link
Collaborator

This base image will never be updated?
This is a bit risky I think.

@jfbenckhuijsen
Copy link
Contributor Author

TL;DR disagree on the "bit risky" part. If you follow best practices of fixing the firebase-tools version, the behavior is not different from any other testcontainer extension.

Long version:

Once the user removes the base image, the image will be recreated, so never is not correct, but not often, but that's the whole point.

However, the same is true for any base image you use (i.e. we need to manually upgrade node-image version used as a base image, although the user can override that image version himself).

Also, note that the base image is tagged with the firebase version. I.e. it would be kinda logical that the base image won't be updated until you specify a new version of the firebase-tools, which is kinda what you would expect when working with docker in a normal way.

The default version for the firebase tools is "latest", which indeed won't be updated. but again the same is true for any docker image. And personally, I'd say it would be a best practice for anyone using the extension to always specify the firebase version (i.e. not use latest). In that case, the current behavior would actually be the expected behavior when you use any testcontainer.

@loicmathieu
Copy link
Collaborator

The default version for the firebase tools is "latest", which indeed won't be updated. but again the same is true for any docker image.

Latest has a special treatment inside Docker, it is pulled by default (checksum test first).

One solution would be to add a tag that is the same as the version of the library so it is updated each time a new release is done.

@jfbenckhuijsen
Copy link
Contributor Author

Ah right, indeed

  • I'll have a look at that tag solution, see if I can make that work
  • We could also make the version field mandatory (and disallow "latest"), bit more work for a user, but we're advocating best practicers
  • I could look into the behavior that we don't use this is "latest" is specified (which effectively does the same then as the docker semantics, but it takes more time). Not really fond of that solution though

@loicmathieu
Copy link
Collaborator

Yes, we don't want to have too much logic here, this piece of code is already very complicated

@jfbenckhuijsen
Copy link
Contributor Author

Understood and agreed, the most simple solution would then be option (2), always require an actual version number, cause then the setup with the base-image "just works".

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.

2 participants