Replace gem thread with concurrent-ruby

[jcmfernandes]

concurrent-ruby is today the reference gem when it comes to concurrent data
structures and concurrency primitives.

Nothing against gem thread, is just that concurrent-ruby is simply much better
documented and actively maintained.

[jondot]

Sounds reasonable. Did you test this at scale?

[jcmfernandes]

No, I did not @jondot. With that said, the setup and tear-down of the pool seems correct to me, and the gem is hugely battle tested. I fully understand that this swaps a critical part of sneakers.

Do you want me to run and post results of the benchmark examples available in the repository?

[michaelklishin]

@jcmfernandes thank you. Can you please rebase this against master so that we can consider it?

[jcmfernandes]

You're welcome @michaelklishin. As requested, I rebased my commit, but seems like we have a flaky test in the CI pipeline. I run the suite locally against the current master and got the same test to fail.

What are we really trying to accomplish with the 'should be possible to terminate when queue is full' spec?

[michaelklishin]

@jcmfernandes looks like that a consumer (worker) that is working off a queue with N messages can be stopped with an OS signal. I suspect that 2 seconds isn't enough for Travis builds. We will bump the limit separately. Let me try re-running the build a few times.

[michaelklishin]

@jondot @gabrieljoelc I think we should accept this PR. I see concurrent-ruby used a fair amount (and I don't really spend much time in Ruby land these days). The changes look reasonable and concurrent-ruby uses more established terminology, at least for someone familiar with java.util.concurrent, which makes understanding the code easier.

[gabrieljoelc]

@michaelklishin i'm cool with accepting it, but should we ensure it works with the build?

[michaelklishin]

@gabrieljoelc there is one failing test that doesn't seem related. One can see it failing in master after a Bunny bump and I can't think of any recent Bunny change that can potentially affect Sneakers shutdown. So I'd merge and investigate the integration tests from there.

[gabrieljoelc]

I created #299 for the failing master build test.

[jcmfernandes]

🎉

[aq1018]

Just want to say, concurrent-ruby did make a difference for me. We had worker threads hanging on 2.5.0, and it took me nearly a week to figure out why:

• thread pool from thread gem will hang if a thread encounters uncaught exceptions, which can occur here.
• Sneakers::Handlers::Maxretry was doing #to_json on exceptions. See here
• Some exceptions contains self-referencing instance variables. When calling #to_json on those will cause SystemStackError: stack level too deep.

I just identified the root cause today, and was about to file a bug report here, but I noticed 2.6.0 is released and confirmed that my issues is gone after the upgrade. I believe concurrent-ruby handles the thread pool properly so that threads continue to work even if it encountered an exception.

Good work guys!

[michaelklishin]

@aq1018 you have to thank @jcmfernandes for that ;)

[jcmfernandes]

Glad to hear that @aq1018! 😉