ruby-amqp / bunny

Bunny is a popular, easy to use, mature Ruby client for RabbitMQ
Other
1.39k stars 303 forks source link

EventMachine/Fibers support #16

Closed cjbottaro closed 13 years ago

cjbottaro commented 13 years ago

Hello,

I added EventMachine+Fibers support: https://github.com/cjbottaro/bunny/commit/68824d77134d1851c2b1c84a20310b9f2214e65e

A part of that process was factoring out the idea of a "connection". You can see from the commit that @socket is replaced with a "connection". As far as I can tell, very little of the socket interface is used and I formalized what is used in Qrack::Connection::AbstractBase. Then I created two connections types that inherit from AbstractBase: Socket (the default) and FiberedEm.

Which connection type to use can be specified in Bunny#new, but if none is specified, it tries to dynamically figure out which to use (See Qrack::Client#default_connection_type).

All of Bunny's tests still pass using the default socket connection type. I've tested the :fibered_em connection type against a live RabbitMQ server (a basic subset of operations) and it seems to work ok. I have not written any specs / unit tests though.

I want to send a pull request, but would like yall to review the changes first and am curious for any ideas about how to write specs / unit tests.

Usage of Bunny + EM/Fibers is as follows:

require "bunny"
require "eventmachine"
require "fiber"

EM.run do
  Fiber.new do

    fm = Fiber.current
    done_count = 0

    f1 = Fiber.new do
      b = Bunny.new :connection_type => :fibered_em
      puts "start1"
      b.start
      puts "stop1"
      b.stop
      done_count += 1
      fm.resume
    end

    f2 = Fiber.new do
      b = Bunny.new :connection_type => :fibered_em
      puts "start2"
      b.start
      puts "stop2"
      b.stop
      done_count += 1
      fm.resume
    end

    f1.resume
    f2.resume

    # Kinda like joining threads.
    Fiber.yield while done_count < 2

    EM.stop

  end.resume
end

Note that specifying the :connection_type is unnecessary in that it should figure out that it's running in an EM+Fibers environment.

Thanks, -- C

michaelklishin commented 13 years ago

Qrack will be replaced with amq-protocol and sync part of amq-client.

From what I see this reinvents the very idea of amq-client, just with different adapters. Although it has been decided to make sync adapters API different because there are too many differences between CPS (aka "callbacks") and APIs with methods returning values (like bunny does).

So wait until amq-client's sync part API is designed. Some of your ideas should probably be used as is.

michaelklishin commented 13 years ago

As far as the testing goes, "regular" EventMachine and Cool.io code in 2011 should be tested with evented-spec, like amq-client does. Fibers add nothing special to the process of starting and stopping EventMachine reactor between spec example groups.

michaelklishin commented 13 years ago

Just curious, what's the benefit of using fibers in this example above? Fibers still share state and doing b.start then b.stop does not demonstrate anything. What's the benefit? I thought days of Mac OS Classic & Windows 98 showed that the tech industry has "been there, done that" with cooperative concurrency models. Now we are taking another round in the Ruby world, and while Goliath makes certain thing simpler, the example above doesn't demonstrate any AMQP operations/patterns to derive conclusions from. There are also no benchmarks posted.

cjbottaro commented 13 years ago

Ahh, yes... fiber do share state, hence why we need a separate Bunny instance in each fiber.

You're preaching to the choir kinda... :) I have been a proponent of simply using threads and blocking IO in Ruby for the past couple of years. But honestly, it's an uphill battle. Hack after hack and it's still not good enough. See this thread for a little taste of what I've been dealing with for so long: http://redmine.ruby-lang.org/issues/show/4266 Not to mention tons of other issues with gems autoloading or dynamically loading files/classes. Or simply just bugs in people's gems when it comes to threads (memcache-client has a good one, see here: https://github.com/mperham/memcache-client/pull/16). I'm just tired of it all.

What does the code snippet demonstrate? That concurrency works. That's it. I don't care if it benchmarks better or worse than the threaded version, I'm just sick of dealing with threading issues in Ruby. I am aware that cooperative concurrency has its own issues, but I'm willing to try something new.

So why evented + continuations? Because plain evented is just too much of a paradigm shift for the Rails world. All the libraries, gems, frameworks are made with blocking style APIs. Fibers give us a way to have concurrency in the normal blocking way we're used to, but without having to use threads (and thus minus all the issues Ruby has with threads).

So let me ask... why not? My commit is very simple, non intrusive code... everything is localized to a single method (and two files) So why not just have the option?

Also, why do you say "doing b.start then b.stop" does not demonstrate anything? Both #start and #stop are blocking net io operations. The snippet provided shows that each fiber runs "concurrently". I can throw it in a loop and provide benchmarks against serial and threaded versions of the snippets if you like, but really I'm not so concerned about performance (vs threaded anyway).

michaelklishin commented 13 years ago

I am not against adding this feature to amq-client (so that the next bunny release will provide it, too). I just wanted to make sure we are not aiming at ROFLSCALE with this move. Now I see that we aren't.

michaelklishin commented 13 years ago

If you can join us in #rabbitmq on freenode (I am antares_), we can discuss what basic methods sync amq-client adapters may have (it doesn't have to be exactly the same set async adapters have, by the way) and figure out what to do with this particular patch (we can merge it to 0.7.x-stable, although point releases with new features will make some semantic versioning proponents unhappy) and then move on to amq-client.

cjbottaro commented 13 years ago

Ha, that Xtranormal video was pretty good... not as good as the MongoDB one though... :)

I was unaware of amq-client, but I'll start to familiarize myself with it. I'm out of town starting tomorrow, so I'll catch up with you on IRC sometime next week.

Thanks.

botanicus commented 13 years ago

Interesting. @cjbottaro would you be interested in working with us on designing & writing the sync API for amq-client? That'd be really helpful, it's a lot of work, but it's also pretty exciting thing to do! It'd be great to discuss on IRC, unfortunately I have no idea how will it be with my time, I'm flying to SF next Monday ... BTW I've been also thinking a lot about Fibers and EM, I think it's a very interesting idea, I've been thinking about having the same API as Bunny now has, just from say q = bunny.queue("test") to return some lazy object which would behave like a promise (future) as well ... but it has some problems.

michaelklishin commented 13 years ago

Like I said before, bunny users choose bunny first and foremost for simplicity. They do not really need Future-like objects. I also think that the sync part of amq-client can be pretty different from the async part, that's fine. The who are pretty different in so many ways, unifying them will likely to only make our code very very complex.

I hope to catch up with @cjbottaro and sketch some API ideas by next week, from there, we can use email as usual. Fortunately, this particular feature (amq-client move) can wait.

botanicus commented 13 years ago

That's right, I've been thinking about "hiding" that it's a promise and let it behave like a lazy object, so the API would be same ... it's a new idea of mine, so I haven't thought it through yet ... but it sounds interesting.

michaelklishin commented 13 years ago

We are going to port bunny to amq-protocol next, looks like this idea is not worth pursuing: framing and network layers in Bunny are fine for what it tries to do. Not reimplementing basic protocol serialization is useful, replacing framing & networking layers will probably not yield bunny users anything.