Support socketry/async

[dblock]

Looks like celluloid-io is no longer maintained. Support https://github.com/socketry/async, maybe make it default?

[ioquatix]

Just to assist with this, here is code for a slightly different slack gem: https://gist.github.com/ioquatix/c24f107e2cc7f48e571a37e8e93b0cda?ts=2

[dblock]

@ioquatix Is there a way to start a rector similar to EM? We do this, which avoids this. Not sure whether it's a good idea, welcome any comments.

[ioquatix]

Reactors isolate and block on nested tasks. It is possible to start one and manually handle it like you do with EM, but I suggest you just use async-rspec which does everything for you: https://github.com/socketry/async-rspec#reactor

This also checks for leaked IO, and gracefully times out the spec if you end up in a situation where your I/O operation would never complete.

Happy to discuss further.

[ioquatix]

Just gave you some feedback on the code. It's a great start and I look forward to seeing how it ends up. Thanks for your hard work implementing this.

[ioquatix]

I've been thinking about a good way forward for this gem.

One thing to consider is that you also want APIs like chat_postMessage to be async. For my hack around the other slack gem, I just made a wrapper for faraday which worked fine.

I suggest you continue with the next release supporting different models for concurrency. However, on your next major release (0.13), I suggest you go all in, removing EM, Celluloid, etc, and using async-http and async-websocket. You can get concurrency across all the APIs.

[dblock]

I opened #220.

There're many people with production celluloid and EM, and by forcing them to choose a newer library in a client like slack-ruby-client we're making life very difficult for a lot of people. This client has to keep up with the Slack API, and that either means lots of forks over the older version of everyone upgrades. There's likely a better without breaking everyone to support async http requests.

That said I am also aware that these other libraries are unmaintained and it's mostly busy work to move forward with all.

Finally, what happens when the next best async library pops up? :) Cause you know, ruby concurrency, EM, Celluloid, Async ... we've seen a few.

[dblock]

@ioquatix Thanks for the code review on dblock@6e07782#commitcomment-30295386! My integration tests are still failing, but the sample bot works, I'll get to it soon.

[ioquatix]

There're many people with production celluloid and EM, and by forcing them to choose a newer library in a client like slack-ruby-client we're making life very difficult for a lot of people. This client has to keep up with the Slack API, and that either means lots of forks over the older version of everyone upgrades. There's likely a better without breaking everyone to support async http requests.

Understood. I think we should make it drop in compatible 0.12.x release which doesn't change the existing concurrency model. I think 0.12.x can continue to exist for a while.

That said I am also aware that these other libraries are unmaintained and it's mostly busy work to move forward with all.

Yes, as suggested, we could remove this in 0.13.x and adjust the concurrency model accordingly.

Finally, what happens when the next best async library pops up? :) Cause you know, ruby concurrency, EM, Celluloid, Async ... we've seen a few.

Here is my opinion on this.

The main reason why a lot of these libraries have "died" is because they were unmaintainable by design.

• EventMachine completely replaced standard Ruby IO with a native C++ code base. It was hard to maintain and broke with IPv6, was always generally buggy (could crash MRI) and for a long time was unmaintained. The actual EventMachine gem was quite complex, had a lot of functionality in one code base, and didn't have a straight forward model for concurrent network IO.

• Celluloid is not just about concurrency, but about actor based concurrency. It's a library which collapsed under it's own weight. There were some good parts (timers and nio4r) but it was simply trying to achieve too much, with too much configurability. There were only a few people capable of maintaining it because it was so complex.

• Async is a bit different. I worked on both the above gems. The complexity of async is split up into a layered approach to concurrency, with the core concepts being exposed in the async gem. The async gem, for almost all intents and purposes, is 100% stable. It has almost 100% test coverage. There is very little chance it would ever break, or need to have backwards incompatible changes. All other functionality is layered on top in other gems, e.g. async-io. Because these concerns are separated and layered, it's possible to maintain more easily and I believe long term it will be far more robust. I believe even in 10 years, async can still work, and be relevant, even without much maintenance.

[ioquatix]

N.B. Even thought EventMachine reached 1.0, it felt like to me it was never completely stable.

Celluloid never reached 1.0, it got stuck in limbo and will never escape.

In contrast, a stable, bug-free, 1.0 release of async with all core functionality was my main goal (which was achieved).

[ioquatix]

You can read a little bit more about it here: https://github.com/socketry/async#motivation

[ioquatix]

Also, the funny thing about ruby-concurrency is that while they call themselves that, ironically, they are all about parallel processing (threads). Fortunately, for them, the GIL ensures that everything is concurrent on MRI anyway :p

I wrote some article about this here: https://www.codeotaku.com/journal/2018-06/asynchronous-ruby/index

[dblock]

Thanks for the details @ioquatix. You're absolutely correct about these points, but there's still human factor. There're only 4 contributors to async and you're 99% of the code. That said adoption is everything and I am happy to recommend async rn.

[dblock]

There's an important point about deprecating other async libraries in, say 0.13, that I think you've missed. To keep up with the slack API this library needs updates. So if the last version that supports celluloid is 0.12, everyone who wants a newer api will have to upgrade to async. Seems unnecessary as supporting these many concurrency libraries is little overhead.

[ioquatix]

Those are all good points.

You'll need to make a decision at some point what kind of concurrency model you want. Right now you are using one thread per connection, but if you want to support more connections you'll need to change the model to something more in-line with how async works. Making celluloid work that way is probably impossible, making EventMachine work that way might be possible. I think if you drop support for anything, it should be Celluloid. At least EventMachine is maintained these days.

One interesting aspect of this is considering how to write a library without factoring in concurrency.

It turn out it is at least somewhat possible. I wrote http-protocol which is concurrency agnostic. The idea is to write a gem which handles the protocol concerns, and leave the actual multiplexing up to the user. I actually wish we could do this for all of async, but without support from MRI (I made a PR), it turns out to be generally difficult.

[dblock]

You'll need to make a decision at some point what kind of concurrency model you want. Right now you are using one thread per connection, but if you want to support more connections you'll need to change the model to something more in-line with how async works.

How would that look compared to what we have in this PR? Maybe open a new issue so we don't lose the info?

[ioquatix]

So, the main point is, that right now you have integration specs which are heavily geared towards using multiple threads. This is apparent based on the implementation of class QueueWithTimeout.

I think this is fine for how the code currently works.

Going forward, you need to be more agnostic towards how this works. I would like to believe this is just a problem with the specs, but it might be more difficult.

If a user is using EventMachine or Async, in theory it should be concurrent. You can start a connection and it should work without any explicit threads.

I would suggest that you actually remove Thread from the async wrapper, i.e.

          def start_async(client, task: ::Async.Task.current)
            task.async do
              client.run_loop
            end
          end

This is actually idiomatic and the best way to do it.

In order to support this though, you can't then call queue.pop_with_timeout(5) because this will block the thread and prevent the reactor from running. The only way this can work is if you call start_async on it's own dedicated thread, and then run your specs (e.g. queue.pop_with_timeout(5)) on a different thread. This way, it should work for celluloid, async and eventmachine.

[ioquatix]

Just FYI, while the EM reactor runs on a background thread, Async doesn't support this:

• It requires a lot of locking/synchronisation which is largely unnecessary if you simply design your program to have one reactor per thread/process.
• It leaks resources (the EM reactor/thread).
• It doesn't scale well.
• It makes program flow much more complex - what thread handles the IO callbacks?