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

Integration: Example with Slim app. #62

Closed
agentd00nut opened this issue Dec 18, 2018 · 12 comments

Comments

@agentd00nut
Copy link

commented Dec 18, 2018

This started as a request for help in understanding the worker error: invalid prefix (checksum) error but I finally managed to figure that it was caused by not running slim silently.

Because that took me so long to realize I thought it might be helpful to show others an example.

ini_set('display_errors', 'stderr');
include "vendor/autoload.php";


use \Psr\Http\Message\ServerRequestInterface as Request;
use \Psr\Http\Message\ResponseInterface as Response;

$relay = new Spiral\Goridge\StreamRelay(STDIN, STDOUT);
$psr7 = new Spiral\RoadRunner\PSR7Client(new Spiral\RoadRunner\Worker($relay));

while ($req = $psr7->acceptRequest()) {
    try {

        // You must re-create the app on each request otherwise the response body doesn't reset in between calls.
        // It's likely there is a more efficient way around this but is outside the scope of the example.
        $app = new \Slim\App;

        // We set the request for slim to the one received by RoadRunners Worker.
        $container = $app->getContainer();
        $container['request'] = $req;

        // Now we're ready to declare any middle ware, routes, etc. etc.
        // You'd likely do this in other files and include them here.
        $app->post('/hello/{name}', function (Request $request, Response $response, array $args) {
            $parsedBody = $request->getParsedBody();
            $name = $args['name'];
            $response->getBody()->write("Hello, $name<br>" . json_encode($parsedBody));
            return $response;
        });

        // you MUST run the app silently!  We capture the Response object and pass hand it to the PSR7Client for responding to the server.
        $resp = $app->run(true);
        $psr7->respond($resp);
    } catch (\Throwable $e) {
        $psr7->getWorker()->error((string)$e);
    }
}

If this should be a pull request i'll make one. Again it started as asking about the invalid prefix error.

In addition: Would it be possible to make the invalid prefix error slightly less ambiguous? Searching for invalid prefix in the git doesn't lead to anything useful and changing it to worker error: invalid prefix (checksum) (make sure you aren't echoing before the worker responds to the server) or some other more verbose message about what is going on could help people new to the project.

@wolfy-j wolfy-j self-assigned this Dec 19, 2018

@wolfy-j

This comment has been minimized.

Copy link
Member

commented Dec 19, 2018

I knew that this error would cause some misunderstanding. 99% it's about wrong data in the buffer, and 1% is when the connection is corrupted on OS level. I guess I can make it less generic, something like worker error: invalid data found in the buffer (possible echo).

Also, check https://github.com/spiral/roadrunner/wiki/PHP-Workers there is info about troubleshooting.

Regarding Slim, is this the approach which will work for all Slim apps? If so I'm happy to add it into the wiki.

@agentd00nut

This comment has been minimized.

Copy link
Author

commented Dec 19, 2018

I've used the provided example as a guide when integrating with more complicated Slim apps that involve a more in depth configuration for the slim container with many more routes defined etc. etc.

Also, I thought i had linked to this page but i must have taken it out by accident... PSR7 With Slim .... So according to them it's designed to plug and play with any PSR7 implementation, so since all we're doing in this example is changing the built in slim psr7 request with one from our worker, it should work!

Lastly, thanks for the PHP-Workers link. The issue was i, stupidly, forgot that $app->run() in slim, at the end, will echo out the response body! But the (possible echo) would have been a big hint in this situation.

@wolfy-j wolfy-j added the integration label Dec 19, 2018

@wolfy-j

This comment has been minimized.

Copy link
Member

commented Dec 20, 2018

@agentd00nut new error message has been added

@wolfy-j

This comment has been minimized.

Copy link
Member

commented Dec 20, 2018

Regarding the integration, I would still like an independent verification just to be sure that I can add it to the Wiki. :)

@wolfy-j wolfy-j removed their assignment Dec 20, 2018

@fenric

This comment has been minimized.

Copy link

commented Jan 10, 2019

You can watch awesome-skeleton-roadrunner for an example (this is not Slim but the principles are similar).

@n1215

This comment has been minimized.

Copy link

commented Jan 24, 2019

It's possible to reuse the same \Slip\App object by reseting Request and Response in the container.

https://github.com/n1215/roadrunner-docker-skeleton/blob/slimphp/worker.php

@wolfy-j

This comment has been minimized.

Copy link
Member

commented Jan 25, 2019

@fenric @n1215 both of your links has been added to the Wiki.

@wolfy-j wolfy-j closed this Feb 2, 2019

@stdex

This comment has been minimized.

Copy link

commented May 21, 2019

@wolfy-j, can you explain and help to improve solution / find bottleneck?
I have benchmark results (slim - php-fpm 7.3, nginx; roadrunner_slim - php-cli 7.3, rr 1.4)
Docker, worker: stdex/web-frameworks@043b275

