Rewrite on top of Aerys

[kelunik]

See #146 for discussion.

Fixes #146, #249, #265.

See also php-pm/php-pm-httpkernel#61.

[andig]

I'm really against this in its current form. Downstream applications like Volkszaehler add additional functionality on basis of reactPHP. Forcing those to move their platform is imho not an option.
If aerys is to be supported it should imho be in addition, not instead.

While reactPHP might be slow I really appreciate the quality of the code delivered.

[kelunik]

This doesn't conflict with applications running on ReactPHP at all. If anything, then this improves the situation for them, because the ReactPHP versions used by PHP-PM are no longer conflicting with the ReactPHP versions used by the application.

However, there is an issue with applications using Amp currently, but I'll resolve that soon.

[marcj]

😱 This is huge. Awesome! If Aerys brings us the features we need (http multipart parsing, file upload and more stable data routing) then this is a pretty huge milestone for PHP-PM, as it means we finally support HTTP, which we probably never will with ReactPHP as it never reached such a state and probably never will with the current man-power/commitment behind it. So, for me the only logical way to continue is to add Aerys support and drop ReactPHP. If we won't, we will wait another 1-2 years until ReactPHP's http library reaches full HTTP support.

Huge +1 from me.

Is is much work to add file upload and POST support? https://github.com/php-pm/php-pm-httpkernel/pull/61/files#diff-7a7e05cf20fe81b51c4f4b40d6a919a7R149

[trowski]

@andig Note that Amp is completely compatible with any code written for ReactPHP. Any projects based on ReactPHP will only need to make a minor change to use the React\Loop\LoopInterface instance provided by the adapter package from Amp: https://github.com/amphp/react-adapter.

@marcj Glad to see you're excited about this – we certainly are excited to bring more features and better performance to PHP-PM.

[andig]

Why remove the Async interface? The idea was to give hosted applications access to the same loop for additional async functionality?

[kelunik]

Because it's entirely unused. With #268 concurrent requests might be unsupported in the future and in that case applications should use their own loop instead of reuse the same loop. Reasons for that are for example that a stopping of the event loop would also stop the outer event loop run.

[andig]

Because it's entirely unused.

It's used here: https://github.com/php-pm/php-pm-psr7/blob/d1c225469f1ad2d38c78c571018967cc0186c290/src/Psr7Bridge.php#L34. The idea was to add it to HttpKernel as well.

[kelunik]

Let's decide on #268 first before continuing the discussion whether it's still something that's needed.

[andig]

Any projects based on ReactPHP will only need to make a minor change to use the React\Loop\LoopInterface

I'm beginning to see. Sounds good! My question would be similar to marcj- one thing we wanted to profit from from react was HTTP request parsing. Also the new PSR7 (half)compatibility is a pro. Are there plans for Aerys to support the same?

[kelunik]

@andig Request parsing is definitely supported, but it might need some minor adjustments to be compatible with PHP's default parsing, i.e. the handling of foo[]=test creating an array instead of being available as foo[] => test. There's an open TODO in the PR for that, I'll definitely add that, but would prefer investing the time only if the other parts are accepted.

PSR-7 (half)compatibility will definitely not be supported in Aerys directly, simply because full support isn't possible in a sane way (i.e. not buffering the complete body into memory).

[kelunik]

@marcj I just added $_POST support. File support is still missing, I'm not sure whether we can even emulate that, I'll have to test that.

[kelunik]

I'd be glad if someone could provide some Symfony demo application with file uploads to test.

[kelunik]

@marcj Seems like we can't get is_uploaded_file and move_uploaded_file working.

[marcj]

@kelunik that's ok :) you're forced to use an abstraction anyway, so no need to call those functions when you use PHP-PM.

[kelunik]

@marcj But Symfony uses these I think.

[marcj]

I believe it uses them only when your create a Request from globals, like in app.php.

[andho]

In Symfony you can choose a different class for File(Upload) handling. I did this for a system which stored files on an NFS Share. move_uploaded_file didn't work with the NFS share, so my alternate class used stream_copy_to_stream instead.

Wait, that might be from the VichUploader bundle.

[max-voloshin]

Hi, @kelunik! I'm going to use PHP-PM on top of Ayres in some new project. What is the easiest way to start writing controllers with that approach? Should I implement ApplicationEnvironmentAwareInterface, BootstrapInterface and BridgeInterface adapters with \Aerys\Router?

