Skip to content

Shard aware support - #6

Merged
fruch merged 18 commits into
scylladb:masterfrom
fruch:shared_aware
Jun 4, 2020
Merged

Shard aware support#6
fruch merged 18 commits into
scylladb:masterfrom
fruch:shared_aware

Conversation

@fruch

@fruchfruch commented Mar 11, 2020

Copy link
Copy Markdown

No description provided.

@fruchfruch self-assigned this Mar 12, 2020
vladzcloudius
vladzcloudius suggested changes Mar 12, 2020
View reviewed changes
Comment threadtests/integration/__init__.py Outdated
Comment threadtests/integration/__init__.py Outdated
vladzcloudius
vladzcloudius suggested changes Mar 12, 2020
View reviewed changes
Comment threadcassandra/pool.py Outdated
Comment threadcassandra/pool.py Outdated
Comment threadcassandra/pool.py
Comment threadcassandra/pool.py
@vladzcloudius

vladzcloudius commented Mar 12, 2020

Copy link
Copy Markdown

Please, don't squash commits - you can always do it later.
Please, put proper description what each next patch is about.

vladzcloudius
vladzcloudius reviewed Mar 16, 2020
View reviewed changes
Comment threadtests/integration/__init__.py Outdated
@fruchfruch mentioned this pull request Mar 30, 2020
Open
@ultrabug

ultrabug commented Apr 8, 2020

Copy link
Copy Markdown

Hi guys, as discussed on Slack I've pushed the changes necessary to a more graceful shard aware connection pooling ;)

@fruch

fruch commented Apr 13, 2020

Copy link
Copy Markdown
Author

@vladzcloudius care to take a look at the test in 5b636fc.

I guess you have some more ideas, on how/what should be test in that regards

@vladzcloudius

vladzcloudius commented Apr 14, 2020

Copy link
Copy Markdown

@fruch I'd like to see how driver behaves when you tear connections down: send RST from the remote side.

dimaqq
dimaqq reviewed Jun 2, 2020
View reviewed changes
Comment threadcassandra/connection.py
dimaqq
dimaqq reviewed Jun 2, 2020
View reviewed changes
Comment threadcassandra/connection.py Outdated
@dimaqqdimaqq mentioned this pull request Jun 2, 2020
Closed
dimaqq
dimaqq reviewed Jun 2, 2020
View reviewed changes
Comment threadcassandra/connection.py
Comment threadcassandra/connection.py
dimaqq
dimaqq reviewed Jun 2, 2020
View reviewed changes
Comment threadtests/integration/standard/test_shard_aware.py Outdated
dimaqq
dimaqq reviewed Jun 2, 2020
View reviewed changes
Comment threadtests/integration/standard/test_shard_aware.py Outdated
dimaqq
dimaqq reviewed Jun 2, 2020
View reviewed changes
Comment threadtests/integration/standard/test_connection.py Outdated
dimaqq
dimaqq reviewed Jun 2, 2020
View reviewed changes
Comment threadtests/integration/standard/test_connection.py Outdated
fruch added 3 commits June 2, 2020 13:33
@fruch
Initial shard aware driver
497cb04
@fruch
Add unit/integration tests for shard aware
af09a53
* unit test for parsing and shard_id calculation
* integration test using trace to verify requests going to the correct shard
@fruch
Add a lock on remaining_callbacks in _set_keyspace_for_all_conns()
697ae4e
since the async callback, to be on the safe side, adding a lock to
the `set.remove()` call
fruchand others added 15 commits June 2, 2020 13:33
@fruch
Making sure we close connections before removing them from shard mapping
c527bbd
@fruch
shard aware: fix _replace to reopen connection for same shard_id
23b8036
refactor the logic of re-opening connection to all shards into it's own function
so we can reuse it in `_replace()`
@fruch
shard aware: make debug print more friendler
f0cede0
base on @ultrabug comment, changing the debug print to be
more clear and informative
@fruch
shard aware: rename shard_id() to shard_id_from_token()
494db7f
based on @ultrabug comments, this should be named better
@fruch
shard aware: change dict initailizing to use built-in keywords
782a1a3
base on @ultrabug comments, this would be faster.
@fruch
shard aware: better document _open_connections_for_all_shards()
dbfdd20
Better explian why do we loop at max twice over the shards in this function.
@ultrabug@fruch
connection: log connection sharding information to debug level
cd0d3a3
@ultrabug@fruch
pool: refactor how connections to shards are created and maintained
ca844f4
Protocol does not (yet?) allow clients to specify which shard they
want to connect to and thus depend on a round-robin allocation of
the shard_id made by the host node (see system.clients table).
This means that on a live cluster where client connections come
and go, we cannot guarantee that we will get a connection to every
shard... even by retrying twice (which slows down the connection
startup also)
This commit switches to use an optimistic approach where we try to
connect as many times as there are shards on the remote cluster at
first.
Then when routing_key is used and shard aware connection picking
can take place, we will try to open missing connections as we
detect them. This is more graceful and allows us to not fail if
we miss shard connections as well as reduce the connection
startup time!
A long running client will hopefully get a connection to all
shards after a while!
That's the best we can do for now until the protocol evolves.
@ultrabug@fruch
pool: remove shard awareness initial cluster connection latency
31495d2
Initial connection tentatives to shards are now scheduled instead
of being blocking on startup.
This allows the shard aware driver to connect to a Scylla cluster
as fast as Cassandra one!
@fruch
shard aware: add tests for shared aware logic
d55f9dc
* add test for multiple client at the same time
* add test for closing connections
* add test for blocking(timing out) connections
@ultrabug@fruch
pool: implement logic to avoid over scheduling of shard connections
c3732c1
On busy systems we could overwhelm the threadpool executor queue
with repeated submissions of speculative shard connections to the
same shard which results in having an unbound number of connection
openings to scylla nodes
This could be seen as a "connection leak" and was also not
respecting the signal of cluster connections shutting down
@ultrabug@fruch
pool: switch connecting tracking from list to set
acc63c1
@ultrabug@fruch
pool: remove useless test when using set.discard()
6e67307
@ultrabug@fruch
pool: simplify _replace logic
a024655
@ultrabug@fruch
connection: use builtin cassandra.murmur3 INT64_MIN
2075978
@fruch

fruch commented Jun 2, 2020

Copy link
Copy Markdown
Author

@ultrabug FYI, I've rebase and force pushed this branch (not very social of me.. but I want travis to start working ontop of this PR too)

@fruch
fruch merged commit a49555b into scylladb:masterJun 4, 2020
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bentsibentsiAwaiting requested review from bentsi
3 more reviewers
@ultrabugultrabugultrabug left review comments
@dimaqqdimaqqdimaqq left review comments
@vladzcloudiusvladzcloudiusvladzcloudius approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@fruchfruch

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@fruch@vladzcloudius@ultrabug@dimaqq