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

Fastcgi #81

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Fastcgi #81

wants to merge 8 commits into from

Conversation

ebernhardson
Copy link
Contributor

As discussed in issue #76 this is a refactor of the the current fork/in-process strategy into independent objects along with the addition of a fastcgi strategy to utilize php-fpm.

A couple notes:

To pass the job between the worker and fastcgi I settled on serializing the job and passing as an environment variable. Specifically when failing a job both the worker and job instances are required. I thought it best to serialize and pass both rather than make changes to the failure code.

I pulled the FCGIClient into the repository, currently under the BitTP namespace as that is where i found it. I had to make a few changes to this class to make it fit our usage; Specifically it was originally written using non-blocking sockets and busy-waiting to read the socket. For our purposes a blocking fastcgi client is more appropriate so i made that change. It may be more appropriate to re-namespace or put into a repo on its own to be referenced by composer.

I could not come up with a good way to programmatically test this code. Its fairly straight forward code, and I've run numerous tests by hand, but needs automated on-going testing one way or the other. I'm open to suggestions which i can flush out into proper tests.

@chrisboulton
Copy link
Owner

This looks good 👍.

Before I bring it in, I want to work on externalizing the FCGIClient (essentially creating a Composer package for it), and obviously work on the testing. If you don't have the time to do this, I'll probably begin work on getting this down sometime in the next two weeks.

Nice job!

@ebernhardson
Copy link
Contributor Author

The FastCGI client has been extracted into a suggested package, https://github.com/ebernhardson/fastcgi , and the pull request has been updated.

Still need to figure out a full featured way to test the JobStrategy interactions.

@chrisboulton
Copy link
Owner

Awesome. 👍

Sorry, I meant to spend some time on this but dropped the ball because I've been busy with work and life at the moment. I definitely appreciate the effort you're putting into this.

homeyjd added a commit to dmvorg/php-resque that referenced this pull request Jan 5, 2015
@allan-simon
Copy link

what is the status on this PR ? is help needed ?

@danhunsaker
Copy link
Contributor

I'd love to see this, or something very much like it, in the new repo at resque/php-resque, if anyone would like to pick it up and rebase/rebuild it there?

@adri
Copy link

adri commented Aug 7, 2023

@ebernhardson Hello and sorry for digging up this old pull request. This approach seems quite interesting to me from a performance perspective. Did you ever end up running this in production?

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.

5 participants