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

docker: Use mpm_event and php-fpm instead of mpm_prefork and mod_php #33783

Merged
merged 1 commit into from
Nov 2, 2023

Conversation

anomiex
Copy link
Contributor

@anomiex anomiex commented Oct 25, 2023

Proposed changes:

The main hope here is that using mpm_event rather than mpm_prefork will make our E2E tests run better.

But mod_php is not compatible with mpm_event. The modern way to do things is to use php-fpm instead.

And while we're at it, we may as well enable apache's http2 module too. Although actually making effective use of that may take more work (if anyone cares), since HTTP/2 likes to run with TLS but our dev environment doesn't do TLS.

Other information:

  • Have you written new tests for your changes, if applicable?
  • Have you checked the E2E test CI results, and verified that your changes do not break them?
  • Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

pc9hqz-2f3-p2#comment-1825

Does this pull request change what data or activity we track or use?

No

Testing instructions:

  • Temporarily change
    image: automattic/jetpack-wordpress-dev:latest
    to refer to ghcr.io/automattic/jetpack-wordpress-dev:pr-33783 and restart your docker env (jetpack docker stop && jetpack docker up -d). Then make sure everything still works.
  • If you can run the E2E tests locally and reproduce the kinds of problems mentioned in pc9hqz-2f3-p2, see if this helps.

The main hope here is that using mpm_event rather than mpm_prefork will
make our E2E tests run better.

But mod_php is not compatible with mpm_event. The modern way to do
things is to use php-fpm instead.

And while we're at it, we may as well enable apache's http2 module too.
Although actually making effective use of that may take more work (if
anyone cares), since HTTP/2 likes to run with TLS but our dev
environment doesn't do TLS.
@anomiex anomiex added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Pri] Normal labels Oct 25, 2023
@anomiex anomiex self-assigned this Oct 25, 2023
@github-actions
Copy link
Contributor

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • ✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖


The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.


Once your PR is ready for review, check one last time that all required checks appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team Review" label and ask someone from your team review the code. Once reviewed, it can then be merged.
If you need an extra review from someone familiar with the codebase, you can update the labels from "[Status] Needs Team Review" to "[Status] Needs Review", and in that case Jetpack Approvers will do a final review of your PR.

@anomiex anomiex enabled auto-merge (squash) October 25, 2023 16:55
@anomiex anomiex requested a review from a team October 31, 2023 16:00
Copy link
Member

@zinigor zinigor left a comment

Choose a reason for hiding this comment

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

Works well for me!

@anomiex anomiex merged commit 0340106 into trunk Nov 2, 2023
54 checks passed
@anomiex anomiex deleted the update/docker-env-use-php-fpm branch November 2, 2023 12:45
@github-actions github-actions bot removed the [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. label Nov 2, 2023
@anomiex anomiex mentioned this pull request Nov 2, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docker [Pri] Normal [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants