search

gemhome / fnordmetric

(possible new home for) FnordMetric is a redis/ruby-based realtime Event-Tracking app
0 stars 1 forks source link

Async fixes #10

Open bf4 opened 10 years ago

bf4 commented 10 years ago

Issue by kazjote Friday Jan 20, 2012 at 11:21 GMT Originally opened as https://github.com/paulasmuth/fnordmetric/pull/49

Fixes some issues with execution flow jumping back and forth between EM reactor and EM non reactor threads.

Should prevent issues like #37 in the future.

kazjote included the following code: https://github.com/paulasmuth/fnordmetric/pull/49/commits

bf4 commented 10 years ago

Comment by paulasmuth Saturday Jan 21, 2012 at 14:57 GMT

I merged 0a701d4, thanks :) The Gemfile.lock is already in the gitignore and I will delete it from the repo. However, I don't really see why we should use the blocking client instead of the non-blocking one? It is faster this way.

bf4 commented 10 years ago

Comment by kazjote Saturday Jan 21, 2012 at 15:16 GMT

Why should it be faster?

EM has by default 1 reactor thread and a pool with 20 'side threads'.

At line https://github.com/paulasmuth/fnordmetric/blob/master/lib/fnordmetric/worker.rb#L35 we delegate whole event processing to 'side thread'. It is perfect as events can be processed in parallel.

However, whenever non blocking redis call is invoked, processing goes back to reactor thread. We experienced it in issue #37 . Things got really messed there.

In my opinion the design where events are taken from the queue in reactor thread and are processed in 'side threads' (without coming back to reactor thread) is much simplier and cleaner.

Other solution would be to process everything in one reactor thread (don't use defer) at all.

bf4 commented 10 years ago

Comment by kazjote Monday Jan 23, 2012 at 08:59 GMT

I am working currently on cohorts. I am about to finish now but I built it on top of a6a4fdf . It would be cool to know what is the final decision about commit a6a4fdf so I could prepare pull request with or without it.

If we are rejecting it I would go for solution without defer. As far as I know, MRI does not support threads running in parallel anyway so keeping the whole processing in reactor thread does not seem like a bad idea. :-)

bf4 commented 10 years ago

Comment by kazjote Saturday Mar 17, 2012 at 18:41 GMT

How is the current status? Should I refresh the merge (resolving conflicts)?

bf4 commented 10 years ago

Comment by chikamichi Thursday Jun 14, 2012 at 10:42 GMT

Hi,

Any news on this? Seemed pretty interesting.