New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Event dispatching backs up IO processing #8

Closed
iande opened this Issue Apr 21, 2011 · 3 comments

Comments

Projects
None yet
1 participant
@iande
Owner

iande commented Apr 21, 2011

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.

@ghost ghost assigned iande Apr 21, 2011

@iande

This comment has been minimized.

Show comment
Hide comment
@iande

iande Apr 28, 2011

Owner

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

Owner

iande commented Apr 28, 2011

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

@iande

This comment has been minimized.

Show comment
Hide comment
@iande

iande Apr 28, 2011

Owner

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)

Owner

iande commented Apr 28, 2011

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

This comment has been minimized.

Show comment
Hide comment
@iande

iande Apr 28, 2011

Owner

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

Owner

iande commented Apr 28, 2011

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

@iande iande closed this Jan 9, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment