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

retry preparing statements when there is a cassandra error #330

Merged
merged 2 commits into from Oct 12, 2016

Conversation

perobertson
Copy link
Contributor

I started discovering new Cassandra Errors being propagated up when updating to v2 of this gem. I traced it down to the fact that preparing the statement can raise most of the same errors as the execution. I also thought it was appropriate to switch to catching the errors specified in the docs.

prepare docs
execute docs

.to receive(:execute)
.with(->(s){ s.cql == statement},
hash_including(:consistency => cequel.default_consistency))
.and_raise(Cassandra::Errors::NoHostsAvailable)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was actually supposed to raise Cassandra::Errors::ExecutionError

context "with a connection error" do
it "reconnects to cassandra with a new client after no hosts could be reached" do
allow(cequel.client)
.to receive(:prepare_statement)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the expected receives were supposed to be just prepare

@pezra
Copy link
Contributor

pezra commented Oct 10, 2016

It makes completel sense that prepare would fail that way. Thanks for the improvement.

@perobertson
Copy link
Contributor Author

looks like a couple fixes didnt get made before my battery died. I can make those changes later tonight when im back online.

sleep(retry_delay)
retry
end
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we extract this and the execute retry logic into a retry method that takes a block?

@perobertson
Copy link
Contributor Author

dried up the retry code and fixed up the tests with the correct exceptions

@pezra pezra merged commit 7dc924e into cequel:master Oct 12, 2016
@pezra
Copy link
Contributor

pezra commented Oct 12, 2016

Thanks for the improvement!

@perobertson
Copy link
Contributor Author

Anytime! The next thing Ill probably look into is making this gem thread safe as it would be nice to use sidekiq.

@perobertson perobertson deleted the retry-prepare branch October 12, 2016 13:34
@pezra
Copy link
Contributor

pezra commented Oct 12, 2016

Thread safety would be awesome.

@pezra
Copy link
Contributor

pezra commented Oct 17, 2016

Released in 2.0.2

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.

None yet

2 participants