-
Notifications
You must be signed in to change notification settings - Fork 0
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
Multiple clusters capability. The api interface has changed because all apis now need an atom identifying the connection. #1
base: master
Are you sure you want to change the base?
Conversation
056ea29
to
7a0fe49
Compare
benchmarks/benchmark.config
Outdated
{request_timeout, 20000}, | ||
{retry_policy, {default, true}} | ||
]} | ||
{clusters, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think will be much better to provide the config as follow:
{clusters, [
{erlcass_conn, [
{keyspace, <<"load_test_erlcass">>},
{contact_points, <<"127.0.0.1">>},
{contact_points, <<"127.0.0.1">>},
{latency_aware_routing, true},
{token_aware_routing, true},
{number_threads_io, 8},
{queue_size_io, 128000},
{core_connections_host, 5},
{max_connections_host, 5},
{tcp_nodelay, true},
{tcp_keepalive, {true, 60}},
{connect_timeout, 5000},
{request_timeout, 20000},
{retry_policy, {default, true}}
]}
]}
Basically:
- remove name and put it as tuple {name, OptionList}
- remove cluster_options and add all those on the same level with the keyspace. (Most probably keyspace should be removed when you send the option list to the NIF).
c_src/nif_cass_cluster.h
Outdated
|
||
struct enif_cass_cluster |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not putting this struct into the .cc file ? In case we need the CassCluster* outside the nif_cass_cluster.cc we can make a function that takes the ERL_NIF_TERM and returns the CassCluster
Another thing that I don't like and I think we can change is the fact that for every single query we are doing that get_existing_table_name which does concatenations and then from binary to atom. I think we can do an improvement here like:
I mean when you add a key just put a tuple of : What do you think ? Silviu |
@silviucpp I thought about this. If we merge both tables, the behavior is a bit weird when the erlcass genserver crashes. Right now init creates a new cluster and session and creates a prepared statement for every cached statement in erlcass_stm_cache. Since erlcass_stm_sessions is a child of the erlcass, that dies as well. A solution could be to remember the cluster and session in the new ets table and use it if it exists when init-ing erlcass. The only alternate solution i can think of is this binary to existing atom code. |
7a0fe49
to
bf56e37
Compare
…ll apis now need an atom identifying the connection.
bf56e37
to
464a98b
Compare
No description provided.