[kelunik]

@max-voloshin If you're using Symfony, everything will be just as before. There's a PR for the Symfony kernel: php-pm/php-pm-httpkernel#61. If you plan to use the Aerys APIs directly, I'd recommend using Aerys directly instead of using PHP-PM as a layer in between. If you have any questions about that approach, feel free to reach out via any of the channels listed on https://amphp.org/support.

[max-voloshin]

@kelunik thank you, I clarified my question: https://stackoverflow.com/questions/46525897/usage-ppm-on-top-of-aerys

[marcj]

Alrighty guys, how should we proceed? As already pointed out, we need Aerys to have the fundamental feature-set (http multipart parsing, file upload, ...) working. Maybe @clue can give us more insights about ReactPHP's development, so we can decide which way to go. I guess there's only one way: either Aerys or ReactPHP, but not both. Or do you guys see a different route?

The only reason I'd prefer Aerys is the features-set they provide. Code quality and other topics keep me away from it. But I'd swallow the pill if ReactPHP can't provide following features in the near feature:

• Multipart parsing
• File upload
• HTTP/2 (would be cool)

Please throw out your opinions so we can make PHP-PM finally production ready. We need a decision soon as all PRs an dangling since we can't assume @kelunik will rebase all changes again. So we should wait until we get a decision on this one.

[andig]

Maybe @clue can give us more insights about ReactPHP's development, so we can decide which way to go.

React is almost there. All required PRs exist and its up to react to decide when they want to merge (dev-master) and what else need be included before 0.8 can be released. Its imho sooner rather than later. Only exception is HTTP/2.

[marcj]

Only exception is HTTP/2.

Is more optional than requirement I guess. (adjusted my list)

[optiman]

Another reason for Aerys is much better performance:
https://github.com/torinaki/aerys-benchmark

[dzubchik]

@marcj and @andig, we see the work you have done and we really appreciate this. It would be nice to have file uploads and multipart parsing now. But, IMO, the project has a lack of tests (I mean integration tests with bridges) and without them, only crazy people would use it on production (as we do). Is it a big problem to choose react or aerys? Maybe we could write an adapter with interfaces like file uploads, http/2, etc?

[kelunik]

@optiman The posted benchmark isn't really fair as ReactPHP doesn't support keep-alive, yet. But even without keep-alive Aerys serves 1.5k requests / s while ReactPHP serves 1.1k requests / s for me.

But, IMO, the project has a lack of tests (I mean integration tests with bridges) and without them, only crazy people would use it on production (as we do).

I'd say that the lack of releases is equally important. Relying on dev-master isn't a good thing.

While HTTP/2 support is nice to have, it's not as important as one might think. Currently I wouldn't recommend using PHP directly for TLS termination, but HTTP/2 will only be used for https:// sites by browsers. It might still be very useful if your reverse proxy supports h2c.

TBH, there are a few other things missing for production readiness. Currently PHP-PM just forwards all client data from a single process to the child processes to handle it. Usually that won't be a bottleneck I guess, but that might depend on the application and how many cores you have available.

A client is currently assigned to exactly one worker. The master process doesn't know anything about requests. That means a worker processes all requests from one connection. This might result in a sub-optimal request scheduling that PHP-PM is supposed to solve by choosing a free worker for a new connection. A solution to this is to directly run react/http / Aerys and using their request handlers to dispatch requests to workers instead of assigning connections to workers.

On the other hand this can be seen as unnecessary, as it works good enough without per-request scheduling. I know of people using Aerys directly for serving a totally synchronous applications and it works good enough for a high-traffic site. Maybe Aerys could be used directly in the future without PHP-PM and the bridges be built to map directly from Aerys requests. Aerys already includes a process manager and restarts crashing workers.

[andig]

These are the things I can currently see on the priority list:

• rely on stable releases (whatever those are) for as-is functionality including multipart and files- afaik react/http will provide a basis with 0.8 after a longish two-year wait
• restructure and rewrite PPM (maybe with additional goals as outlined by @kelunik) - while the current code works it's highly complex (large methods, only few source files) and hard to maintain. A moot point unless dedicated capacity becomes available.
• create a solid test infrastructure

