arangodb / arangodb-php

PHP ODM for ArangoDB
https://www.arangodb.com
Apache License 2.0
182 stars 46 forks source link

^2.8 timeout does not timeout #231

Closed hlgrrnhrdt closed 6 years ago

hlgrrnhrdt commented 7 years ago

Hi,

we did a load test on our website and noticed, that the timeout we set did not apply to the requests during the load test.

The timeout is set to 10s, but it is exceeding it in HttpHelper::transfer() and fails at 100s.

Any Idea, why this is happening?

jsteemann commented 7 years ago

I think this is possible if each individual request only takes less than the configured timeout. So when the timeout is set to 10s, and each individual request takes less than 10s, it should be ok, even if there are many requests done for the same connection.

When I gave it a quick test, it seems that the timeout is honored. I set it to 10s in the following script:

<?php

namespace ArangoDBClient;

require __DIR__ . DIRECTORY_SEPARATOR . 'autoload.php';

$connectionOptions =array(
ConnectionOptions::OPTION_ENDPOINT => 'tcp://127.0.0.1:8529',
ConnectionOptions::OPTION_AUTH_TYPE => 'Basic',
ConnectionOptions::OPTION_AUTH_USER => 'root',
ConnectionOptions::OPTION_AUTH_PASSWD => '',
ConnectionOptions::OPTION_CONNECTION => 'Keep-Alive',
ConnectionOptions::OPTION_TIMEOUT => 10,
ConnectionOptions::OPTION_RECONNECT => true,
);

$connection = new Connection($connectionOptions);
$statement = new Statement($connection, array("query" => "RETURN SLEEP(20)"));
$cursor = $statement->execute();
$x = $cursor->getAll();
var_dump($x);

When I then run this script with time php script.php, it produces:

time php script.php 
PHP Fatal error:  Uncaught ArangoDBClient\ClientException: Got a timeout while waiting for the server's response
  thrown in ./ArangoDB-PHP/lib/ArangoDBClient/Connection.php on line 470

real    0m10,070s
user    0m0,016s
sys 0m0,004s
jsteemann commented 6 years ago

Do you think this issue is still relevant, or can we close it by now? @hlgrrnhrdt

hlgrrnhrdt commented 6 years ago

Can be closed

jsteemann commented 6 years ago

Thanks for confirming!