| Language (Runtime) | Framework (Middleware) | Average | 50th percentile | 90th percentile | 99th percentile | 99.9th percentile | Standard deviation |
|---------------------------|---------------------------|----------------:|----------------:|----------------:|----------------:|----------------:|----------------:|
| php (7.3) | [slim](http://slimframework.com) (3.12) | 81.05 ms | 3.60 ms | 226.28 ms | 1302.86 ms | 2931.44 ms | 253229.67 | 
| php (7.3) | [roadrunner_slim](http://roadrunner.dev) (1.4.1) | 86.94 ms | 85.94 ms | 94.82 ms | 141.02 ms | 315.53 ms | 13787.67 | 
| Language (Runtime) | Framework (Middleware) | Requests / s | Throughput |
|---------------------------|---------------------------|----------------:|---------:|
| php (7.3) | [slim](http://slimframework.com) (3.12) | 66339.00 | 329.63 MB |
| php (7.3) | [roadrunner_slim](http://roadrunner.dev) (1.4.1) | 11279.33 | 15.12 MB |
@wolfy-j

This comment has been minimized.

Copy link
Member

commented May 21, 2019

Few things:

  1. Make sure to set number of rr workers equal to the number of logical CPU threads (current value is hardcoded to 4, you can remove it to let RR detect numCores automatically)
  2. Do not bench rr with -d flag, it enables debug mode and bottlenecks in stdout of docker.
  3. Synthetic bench with an empty application is not very informative, rr will speed up larger applications more (due to the bigger amount of libraries loaded to memory). Slim is very small and it does not create noticeable overhead.

My local bench results after the adjustments:

| Language (Runtime) | Framework (Middleware) | Average | 50th percentile | 90th percentile | 99th percentile | 99.9th percentile | Standard deviation |
|---------------------------|---------------------------|----------------:|----------------:|----------------:|----------------:|----------------:|----------------:|
| php (7.3) | [slim](http://slimframework.com) (3.12) | 197.30 ms | 1.53 ms | 350.02 ms | 4265.07 ms | 7491.62 ms | 724330.67 |
| php (7.3) | [roadrunner_slim](http://roadrunner.dev) (1.4.1) | 31.02 ms | 31.37 ms | 35.52 ms | 40.86 ms | 310.41 ms | 12043.33 |
| Language (Runtime) | Framework (Middleware) | Requests / s | Throughput |
|---------------------------|---------------------------|----------------:|---------:|
| php (7.3) | [slim](http://slimframework.com) (3.12) | 40219.67 | 200.05 MB |
| php (7.3) | [roadrunner_slim](http://roadrunner.dev) (1.4.1) | 29374.67 | 39.43 MB |

I'm not sure how to interpret these results, the rr shows much lower timings with much lower deviation, though the results in favor of nginx. The benchmark suite seems broken.

Once I tried to bench containers using ab the results are very different (16 threads):

ab -c 32 -n 10000 -k http://localhost:3001/

slim + nginx:

Concurrency Level:      32
Time taken for tests:   1.699 seconds
Complete requests:      10000
Failed requests:        0
Keep-Alive requests:    9904
Total transferred:      1859520 bytes
HTML transferred:       0 bytes
Requests per second:    5885.62 [#/sec] (mean)
Time per request:       5.437 [ms] (mean)
Time per request:       0.170 [ms] (mean, across all concurrent requests)
Transfer rate:          1068.79 [Kbytes/sec] received

slim + roadrunner:

Concurrency Level:      32
Time taken for tests:   0.488 seconds
Complete requests:      10000
Failed requests:        0
Keep-Alive requests:    10000
Total transferred:      990000 bytes
HTML transferred:       0 bytes
Requests per second:    20504.70 [#/sec] (mean)
Time per request:       1.561 [ms] (mean)
Time per request:       0.049 [ms] (mean, across all concurrent requests)
Transfer rate:          1982.39 [Kbytes/sec] received

Also same bench running wrk manually:

slim + nginx:

Running 10s test @ http://localhost:3001
  17 threads and 100 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    12.36ms    2.84ms  46.97ms   91.25%
    Req/Sec   408.70    113.99     4.71k    98.65%
  69228 requests in 10.10s, 12.28MB read
Requests/sec:   6855.18
Transfer/sec:      1.22MB

slim + roadrunner

Running 10s test @ http://localhost:3000
  17 threads and 100 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     3.68ms    2.58ms 101.10ms   97.45%
    Req/Sec     1.40k   228.22     9.67k    97.35%
  237679 requests in 10.10s, 17.00MB read
Requests/sec:  23539.61
Transfer/sec:      1.68MB
@wolfy-j

This comment has been minimized.

Copy link
Member

commented May 21, 2019

@waghanza

This comment has been minimized.

Copy link

commented May 31, 2019

@wolfy-j @stdex I didn't know about https://roadrunner.dev/, but docker used in benchmarks are breaking any results

I'm working on a PR the-benchmarker/web-frameworks#632 to have more reliable results

@wolfy-j

This comment has been minimized.

Copy link
Member

commented May 31, 2019

Yeah, something is really funky in the current state of this benchmark.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.