React does imho provide battle-proven code of excellent quality and a solid base of maintainers. For react/http full PSR7 support via RequestBodyBufferMiddleware is a (small) plus that could remove the need for the PSR7 bridge.
For those who're interested in archeology both ppm and react still support php5. I can only assume that might at least contribute to the performance delta due to clumsier code.

Maybe Aerys could be used directly in the future without PHP-PM and the bridges be built to map directly from Aerys requests. Aerys already includes a process manager and restarts crashing workers.

Interesting. Would this speak against all of the above topics?

[andig]

The only reason I'd prefer Aerys is the features-set they provide. ... Multipart parsing ... File upload

Please see #271. I would appreciate testing as I'm not fluent with symfony. File uploads should be working (not tested!) as the react MultipartParser already supports files.

not working yet is UDS on OSX (waiting for react/socket release this weekend, workaround included) and chunked request handling (waiting for react/http merge).

[clue]

@marcj Thank your for your patience and friendly ping. I think that you brought up some very valid points and I very much appreciate this PR and the related discussion!

I understand that it may be frustrating that @reactphp may not always develop at the speed we all wish :-) That being said, I think we've made some major improvements in the past months and are actively working on getting these features out. All of your above wishlist items are already implemented in the current development version and we're currently actively working with @andig (thank you!) to get some minor additional features released that are also used as part of php-pm.

I mostly agree with your assertion, I can assure you the @reactphp teams values stable, sustainable and high quality components over quick hacks. We're currently in the process of stabilizing things and can assure you we'll get a stable release out as soon as possible.

