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

Set CQL3 consistency per connection #64

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

outoftime
Copy link
Contributor

Not thread-safe as the assumption is you'll use one connection per thread.

Adapted from outoftime#3

@outoftime
Copy link
Contributor Author

@ottbot Finally sending this upstream... sorry to let it lie fallow!

@kreynolds
Copy link
Owner

Can we mutex the with_consistency call so that this becomes thread safe again?

@outoftime
Copy link
Contributor Author

My initial assumption was that connections would not be shared between threads, but realistically one would probably be using some sort of connection pooling which makes this particular approach pretty bad.

How about as an alternative, #execute_cql_query takes an options hash as its second argument, which allows :compression and :consistency? We could just raise an ArgumentError if consistency were specified for a CQL2 connection. And it should be easy enough to check if the second argument is just a consistency argument directly for backwards compatibility.

If this seems reasonable I'd be glad to code it up and update this PR.

@kreynolds
Copy link
Owner

I think that makes a lot more sense. Its a breaking change but we can just bump the minor version and if you could slightly alter the README to let people know about the new API, that'd be fantastic.

@outoftime
Copy link
Contributor Author

Great! Although I don't think it has to be a breaking change if we just check the type of the second argument?

@kreynolds
Copy link
Owner

I suppose, but I prefer to minimize the overhead per query as much as possible and adding hash munging to it already makes me unhappy, though thats just how ruby rolls. We could write multiple methods and have RUBY_VERSION >= 2 use named keywords :)

@outoftime
Copy link
Contributor Author

OK, no strong objection here if you're down for a breaking change – do you think it's worth providing an alternate implementation for Ruby 2?

@kreynolds
Copy link
Owner

I know that hash setup/teardown is pretty gross for GC/CPU in ruby, but I've not specifically benchmarked the difference between using keyword arguments and a hash ... I don't know what ruby does under the hood for that. For now don't bother, maybe just add a TODO note or something.

@outoftime
Copy link
Contributor Author

Will do!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants