Skip to content

Commit

Permalink
Give meaningful error when retry_count is zero
Browse files Browse the repository at this point in the history
Clarified docs about this setting.

There may be a confusion whether the setting means overall amount of
connection attempts or amount after the initial unsuccessful attempt.
Usually a retries amount option means the latter, but here it means the
former.

Decided to don't change behaviour or the setting name to provide perfect
backward compatibility, but give meaningful error and clarify docs. See
the discussion in [1].

[1]: #145 (comment)

Fixes #83
  • Loading branch information
Totktonada committed Jul 15, 2020
1 parent e67cb23 commit c68fdee
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 1 deletion.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ Place it into project library path in your IDE.

* `tarantool.persistent` - Enable persistent connections (don't close connections between sessions) (defaults: True, **can't be changed in runtime**)
* `tarantool.timeout` - Connection timeout (defaults: 10 seconds, can be changed in runtime)
* `tarantool.retry_count` - Count of retries for connecting (defaults: 1, can be changed in runtime)
* `tarantool.retry_count` - Amount of attempts to connect (defaults: 1, can be changed in runtime)
* `tarantool.retry_sleep` - Sleep between connecting retries (defaults: 0.1 second, can be changed in runtime)
* `tarantool.request_timeout` - Read/write timeout for requests (defaults: 10 second, can be changed in runtime)

Expand Down
5 changes: 5 additions & 0 deletions src/tarantool.c
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,11 @@ static int __tarantool_connect(tarantool_object *t_obj) {
double_to_ts(INI_FLT("retry_sleep"), &sleep_time);
char *err = NULL;

if (count < 1) {
THROW_EXC("tarantool.retry_count should be 1 or above");
return FAILURE;
}

if (t_obj->is_persistent) {
if (!obj->persistent_id)
obj->persistent_id = pid_pzsgen(obj->host, obj->port,
Expand Down
28 changes: 28 additions & 0 deletions test/CreateTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -163,5 +163,33 @@ public static function provideGoodCredentials()
['guest', null],
];
}

public function test_10_zero_retry_exception() {
/* A connection to call iproto_connect_count(). */
$tarantool = new Tarantool('localhost', self::$port, 'test', 'test');
$iproto_connect_count_before =
$tarantool->call('iproto_connect_count')[0][0];

$saved_retry_count = ini_get('tarantool.retry_count');
ini_set('tarantool.retry_count', 0);

$exp_err = 'tarantool.retry_count should be 1 or above';
try {
$c = new Tarantool('localhost', self::$port);
$c->connect();
$this->assertFalse(true);
} catch (Exception $e) {
$this->assertInstanceOf(TarantoolException::class, $e);
$this->assertEquals($exp_err, $e->getMessage());
} finally {
ini_set('tarantool.retry_count', $saved_retry_count);
}

/* Verify that no connection attempts were performed. */
$iproto_connect_count_after =
$tarantool->call('iproto_connect_count')[0][0];
$this->assertEquals($iproto_connect_count_before,
$iproto_connect_count_after);
}
}

17 changes: 17 additions & 0 deletions test/shared/box.lua
Original file line number Diff line number Diff line change
Expand Up @@ -180,5 +180,22 @@ function test_6(...)
return ...
end

iproto_connect_counter = 0
function iproto_connect_count()
return iproto_connect_counter
end

box.session.on_connect(function()
-- box.session.type() was introduced in 1.7.4-370-g0bce2472b.
--
-- We're interested in iproto sessions, but it is okay for our
-- usage scenario to count replication and console sessions
-- too: we only see to a delta and AFAIK our testing harness
-- does not perform any background reconnections.
if box.session.type == nil or box.session.type() == 'binary' then
iproto_connect_counter = iproto_connect_counter + 1
end
end)

require('console').listen(os.getenv('ADMIN_PORT'))

0 comments on commit c68fdee

Please sign in to comment.