Given that your above wishlist items are already implemented, you can try this out by relying on dev-master (see also @andig's excellent PR!). Of course, relying on a branch long-term is not a good idea, but this should give you a good overview of the current state which should be really stable already and will be part of the next release.

I'm giving a few talks in the next two weeks, so I won't be able to make any promises in this time frame. But rest assured, this is a high priority item on my list immediately thereafter.

Thank you! <3

[trowski]

Note that choosing Amp/Aerys as a basis for PHP-PM has the advantage of offering both a ReactPHP-compatible environment and an Amp-compatible environment. Amp is capable of running any code written for React via a simple loop adaptor.

[mvhirsch]

This change forces to use php 7.0 or higher. composer.json only requires php 5.6 or higher.
This results in a BC-break.

[kelunik]

There are anyway no releases, yet. There are bigger breaks. And increasing the required PHP version isn't a BC break.

[trowski]

I'm certain that not changing composer.json to require 7.0 instead of 5.6 was simply an oversight and will be fixed if this PR is merged. Composer will not pull versions that increase the PHP version requirement if at least that version of PHP is not installed.

[mvhirsch]

The null coalescing operator is only avaiable since php 7.0 or higher. composer.json allows php 5.6 or higher. This results in a BC break.

[kelunik]

7.0 is anyway required by this PR.

[mvhirsch]

Please be aware, that this PR has syntactic changes which requires PHP 7.0 or higher!
I've noted two warnings about that in the diff (Null Coalescing Operator && Return Type Hinting).

Since composer.json currently allows PHP 5.6 or higher, this MR will break user's projects if they depend on this! Please keep in mind, that using new versions of PHP is good, but not by breaking backwards compatibility and leaving users behind (not knowing that they need to upgrade to PHP 7; or even worse: by forcing them to upgrade to PHP 7).

Please provide an upgrade-path (changelog, documentation, ...), so users of this package can still use PHP-PM on ~5.6.0 and smoothly upgrade to PHP 7.

[dzubchik]

this MR will break user's projects if they depend on this

No, will not. Because if you use composer correctly, have composer.lock in the repository it will install the latest working version of php-pm that does not have new requirements.

[mvhirsch]

No, will not. Because if you use composer correctly, have composer.lock in the repository it will install the latest working version of php-pm that does not have new requirements.

Sure? Requiring php-pm and updating it afterwards via composer on PHP 5.6.32 this won't work in my opinion. Anyway, just wanted to hint on this, before it gets merged. :-)

[dzubchik]

@mvhirsch , unless you run composer update without --ignore-platform-reqs, it will not upgrade package to unsupported PHP version (to PHP 7, if you use PHP 5.6).

[andig]

Please be assured that we're aware of the fact that Amp requires PHP7 while as of now php-pm still supports PHP 5.6. Since this PR has evolved into a roadmap discussion I'd like to make the following proposal for proceeding:

1. Upgrade ppm to stable react releases (i.e. Upgrade React/Http to v0.8 #271). This work is ready for testing but currently lacking any feedback. At this stage development is stuck.
2. Release a ppm 1.0 version with PHP 5.6 support on this basis.
3. Upgrade master to PHP7.
4. Re-assess Amp vs. React. At this stage I don't see any advantage in one or the other. The painpoints with ppm are imho more in the complexity and lack of tests, less in the underlying library once react has been upgraded to stable. If one library provides better base features and can help reduce code for worker management, scheduling etc it might be the more suitable candidate.

[marcj]

The biggest pain point and showstopper for most people is missing file upload and multipart parsing support. Without that, it is basically useless if you have a web-service that allows users to upload things. We need that in React or we can't rely on React. Also I haven't read yet a thing about that support anywhere. Do you know the progress @andig? I'd go ReactPHP when we get that support. Any ideas @clue?

[andig]

The biggest pain point and showstopper for most people is missing file upload and multipart parsing support.

React already has file upload and multipart in dev-master afaik. It's also implemented as part of #271.

See https://github.com/reactphp/http/blob/master/src/Io/MultipartParser.php and https://github.com/reactphp/http/blob/master/src/Io/UploadedFile.php.

[colinmollenhour]

Regarding users depending on file upload, perhaps this could be worked around using nginx locations that proxy to fcgi for example.

server {
    root /app;
    server_name example.com;

    # Uploads are not yet supported
    location = /my/app/uploader/ {
        fastcgi_pass 127.0.0.1:9000;
        include fastcgi_params;
    }

    # All other requests can go to PPM
    location / {
        try_files $uri @ppm;
    }
    location @ppm {
        proxy_set_header Host $http_host;
        proxy_set_header X-Real-IP $remote_addr;
        proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
        proxy_set_header X-Forwarded-Proto $scheme;
        proxy_pass http://127.0.0.1:8080;
    }
}

[clue]

Small progress upgrade: Conference time is over for me this year and I've picked up working on the outstanding PRs again. We've released a number of feature releases in the last weeks and are currently working on stabilizing the HTTP component. As far as I can tell everything that is needed for PHP-PM should already be in

@marcj The biggest pain point and showstopper for most people is missing file upload and multipart parsing support.

This feature has already been implemented a few weeks back, have you seen https://github.com/reactphp/http#requestbodyparsermiddleware yet?

I'm unsure what the current status of this ticket is. I would like to suggest continuing with @andig's excellent PR #271 which updates to latest ReactPHP components. This should be a solid base to then look into evaluating multipart parsing / file uploads 👍

If you feel anything's missing from ReactPHP's side of things, please let me know and I'm happy to look into this.

[marcj]

@clue @andig ok awesome, that was actually the only points that would have made Aerys lucrative.

By all hard work from @kelunik, we really appreciate your effort in building such a huge PR, but for the time being I'm closing this PR in favour of #271. Thanks anyways again! 🍺❤️

Why?

Well, we started with ReactPHP and we think that ReactPHP is a great library (beside its lack of development in the last 1 year, but that got fixed obviously). The reasons we wanted to go away from ReactPHP are mainly the lack of a tagged version with HTTP support, concretely multipart handling. That seems now to be fixed, which let us now provide a PHP-PM version that has everything necessary to let it run for production system. I had it running for aetros.com which has ~15mio php request/month, and it worked great, but had to remove it duo to multiparse requests.

Aerys would have been a great replacement for ReactPHP if latter wouldn't have increased its dev speed lately and introduced the missing features. Having said that, I liked the features of Aerys but they mainly conflicts with PHP-PM as the features Aerys is providing is either from the same use-case as PHP-PM's or is not necessary by PHP-PM. The small part of the actual HTTP handling is not worth bringing a huge library like Aerys under the hood of PHP-PM.

Also, Aerys does not bring any features on the table we're currently missing (anymore). It's actually vice-versa, Aerys brings features we don't need, which makes it very bloated for our use-case. A very well abstracted and in little parts split library like ReactPHP is here better. Also there're other reasons I'd definitely prefer ReactPHP, but I think that addressed ones are enough to justify to choose ReactPHP for now.

Thanks anyone for contributing!