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

Server requestHandlers not concurrent, waits for response #334

Closed
woytam opened this issue Feb 20, 2019 · 8 comments
Closed

Server requestHandlers not concurrent, waits for response #334

woytam opened this issue Feb 20, 2019 · 8 comments

Comments

@woytam
Copy link

woytam commented Feb 20, 2019

I found out, that Server requestHandlers are executed simultaneously waiting for response for preceding call.
I have make use case, where requestHandler returns promise that is resolve in $loop Timer after 2 seconds. In this 2 seconds, server and other connections is waiting. When the response is sent (it can be response with stream), next request is being processed.
Use case:

$socket = new \React\Socket\Server(SERVER_LISTEN . ':' . SERVER_PORT, $loop);
$socket->on('connection', function (ConnectionInterface $connection) {
	echo $connection->getRemoteAddress() . "\tNew connection"));
});
$server = new \React\Http\Server(array(
  new \React\Http\Middleware\LimitConcurrentRequestsMiddleware(10),
  function($request) use ($loop) {
    $reqId = \Nette\Utils\Random::generate(2, 'A-Z');   // Identify request by random ID
    echo 'Request #' . $reqId . PHP_EOL;
    $resolver = new \React\Promise\Deferred();
    $loop->addTimer(2, function($request) use ($resolver, $reqId) {
      $stream = new \React\Stream\ThroughStream();
      echo 'Response #' . $reqId . PHP_EOL;
      $resolver->resolve( new \React\Http\Response(
          200,
          ['Content-Type' => 'text/event-stream; charset=utf-8'],
          $stream
        ));
    });
    return $resolver->promise();
  })
);

Trying 3 concurrent connections.
Expected possible output on STDOUT, running time about 2 seconds:

tcp://***:58924       New connection
tcp://***:58925       New connection
tcp://***:58940       New connection
Request #IX
Request #QC
Request #YI
Response #IX
Response #QC
Response #YI

The generated output. Make attention to order of request - response pair. Running time about 6 seconds.

tcp://***:58924       New connection
tcp://***:58925       New connection
tcp://***:58940       New connection
Request #IX
Response #IX
Request #QC
Response #QC
Request #YI
Response #YI
@WyriHaximus
Copy link
Member

What are you using to test this? There doesn't seem to be anything wrong with your code so I'm suspecting the issue might be on the requesting side. Also what are you using the empty ThroughStream for?

@woytam
Copy link
Author

woytam commented Feb 20, 2019

I'm using ApacheBench, Version 2.3 <$Revision: 1430300 $>

ab -c 3 -s 3600 -t 3600 -v 2 -H "Accept: text/event-stream" "http://***:*/"

The code is cutted from project, where ThroughStream is sending data on some events. Not in this use case.

@woytam
Copy link
Author

woytam commented Feb 20, 2019

Now, I have tested in two different browsers (updated Content-Type to text/html) and the result is the same.

Req1 -> waiting 2 seconds -> Res1, Req2 -> waiting 2 seconds -> Res2, Req3, ...

PHP v7.1.24 on CentOS 7, running in CLI

Edit
Composer.json of the project

"require": {
	"php": ">= 5.3.7",
	"nette/nette": "~2.4.0",
	"kdyby/console": "^2.7",
	"react/event-loop": "^1.0",
	"react/socket": "^1.0",
	"react/http": "^0.8.3",
	"react/mysql": "^0.5.0",
	"react/filesystem": "^0.1.2",
	"christoph-kluge/reactphp-http-response-compression-middleware": "dev-master"
},

@WyriHaximus
Copy link
Member

Can't seem to reproduce the issue you're seeing when using chrome and opening a few tabs in a row:

screenshot from 2019-02-20 17-30-12

@woytam
Copy link
Author

woytam commented Feb 20, 2019

Thanks @WyriHaximus for your support. I have tested it now on Windows and still no luck.
I have created gist with simple test case and composer.json for installation. For compatibility, I have used StreamSelectLoop.
But still getting serial processing (3 opened tabs in Firefox or Chrome):

[20:46:16.851] tcp://127.0.0.1:52061    New connection
[20:46:16.880] Request #c7ca
[20:46:17.414] tcp://127.0.0.1:52062    New connection
[20:46:17.938] tcp://127.0.0.1:52063    New connection
[20:46:19.882] Response #c7ca
[20:46:19.892] Request #32e2
[20:46:22.898] Response #32e2
[20:46:22.925] Request #9493
[20:46:25.930] Response #9493

As we can see, connections are in time parallel, but processing of request is waiting to resolving Response in Promise.

@woytam
Copy link
Author

woytam commented Feb 20, 2019

I figured out, when used StreamingServer instead of Server, it is working as expected - async.
So, there will be probably problem with LimitConcurrentRequestsMiddleware integrated in Server?
If I specifically define new \LimitConcurrentRequestsMiddleware(10) in requestHandler, it will not overwrite the integrated one.
In my case, problem is memory_limit set to 128M and post_max_size also 128M. So inside Server I get new \LimitConcurrentRequestsMiddleware(1).

@WyriHaximus
Copy link
Member

Glad to hear that you solved it. The Server is a convinence class that wraps StreamingServer and several middleware to emulate PHP's behavior. You can influence those with the ini settings you mention. My suggestion would be to figure out a sane configuration for those ini settings for your application. If you need to handle 128M file uploads you would need more then 1G of memory to handle 5 of those savely and concurrently. Dive into what is required for your application and adjust settings accordingly

@woytam
Copy link
Author

woytam commented Feb 20, 2019

Thank you very much for your very quick help. I was a little confused about using LimitConcurrentRequestsMiddleware.
I have made a PR #335 with a small note about LimitConcurrentRequestsMiddleware when using basic Server.
PS. My server needs tuning (php.ini) before 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

No branches or pull requests

2 participants