From fbfb006d6b8f25c3b9b8e88f277e2b2323a7e1ba Mon Sep 17 00:00:00 2001 From: bigbes Date: Mon, 6 Feb 2017 18:40:02 +0300 Subject: [PATCH] Distinguish timeout from errors. Honor request timeout. Still wasn't beign able to fix gh-113 Closes gh-110 --- src/tarantool.c | 49 +++++++++++++++++++++++++------------ src/tarantool_network.c | 33 +++++++++++++++++-------- src/tarantool_network.h | 2 ++ src/tarantool_schema.c | 1 + test/AssertTest.php | 8 +++--- test/CreateTest.php | 2 ++ test/RandomTest.php | 4 +-- test/shared/phpunit.xml | 2 +- test/shared/tarantool-3.ini | 4 +-- 9 files changed, 71 insertions(+), 34 deletions(-) diff --git a/src/tarantool.c b/src/tarantool.c index c4e57da..4121018 100644 --- a/src/tarantool.c +++ b/src/tarantool.c @@ -111,16 +111,16 @@ PHP_INI_BEGIN() STD_PHP_INI_BOOLEAN("tarantool.connection_alias", "0", PHP_INI_SYSTEM, OnUpdateBool, connection_alias, zend_tarantool_globals, tarantool_globals) - STD_PHP_INI_ENTRY("tarantool.timeout", "10.0", PHP_INI_ALL, + STD_PHP_INI_ENTRY("tarantool.timeout", "3600.0", PHP_INI_ALL, OnUpdateReal, timeout, zend_tarantool_globals, tarantool_globals) - STD_PHP_INI_ENTRY("tarantool.request_timeout", "10.0", PHP_INI_ALL, + STD_PHP_INI_ENTRY("tarantool.request_timeout", "3600.0", PHP_INI_ALL, OnUpdateReal, request_timeout, zend_tarantool_globals, tarantool_globals) STD_PHP_INI_ENTRY("tarantool.retry_count", "1", PHP_INI_ALL, OnUpdateLong, retry_count, zend_tarantool_globals, tarantool_globals) - STD_PHP_INI_ENTRY("tarantool.retry_sleep", "0.1", PHP_INI_ALL, + STD_PHP_INI_ENTRY("tarantool.retry_sleep", "10", PHP_INI_ALL, OnUpdateReal, retry_sleep, zend_tarantool_globals, tarantool_globals) PHP_INI_END() @@ -223,9 +223,14 @@ static zend_string *pid_pzsgen(const char *host, int port, const char *login, inline static int tarantool_stream_read(tarantool_connection *obj, char *buf, size_t size) { size_t got = tntll_stream_read2(obj->stream, buf, size); + const char *suffix = ""; + if (got == 0 && tntll_stream_is_timedout()) + suffix = " (request timeout reached)"; + char errno_suffix[256] = {0}; if (got != size) { + tarantool_throw_ioexception("Failed to read %ld bytes %s", + size, suffix); tarantool_stream_close(obj); - tarantool_throw_ioexception("Failed to read %ld bytes", size); return FAILURE; } return SUCCESS; @@ -233,9 +238,8 @@ tarantool_stream_read(tarantool_connection *obj, char *buf, size_t size) { static void tarantool_stream_close(tarantool_connection *obj) { - if (obj->stream || obj->persistent_id) { + if (obj->stream || obj->persistent_id) tntll_stream_close(obj->stream, obj->persistent_id); - } obj->stream = NULL; if (obj->persistent_id != NULL) { zend_string_release(obj->persistent_id); @@ -281,11 +285,9 @@ static int __tarantool_connect(tarantool_object *t_obj) { obj->suffix_len); } - if (tntll_stream_open(obj->host, obj->port, - obj->persistent_id, - &obj->stream, &err) == -1) { + if (tntll_stream_open(obj->host, obj->port, obj->persistent_id, + &obj->stream, &err) == -1) continue; - } if (tntll_stream_read2(obj->stream, obj->greeting, GREETING_SIZE) != GREETING_SIZE) { continue; @@ -299,6 +301,7 @@ static int __tarantool_connect(tarantool_object *t_obj) { } if (count == 0) { ioexception: + // raise (SIGABRT); tarantool_throw_ioexception("%s", err); efree(err); return FAILURE; @@ -401,9 +404,13 @@ static int tarantool_step_recv( zval *body) { char pack_len[5] = {0, 0, 0, 0, 0}; if (tarantool_stream_read(obj, pack_len, 5) == FAILURE) { - goto error; + header = NULL; + body = NULL; + goto error_con; } if (php_mp_check(pack_len, 5)) { + header = NULL; + body = NULL; tarantool_throw_parsingexception("package length"); goto error_con; } @@ -411,24 +418,34 @@ static int tarantool_step_recv( smart_string_ensure(obj->value, body_size); if (tarantool_stream_read(obj, SSTR_POS(obj->value), body_size) == FAILURE) { - goto error; + header = NULL; + body = NULL; + goto error_con; } SSTR_LEN(obj->value) += body_size; char *pos = SSTR_BEG(obj->value); if (php_mp_check(pos, body_size)) { + header = NULL; + body = NULL; tarantool_throw_parsingexception("package header"); goto error_con; } if (php_mp_unpack(header, &pos) == FAILURE || Z_TYPE_P(header) != IS_ARRAY) { + header = NULL; + body = NULL; goto error_con; } if (php_mp_check(pos, body_size)) { + header = NULL; + body = NULL; tarantool_throw_parsingexception("package body"); goto error_con; } if (php_mp_unpack(body, &pos) == FAILURE) { + header = NULL; + body = NULL; goto error_con; } @@ -466,7 +483,7 @@ static int tarantool_step_recv( "Bad error field type. Expected" " STRING, got %s", tutils_op_to_string(z_error_str)); - goto error; + goto error_con; } } else { error_str = "empty"; @@ -941,9 +958,9 @@ PHP_RINIT_FUNCTION(tarantool) { static void php_tarantool_init_globals(zend_tarantool_globals *tarantool_globals) { tarantool_globals->sync_counter = 0; tarantool_globals->retry_count = 1; - tarantool_globals->retry_sleep = 0.1; - tarantool_globals->timeout = 1.0; - tarantool_globals->request_timeout = 10.0; + tarantool_globals->retry_sleep = 10; + tarantool_globals->timeout = 3600.0; + tarantool_globals->request_timeout = 3600.0; } static void tarantool_destructor_connection(zend_resource *rsrc TSRMLS_DC) { diff --git a/src/tarantool_network.c b/src/tarantool_network.c index 9ad0460..077c3c5 100644 --- a/src/tarantool_network.c +++ b/src/tarantool_network.c @@ -2,6 +2,7 @@ #include #include #include +#include #include #include @@ -23,21 +24,23 @@ void double_to_ts(double tm, struct timespec *ts) { /* `pid` means persistent_id */ // void tntll_stream_close(php_stream *stream, const char *pid) { void tntll_stream_close(php_stream *stream, zend_string *pid) { - TSRMLS_FETCH(); int rv = PHP_STREAM_PERSISTENT_SUCCESS; - if (stream == NULL) + if (stream == NULL) { rv = tntll_stream_fpid2(pid, &stream); - int flags = PHP_STREAM_FREE_CLOSE; - if (pid) - flags = PHP_STREAM_FREE_CLOSE_PERSISTENT; - if (rv == PHP_STREAM_PERSISTENT_SUCCESS && stream) { - php_stream_free(stream, flags); + } + if (rv == PHP_STREAM_PERSISTENT_SUCCESS && stream != NULL) { + if (pid) { + php_stream_free(stream, PHP_STREAM_FREE_CLOSE_PERSISTENT); + } else { + php_stream_close(stream); + } } } int tntll_stream_fpid2(zend_string *pid, php_stream **ostream) { TSRMLS_FETCH(); return php_stream_from_persistent_id(pid->val, ostream TSRMLS_CC); + return rv; } int tntll_stream_fpid(const char *host, int port, zend_string *pid, @@ -93,7 +96,7 @@ int tntll_stream_open(const char *host, int port, zend_string *pid, // printf("request_timeout: 'sec(%d), usec(%d)'\n", // (int )tv.tv_sec, (int )tv.tv_usec); - if (tv.tv_sec != 0 && tv.tv_usec != 0) { + if (tv.tv_sec != 0 || tv.tv_usec != 0) { php_stream_set_option(stream, PHP_STREAM_OPTION_READ_TIMEOUT, 0, &tv); } @@ -111,10 +114,12 @@ int tntll_stream_open(const char *host, int port, zend_string *pid, return 0; error: if (errstr) zend_string_release(errstr); - if (stream) tntll_stream_close(stream, pid); + if (stream) tntll_stream_close(NULL, pid); return -1; } +#include + /* * Legacy rtsisyk code, php_stream_read made right * See https://bugs.launchpad.net/tarantool/+bug/1182474 @@ -122,7 +127,7 @@ int tntll_stream_open(const char *host, int port, zend_string *pid, size_t tntll_stream_read2(php_stream *stream, char *buf, size_t size) { TSRMLS_FETCH(); size_t total_size = 0; - size_t read_size = 0; + ssize_t read_size = 0; while (total_size < size) { read_size = php_stream_read(stream, buf + total_size, @@ -132,9 +137,17 @@ size_t tntll_stream_read2(php_stream *stream, char *buf, size_t size) { break; total_size += read_size; } + return total_size; } +bool tntll_stream_is_timedout() { + int err = php_socket_errno(); + if (err == EAGAIN || err == EWOULDBLOCK || err == EINPROGRESS) + return 1; + return 0; +} + int tntll_stream_read(php_stream *stream, char *buf, size_t size) { TSRMLS_FETCH(); return php_stream_read(stream, buf, size); diff --git a/src/tarantool_network.h b/src/tarantool_network.h index ba5a20a..8858424 100644 --- a/src/tarantool_network.h +++ b/src/tarantool_network.h @@ -30,4 +30,6 @@ int tntll_stream_read (php_stream *stream, char *buf, size_t size); size_t tntll_stream_read2(php_stream *stream, char *buf, size_t size); int tntll_stream_send (php_stream *stream, char *buf, size_t size); +bool tntll_stream_is_timedout(); + #endif /* PHP_TNT_NETWORK_H */ diff --git a/src/tarantool_schema.c b/src/tarantool_schema.c index 54965db..842a4d5 100644 --- a/src/tarantool_schema.c +++ b/src/tarantool_schema.c @@ -4,6 +4,7 @@ #include #include #include +#include #include "php_tarantool.h" #include "tarantool_schema.h" diff --git a/test/AssertTest.php b/test/AssertTest.php index f70d123..6063866 100644 --- a/test/AssertTest.php +++ b/test/AssertTest.php @@ -22,18 +22,20 @@ protected function tearDown() { public function test_00_timedout() { self::$tarantool->eval(" - function assertf() - require('fiber').sleep(1) + function assert_f() + os.execute('sleep 1') return 0 end"); try { - self::$tarantool->call("assertf"); + $result = self::$tarantool->call("assert_f"); $this->assertFalse(True); } catch (TarantoolException $e) { + // print($e->getMessage()); $this->assertContains("Failed to read", $e->getMessage()); } /* We can reconnect and everything will be ok */ + self::$tarantool->close(); self::$tarantool->select("test"); } diff --git a/test/CreateTest.php b/test/CreateTest.php index 05efae0..2dff2f5 100644 --- a/test/CreateTest.php +++ b/test/CreateTest.php @@ -7,7 +7,9 @@ class CreateTest extends PHPUnit_Framework_TestCase public static function setUpBeforeClass() { self::$port = getenv('PRIMARY_PORT'); self::$tm = ini_get("tarantool.timeout"); + // error_log("before setting tarantool timeout"); ini_set("tarantool.timeout", "0.1"); + // error_log("after setting tarantool timeout"); } public static function tearDownAfterClass() { diff --git a/test/RandomTest.php b/test/RandomTest.php index b687352..34d942a 100644 --- a/test/RandomTest.php +++ b/test/RandomTest.php @@ -28,7 +28,7 @@ public function test_01_random_big_response() { } public function test_02_very_big_response() { - $request = "do return '" . str_repeat('x', 1024 * 1024 * 10) . "' end"; + $request = "do return '" . str_repeat('x', 1024 * 1024 * 1) . "' end"; self::$tarantool->evaluate($request); $this->assertTrue(True); } @@ -40,6 +40,6 @@ public function test_03_another_big_response() { } } public function test_04_get_strange_response() { - print_r(self::$tarantool->select("_schema", "12345")); + self::$tarantool->select("_schema", "12345"); } } diff --git a/test/shared/phpunit.xml b/test/shared/phpunit.xml index a4fbf47..c1deabc 120000 --- a/test/shared/phpunit.xml +++ b/test/shared/phpunit.xml @@ -1 +1 @@ -phpunit_tap.xml \ No newline at end of file +phpunit_bas.xml \ No newline at end of file diff --git a/test/shared/tarantool-3.ini b/test/shared/tarantool-3.ini index 8fcbb53..e42de1a 100644 --- a/test/shared/tarantool-3.ini +++ b/test/shared/tarantool-3.ini @@ -3,10 +3,10 @@ extension = ./tarantool.so [tarantool] tarantool.timeout = 10 -tarantool.request_timeout = 0.1 +tarantool.request_timeout = 10 tarantool.con_per_host = 1 tarantool.persistent = 1 -tarantool.retry_count = 1 +tarantool.retry_count = 2 [xdebug] remote_autostart = 0