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

An exception occurs while trying to disable connection retries #83

Closed
rybakit opened this issue Apr 27, 2016 · 3 comments · Fixed by #166
Closed

An exception occurs while trying to disable connection retries #83

rybakit opened this issue Apr 27, 2016 · 3 comments · Fixed by #166
Assignees
Labels
bug Something isn't working

Comments

@rybakit
Copy link
Collaborator

rybakit commented Apr 27, 2016

$t = new Tarantool();

ini_set('tarantool.retry_count', 0);

$t->ping();

outputs

Uncaught exception 'Exception' with message '(null)'
@bigbes bigbes added the bug Something isn't working label May 3, 2016
@bigbes bigbes self-assigned this Jun 23, 2016
bigbes added a commit to bigbes/tarantool-php that referenced this issue Aug 14, 2018
bigbes added a commit to bigbes/tarantool-php that referenced this issue Dec 2, 2018
@Totktonada Totktonada added this to the PHP 7.* + bugfixes milestone Mar 23, 2020
MariaHajdic added a commit to bigbes/tarantool-php that referenced this issue Jun 16, 2020
MariaHajdic added a commit to bigbes/tarantool-php that referenced this issue Jun 16, 2020
MariaHajdic added a commit to bigbes/tarantool-php that referenced this issue Jun 19, 2020
Totktonada pushed a commit to bigbes/tarantool-php that referenced this issue Jul 14, 2020
Totktonada pushed a commit to bigbes/tarantool-php that referenced this issue Jul 14, 2020
This commit changes meanging of the `retry_count` option: now it means
amount of attempts to connect after the first one, not overall attempts
count.

Closes tarantool#83
Totktonada pushed a commit to bigbes/tarantool-php that referenced this issue Jul 14, 2020
This commit changes meanging of the `retry_count` option: now it means
amount of attempts to connect after the first one, not overall attempts
count.

Closes tarantool#83
@Totktonada
Copy link
Member

Now retry_count option means the following:

  • retry_count = 1 — perform only one connection attempt, fail if it is unsuccessful;
  • retry_count = N (N >= 2) — perform N attempts to connect and ignore transient errors (connection failures) for the first (N-1) attempts.

So retry_count = 1 is what you want to achieve with retry_count = 0.

retry_count = 0 has no sense and an error should make it clear.

I'm not sure whether we should change meaning of the option (I'm afraid of behaviour changes between versions). I'll update the discussion.

@rybakit
Copy link
Collaborator Author

rybakit commented Jul 14, 2020

retry_count = 0 has no sense and an error should make it clear.

I would agree with your points if the setting was called number_of_attemps or number_of_tries, but the re prefix already implies that something will be repeated. So it makes perfect sense to me to have retry_count = 0 and in the other thread I gave you examples of well-established APIs that follow this logic. Maybe you have a counterexample? To be honest, I've never seen anything like that before.

@Totktonada
Copy link
Member

I have updated the linked discussion.

Totktonada added a commit that referenced this issue Jul 14, 2020
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
Totktonada added a commit that referenced this issue Jul 15, 2020
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
Totktonada added a commit that referenced this issue Jul 17, 2020
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
4 participants