Skip to content

Conversation

@dasch
Copy link
Contributor

@dasch dasch commented Nov 17, 2017

Users shouldn't get IOError when we close down the client.

Fixes #484.

@dasch
Copy link
Contributor Author

dasch commented Nov 17, 2017

@klippx can you see if this fixes your problem?

@klippx
Copy link
Contributor

klippx commented Nov 18, 2017

This fix takes care of one problem but creates a new problem of different nature (a barrage of tests are still failing but for timeout reasons).

The new problems stem from that there are a number of things in the consumer loop that can still happen concurrently, for instance join_group: There is some code here which ends up with Timeout errors, which in turn are recast to ConnectionError, and this will retry for a long long time. This is just one example of many things that I have seen can get "stuck" in the consumer_loop which has to do with disconnecting the cluster and retrying.

I guess you could be interested in seeing some of these long running retries if you want the consumer to stop immediately. Perhaps some line of code to not retry if consumer is stopping and/or cluster has been explicitly disconnected.

In the end, this means that the events we wait for suddenly takes a very long time to resolve, and we have to increase our timeouts waiting for the consumer loop to give up from 10sec to 30sec. In the end the test suite goes from 30seconds to 3-4minutes (on my local machine), and we unfortunately have a lot of flakiness in terms of failing specs:

https://travis-ci.org/klarna/phobos/builds/303907111

But since that still doesn't solve the problem, I am thinking about stubbing in spec helper:

  config.before(:each) do
    allow_any_instance_of(Kafka::Cluster).to receive(:disconnect)
  end

Build: https://travis-ci.org/klarna/phobos/builds/303908659

Opting for this solution though, I might of course as well use the master branch 🤔

@dasch dasch merged commit 84986d0 into master Nov 20, 2017
@dasch dasch deleted the dasch/dont-raise-ioerror branch November 20, 2017 09:38
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.

Phobos issues in 0.5.x of Ruby Kafka

3 participants