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

Optimize docker memory #976

Merged
merged 3 commits into from
Dec 22, 2023

Conversation

ninele7
Copy link
Contributor

@ninele7 ninele7 commented Oct 18, 2023

When running this bot in docker container on my VPS server I encountered high ram usage. After investigation I found out that it runs with chain of yarn/npm/tsx/run-with-database-url.ts.

In this PR I:

  • Fixed imports to make code work after tsc.
  • Added compilation stage with tsc.
  • Replaced run-with-database-url.ts with simpler solution that doesn't run additional node processes in runtime.
  • Replaced yarn start with direct invocation of node in dockerfile.

Before PR docker stats displayed 300-400mb of memory usage. Now it's 60-120mb.

  • I updated the changelog

@github-actions
Copy link

github-actions bot commented Oct 18, 2023

📦 A new release has been made for this pull request.

To play around with this PR, pull codetheweb/muse:pr-976 or codetheweb/muse:4860adb9f168517e2680fed8781c92cd38a206fc.

Images are available for x86_64 and ARM64.

Latest commit: 4860adb

@ninele7 ninele7 force-pushed the optimize-docker-memory branch from 9940687 to 1666045 Compare October 18, 2023 22:33
@ninele7 ninele7 marked this pull request as draft October 18, 2023 22:34
@ninele7 ninele7 closed this Oct 18, 2023
@ninele7 ninele7 force-pushed the optimize-docker-memory branch from 1666045 to a325308 Compare October 18, 2023 22:43
@ninele7 ninele7 reopened this Oct 18, 2023
@ninele7 ninele7 force-pushed the optimize-docker-memory branch from ce3d97b to 1e85acb Compare October 18, 2023 23:30
@ninele7 ninele7 marked this pull request as ready for review October 18, 2023 23:31
Copy link
Collaborator

@codetheweb codetheweb left a comment

Choose a reason for hiding this comment

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

apologies for the late review, thanks for the contribution!

Dockerfile Outdated
@@ -34,4 +45,6 @@ ENV NODE_ENV production
ENV COMMIT_HASH $COMMIT_HASH
ENV BUILD_DATE $BUILD_DATE

CMD ["tini", "--", "yarn", "start"]
RUN echo "DATABASE_URL=$(node dist/scripts/print-database-url.js)" > .env
Copy link
Collaborator

Choose a reason for hiding this comment

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

we shouldn't bake the DATABASE_URL into the image, as it potentially exposes a secret, won't work for the images published publicly on Docker Hub, and makes it harder to change

I think it should work without this step?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now it can't expose any secret because location of file with SQLite DB isn't a secret.

But I changed it to be more configurable.

@@ -14,18 +14,29 @@ COPY package.json .
COPY yarn.lock .

RUN yarn install --prod
RUN cp -R node_modules /usr/app/prod_node_modules
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would recommend doing yarn install (all dependencies) in this dependencies layer, then not copying node_modules and instead running yarn install --prod in the runner image to simplify it a little

Copy link
Contributor Author

@ninele7 ninele7 Nov 9, 2023

Choose a reason for hiding this comment

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

I've done it to avoid downloading prod dependencies twice. That way it seems to work faster. But if you prefer calling downloading twice I will change it.

@codetheweb codetheweb merged commit d8d7207 into museofficial:master Dec 22, 2023
5 checks passed
Copy link

🚀 Released in Release v2.4.4.

@ninele7 ninele7 deleted the optimize-docker-memory branch December 23, 2023 08:24
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