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

Added PHP_CLI_MEMORY_LIMIT envvar to configure php cli memory limit #322

Merged
merged 1 commit into from
Oct 30, 2024

Conversation

nicksantamaria
Copy link
Contributor

@nicksantamaria nicksantamaria commented Oct 30, 2024

Motivation

After reducing the PHP_MEMORY_LIMIT envvar on content-vic, builds started breaking due to hitting memory limits in the cli container.

Changes

  • Removes fpm worker limit from CLI container (this container doesnt run a fpm processs so its cruft that can be discarded)
  • Adds a new envvar PHP_CLI_MEMORY_LIMIT that specifically targets the CLI container.
  • PHP_CLI_MEMORY_LIMIT has a default value of 1024M
  • Adds some missing docs for the metrics exporter.

Testing

Check out this branch and build image

docker build -t test -f Dockerfile.cli .

Run the image and check for the memory_limit value.

docker run --rm -it -e PHP_CLI_MEMORY_LIMIT=1024M test php -i | grep memory_limit
memory_limit => 1024M => 1024M
docker run --rm -it -e PHP_CLI_MEMORY_LIMIT=-1 test php -i | grep memory_limit
memory_limit => -1 => -1

@@ -46,9 +46,9 @@ COPY db-build.sh /bay
COPY db-dump-sanitized.sh /bay
COPY mtk /bay/mtk

# Change worker pool from dynamic to static. Change default value to 24.
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to delete this?

@@ -46,9 +46,9 @@ COPY db-build.sh /bay
COPY db-dump-sanitized.sh /bay
COPY mtk /bay/mtk

# Change worker pool from dynamic to static. Change default value to 24.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are these lines being removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh just noticed these to clean up - no fpm process in the cli container so we dont need the worker config here at all. Just cruft.

The same line still exists in the fpm image.

@nicksantamaria nicksantamaria merged commit f1dcad7 into 6.x Oct 30, 2024
1 check passed
@nicksantamaria nicksantamaria deleted the feature/php-cli-memory-limit branch October 30, 2024 05:21
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.

3 participants