Skip to content

Reconnection strategy - round-robin between instances #106

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.

Sign up for GitHub

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

Jump to bottom
Closed
sharonovd opened this issue Aug 30, 2018 · 0 comments
Closed

Reconnection strategy - round-robin between instances #106

sharonovd opened this issue Aug 30, 2018 · 0 comments
Assignees
@kbelyavs

Comments

@sharonovd
Copy link

Use-case: tarantool is deployed as a geo-distributed cluster, and when one DC goes down, application must switch to reserve DC with minimal service disruption.

Proposal 1: add built-in reconnect strategy which would take several connection URI's as a config, and would quickly to establish connection to them in a circle, returning the first succesful one.
Edge-case: if all URIs are unavailable, return error.

Proposal 2: implement overridable connection strategy class (as in tarantool-java), and provide proposal 1 as example.
@kbelyavs kbelyavs self-assigned this Sep 13, 2018
kbelyavs pushed a commit that referenced this issue Sep 13, 2018
inconnect bug fix
18bf408 
Add missing inconnect = false.
This bug leads to only one disconnection allow.

Needed for #106
@kbelyavs kbelyavs mentioned this issue Sep 13, 2018
Merged
kbelyavs pushed a commit that referenced this issue Sep 13, 2018
Implement reconnection strategy class
f12521e 
Add built-in reconnect strategy class based on Round Robin.
@params:
  peers
  nattempts

Return next connection or error if all URIs are unavailable.

Closes #106
kbelyavs pushed a commit that referenced this issue Sep 14, 2018
Implement reconnection strategy class
9b6253d 
Add built-in reconnect strategy class based on Round-Robin alg.
@params:
 - peers
 - nattempts

Return next connection or an error if all URIs are unavailable.

Closes #106
kbelyavs pushed a commit that referenced this issue Sep 14, 2018
Implement reconnection strategy class
ff0f4af 
Extend base Connection class to support a list of nodes and an
optional Strategy parameter to choose next item from this list.

Add built-in reconnect strategy class based on Round-Robin alg.
@params:
 - peers

Return next connection or an error if all URIs are unavailable.

Closes #106
@kbelyavs kbelyavs mentioned this issue Sep 19, 2018
Merged
kbelyavs pushed a commit that referenced this issue Sep 25, 2018
Implement reconnection strategy class
fd992b0 
Extend base Connection class to support a list of nodes and an
optional Strategy parameter to choose next item from this list.

Add built-in reconnect strategy class based on Round-Robin alg.
@params:
 - addrs (a map of {host: HOSTNAME|IP_ADDRESS, port: PORT})

Return next connection or an error if all URIs are unavailable.

Closes #106
kbelyavs pushed a commit that referenced this issue Oct 1, 2018
Implement reconnection strategy class
b799675 
Extend base Connection class to support a list of nodes and an
optional Strategy parameter to choose next item from this list.

Add built-in reconnect strategy class based on Round-Robin alg.
@params:
 - addrs (a map of {host: HOSTNAME|IP_ADDRESS, port: PORT})

Return next connection or an error if all URIs are unavailable.

Closes #106
kbelyavs pushed a commit that referenced this issue Oct 3, 2018
Implement reconnection strategy class
495dec1 
Extend base Connection class to support a list of nodes and an
optional Strategy parameter to choose next item from this list.

Add built-in reconnect strategy class based on Round-Robin alg.
@params:
- addrs, list of maps {'host':(HOSTNAME|IP_ADDR), 'port':PORT}.

Return next connection or an error if all URIs are unavailable.

Closes #106
kbelyavs added a commit that referenced this issue Oct 16, 2018
@kbelyavs
Merge pull request #109 from tarantool/kbelyavs/gh-106
14f52b5 
Implement reconnection strategy class
@Totktonada Totktonada mentioned this issue Oct 16, 2018
Closed
Totktonada added a commit that referenced this issue Feb 16, 2021
@Totktonada
test: move basic mesh test under './setup.py test'
95c43d4 
In short: this commit moves the basic MeshConnection test cases to the
`./setup.py test` test suites (which are located in the unit/ now) and
removes the test-run submodule, which is not needed anymore.

A bit background: we have a testing framework called test-run, whose
primary goal is to give ability to manage tarantool instances from a
test. Historically tarantool-python does not use test-run for testing
(it uses several simple python helpers instead), but a test-run based
test was added in the scope of #106. The idea was to reuse test-run code
more and eventually port other tests to test-run.

