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

Support listen to specify sock file #114

Closed
wants to merge 12 commits into from

Conversation

LeiZhang-Hunter
Copy link

I thought of a solution to solve the php-fpm -F problem

  1. Remove libc::prctl (libc::PR_SET_PDEATHSIG, libc::SIGTERM);

  2. Add pid lock to protect the uniqueness of the process, so that the startup process will not be restarted repeatedly

  3. Fix the pid file location

After adding these three items, I solved this problem

@wu-sheng wu-sheng requested a review from jmjoy June 21, 2024 16:19
@wu-sheng wu-sheng added the enhancement New feature or request label Jun 21, 2024
@jmjoy
Copy link
Member

jmjoy commented Jun 22, 2024

Will this result in the forked agent process still existing when php-fpm is exited normally?

@LeiZhang-Hunter
Copy link
Author

Will this result in the forked agent process still existing when php-fpm is exited normally?

Added linux file lock, which solves the problem of repeated fork process; if the process is running, the same process will not be started again due to the file lock. The specific design idea comes from APUE

@LeiZhang-Hunter
Copy link
Author

Will this result in the forked agent process still existing when php-fpm is exited normally?

This perfectly solves the problem of repeated startup of multiple processes

@jmjoy
Copy link
Member

jmjoy commented Jun 22, 2024

Sounds good! Let me try try.

@jmjoy
Copy link
Member

jmjoy commented Jun 22, 2024

For php-fpm -F problem, I think Remove libc::prctl (libc::PR_SET_PDEATHSIG, libc::SIGTERM); is not good. Although it is guaranteed that there is only one worker process, the worker process cannot be stopped by stopping php-fpm, and it will remain on the OS. This is not a good solution and cannot solve the problem of php-fpm daemon mode well. In fact, I want to make an standalone worker binary program to solve this problem later.

For reload php-fpm problem, because it is guaranteed that there is only one worker process, so there can be only one socket file, rather than creating socket files with random names. It can solve the problem well.

@LeiZhang-Hunter
Copy link
Author

For php-fpm -F problem, I think Remove libc::prctl (libc::PR_SET_PDEATHSIG, libc::SIGTERM); is not good. Although it is guaranteed that there is only one worker process, the worker process cannot be stopped by stopping php-fpm, and it will remain on the OS. This is not a good solution and cannot solve the problem of php-fpm daemon mode well. In fact, I want to make an standalone worker binary program to solve this problem later.

For reload php-fpm problem, because it is guaranteed that there is only one worker process, so there can be only one socket file, rather than creating socket files with random names. It can solve the problem well.

Nginx does not set the libc::prctl (libc::PR_SET_PDEATHSIG, libc::SIGTERM) option. The user can write a script to stop the agent instead of following the PHP life cycle. Otherwise, if php-fpm does not add -F, the agent process will automatically exit. The user should decide whether to exit the agent, not php-fpm.

@LeiZhang-Hunter
Copy link
Author

For php-fpm -F problem, I think Remove libc::prctl (libc::PR_SET_PDEATHSIG, libc::SIGTERM); is not good. Although it is guaranteed that there is only one worker process, the worker process cannot be stopped by stopping php-fpm, and it will remain on the OS. This is not a good solution and cannot solve the problem of php-fpm daemon mode well. In fact, I want to make an standalone worker binary program to solve this problem later.

For reload php-fpm problem, because it is guaranteed that there is only one worker process, so there can be only one socket file, rather than creating socket files with random names. It can solve the problem well.

libc::prctl (libc::PR_SET_PDEATHSIG, libc::SIGTERM) I think this option is not suitable here. The user should decide to stop the agent, not php-fpm, which will cause accidental killing in daemon mode.

@LeiZhang-Hunter
Copy link
Author

  • libc::prctl (libc::PR_SET_PDEATHSIG, libc::SIGTERM);

libc::prctl (libc::PR_SET_PDEATHSIG, libc::SIGTERM);

For php-fpm -F problem, I think Remove libc::prctl (libc::PR_SET_PDEATHSIG, libc::SIGTERM); is not good. Although it is guaranteed that there is only one worker process, the worker process cannot be stopped by stopping php-fpm, and it will remain on the OS. This is not a good solution and cannot solve the problem of php-fpm daemon mode well. In fact, I want to make an standalone worker binary program to solve this problem later.

For reload php-fpm problem, because it is guaranteed that there is only one worker process, so there can be only one socket file, rather than creating socket files with random names. It can solve the problem well.

libc::prctl (libc::PR_SET_PDEATHSIG, libc::SIGTERM);
This option will cause users to add the -F option. I think it should not change the user's usage habits.

@jmjoy
Copy link
Member

jmjoy commented Jun 23, 2024

  1. Nginx can handle the master process TERM signal, but php extension can't.
  2. Most current Linux distributions have systemd built in. You can use systemd to host php-fpm without using daemon mode. If you don't have systemd, you can also use supervisord.
  3. The user can write a script to stop the agent instead of following the PHP life cycle., which is not a good user experience.

@LeiZhang-Hunter
Copy link
Author

  • Nginx can handle the master process TERM signal, but php extension can't.
  • Most current Linux distributions have systemd built in. You can use systemd to host php-fpm without using daemon mode. If you don't have systemd, you can also use supervisord.
  • The user can write a script to stop the agent instead of following the PHP life cycle., which is not a good user experience.
  1. Nginx can handle the master process TERM signal, but php extension can't.
  2. Most current Linux distributions have systemd built in. You can use systemd to host php-fpm without using daemon mode. If you don't have systemd, you can also use supervisord.
  3. The user can write a script to stop the agent instead of following the PHP life cycle., which is not a good user experience.

If the user being installed does not use systemd, we should not ask the user to modify the deployment method. We should let the user read the pid through the pid file and then send the SIGTERM signal by himself.

@LeiZhang-Hunter
Copy link
Author

For php-fpm -F problem, I think Remove libc::prctl (libc::PR_SET_PDEATHSIG, libc::SIGTERM); is not good. Although it is guaranteed that there is only one worker process, the worker process cannot be stopped by stopping php-fpm, and it will remain on the OS. This is not a good solution and cannot solve the problem of php-fpm daemon mode well. In fact, I want to make an standalone worker binary program to solve this problem later.

For reload php-fpm problem, because it is guaranteed that there is only one worker process, so there can be only one socket file, rather than creating socket files with random names. It can solve the problem well.

Has this discussion reached a conclusion?

@apache apache deleted a comment from ruiyuantian Jun 25, 2024
@apache apache deleted a comment from ruiyuantian Jun 25, 2024
@wu-sheng
Copy link
Member

Is this going to work? I can see it id stale for weeks.

@wu-sheng wu-sheng closed this Aug 11, 2024
@wu-sheng wu-sheng added the wontfix This will not be worked on label Aug 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request wontfix This will not be worked on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants