iande / onstomp

A STOMP messaging client library for Ruby
http://mathish.com/projects/onstomp.html
Other
23 stars 11 forks source link

Event dispatching backs up IO processing #8

Closed iande closed 12 years ago

iande commented 13 years ago

Originally I wasn't too concerned about this, but now I am.

Dispatching events and invoking their callbacks within the IO processing thread is problematic for a couple reasons.

First, event callbacks are invoked either after an IO process cycle (connection events), or while io_process_write or io_process_read are doing their work (in the case of transmitted / received frames.) So, long running callbacks will nullify some of the benefits of using non-blocking IO.

Second, if an event callback raises an exception, the end result (depending upon where the callback was invoked) will either be a shutdown of the connection, or a shutdown of the IO processor thread. While this isn't as big of a deal as it may initially seem, it's still a bit troubling and it makes my pants very angry.

I think the solution will be to introduce a thread for clients to enqueue events and invoke callbacks. This will create some new issues to be handled, such as:

  1. Presently, the before_transmitting and all before_<client_frame> events are handled within the same thread that the client frame method was called in. In other words, if client.send '/queue/test', 'hello world' is called within Thread-1, then the events before_transmitting and before_send are also dispatched in Thread-1. This makes it very simple to allow event handlers to modify frames before their pumped off to the IO processor, since the invocation of send will not return until all of those event callbacks have been invoked. If we handle these events in a separate thread, then care must be taken to ensure all appropriate events have been processed before the frame is finally handed off to the underlying connection.
  2. From the end user perspective, it shouldn't generally matter if the after_transmitting, before_receiving, after_receiving, on_<frame>, on_connection_<event_name> events are triggered from a new thread, since that's already happening with the IO processor. All that matters is that the order of event processing is preserved. However, the failover extension does install handlers for certain connection events and having them dispatched independently of when/where the event originally occurred may upset it.
  3. Transmitting frames within an event callback requires some special consideration. The write mutex held by an OnStomp::Connection::Base instance does a fine job now, but I definitely need to double check the code to keep any badness at bay.
  4. Spec'ing these changes is going to suck quite a bit. Testing, mocking, and stubbing the interactions between multiple threads is precisely as much fun as it sounds like.

The introduction of a new event dispatcher probably warrants a minor version increment, so all of this work will be committed to the 1.1.0 branch until it's ready to go.

iande commented 13 years ago

Got this worked out pretty well in the event_thread branch. But, there is some additional overhead with my approach thus far.

iande commented 13 years ago

The performance is now essentially the same as the previous approach. Read speed is a little faster, write speed fluctuates between slightly faster and slightly slower, so I'm going to call that "the same." The read performance becomes especially noticeable when dealing with large-ish frames (body size of 1 MB)

iande commented 13 years ago

Now that it's working, it's time to re-write specs and BDD the crap out of the Failover extension.