The objective reality reveals several problems in the idea, which looked
nice in theory.

The main problem is the cyclic dependency between test-run and
tarantool-python submodules. It consumes an extra time at recursive git
clone and places old submodule revisions in a deeply nested level. The
latter may confuse linter tools, which search for files recursively (see
[1]).

Other problems look solvable, but I'll list them, because they give
considerable weight in my impression that we should get rid of the
test-run submodule within this repository:

1. test-run based tests were not run in CI and may break silently so
   (it already occurs once).
2. The first bullet looks easy to fix, but it is unclear whether it is
   right to depend on a submodule in the `./setup.py test` testing or we
   should keep only built-in and packaged testing tools in the
   dependencies.
3. Porting tests to test-run may require extra effort and nobody was
   eager to pay time for that.
4. Existing tooling for managing tarantool instances is enough for
   testing of the connector and, at the same time, it is quite simple.
   So if we'll meet a problem, it is easier to fix.
5. test-run supports only Python 2 at the moment (however it'll be fixed
   soon, see [2]).

To sum up, the experiment with the test-run submodule looks unsuccessful
and I think we should stop it for now. If we'll decide to try again, we
should consider all described problems and implement everything in a way
that does not hurt us.

[1]: tarantool/test-run#266 (comment)
[2]: tarantool/test-run#20

Fixes #111
Totktonada added a commit that referenced this issue Feb 16, 2021
@Totktonada
test: move unit/ to test/
1676e42 
This virtually reverts commit 436218d
('move unit tests to unit'), that was made in the scope of #106.

Despite the fact that testing of the connector uses `unittest`
framework, it is functional (and integration) testing by its nature:
most of the test cases verify that public API of the connector properly
works with tarantool.

In seems meaningful to locate such kind of test cases in the `test/`
directory, not `unit/`, disregarding of used framework.

Follows up #106.
Totktonada added a commit that referenced this issue Feb 18, 2021
@Totktonada
test: move basic mesh test under './setup.py test'
dccf18f 
In short: this commit moves the basic MeshConnection test cases to the
`./setup.py test` test suites (which are located in the unit/ now) and
removes the test-run submodule, which is not needed anymore.

A bit background: we have a testing framework called test-run, whose
primary goal is to give ability to manage tarantool instances from a
test. Historically tarantool-python does not use test-run for testing
(it uses several simple python helpers instead), but a test-run based
test was added in the scope of #106. The idea was to reuse test-run code
more and eventually port other tests to test-run.

The objective reality reveals several problems in the idea, which looked
nice in theory.

The main problem is the cyclic dependency between test-run and
tarantool-python submodules. It consumes an extra time at recursive git
clone and places old submodule revisions in a deeply nested level. The
latter may confuse linter tools, which search for files recursively (see
[1]).

Other problems look solvable, but I'll list them, because they give
considerable weight in my impression that we should get rid of the
test-run submodule within this repository:

1. test-run based tests were not run in CI and may break silently so
   (it already occurs once).
2. The first bullet looks easy to fix, but it is unclear whether it is
   right to depend on a submodule in the `./setup.py test` testing or we
   should keep only built-in and packaged testing tools in the
   dependencies.
3. Porting tests to test-run may require extra effort and nobody was
   eager to pay time for that.
4. Existing tooling for managing tarantool instances is enough for
   testing of the connector and, at the same time, it is quite simple.
   So if we'll meet a problem, it is easier to fix.
5. test-run supports only Python 2 at the moment (however it'll be fixed
   soon, see [2]).

To sum up, the experiment with the test-run submodule looks unsuccessful
and I think we should stop it for now. If we'll decide to try again, we
should consider all described problems and implement everything in a way
that does not hurt us.

[1]: tarantool/test-run#266 (comment)
[2]: tarantool/test-run#20

Fixes #111
Totktonada added a commit that referenced this issue Feb 18, 2021
@Totktonada
test: move unit/ to test/
ea942f9 
This virtually reverts commit 436218d
('move unit tests to unit'), that was made in the scope of #106.

Despite the fact that testing of the connector uses `unittest`
framework, it is functional (and integration) testing by its nature:
most of the test cases verify that public API of the connector properly
works with tarantool.

In seems meaningful to locate such kind of test cases in the `test/`
directory, not `unit/`, disregarding of used framework.

Follows up #106.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kbelyavs kbelyavs

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@sharonovd @kbelyavs