Pool has to be terminated after use

[kakra]

Otherwise I end up with huge numbers of actors when the application crashes, with the database pool completely used...

[jwo]

Thanks for the pull!

Is there a possibility of testing this code? Or, simulating in my local environment?

[kakra]

Well, I'm using it with a Rails application pulling around 8000 import entries which in turn have some file attachments that need to be downloaded. With my fix, it works. Without, it terminates after a few seconds telling me that over 40-50 actors had been killed. The Celluloid docs also state you have to cleanup your actors yourself because they are not garbage collected. Maybe it'd be even better to wrap it into a rescue block and use the "ensure" keyword to ensure cleaning the pool. The tap method was just lazier, tho... ;-)

I'm not sure about the supervisor part... You mention it but I cannot see it in your code.

https://github.com/celluloid/celluloid/wiki/Actor-lifecycle

[jwo]

I get that issue with Celluloid 0.15, but not 0.12. Celluloid added its own feature of pmap in since 0.12 that doesn't work as nice as mine.

I need to rework my gem to work around this.

Example, if you add this, it passed with mine, but not with Celluloid 0.15

it 'does not fail with 800 things' do
   (1..800).to_a.pmap(4) {|x|  x * rand }
end

[jwo]

Anyway, tl;dr @kakra I'll get to this for sure.

[kakra]

I'm using it with celluloid 0.12 due to issue #1...

To construct a test case which could fail you need to involve ActiveRecord with a connected database pool into the test... Or at least find a way to count the active actors after the test which should go down back to where it started. Something like

it 'does not leave active actors behind' do
  # pseude-code but you get the idea:
  assert_difference :active_actors, 0 do
     (1..800).to_a.pmap(4) {|x|  x * rand }
  end
end

[kakra]

I still had problems with orphans left behind when exceptions occured within actors. This one should finally fix it. It may be related to our problem that Passenger looses track of ruby processes sometimes which in turn occupied 1-2 GB of memory that was then never freed. I'll see if this fixes it. The patch is important anyways.

[jwo]

Looking very good :)

[kakra]

I need your advise. I tried to fiddle around with this a little more. Your implementation easily leaks lingering database connection for activerecord if not handled explicitly. I've wrapper my pmap workers with additional code like so:

some_ar_objects.pmap(4) do |object|
  ActiveRecord::Base.connection_pool.with_connection { object.do_hefty_stuff! }
end

And guess what: The pool exhaustion issues and connection timeouts are gone!

I think this wrapping should somehow be integrated into your pmap implementation but it look quite wrong to just put it right into your pmap method because one may not even touch ActiveRecord there. Well, in my case it could patch it in because using AR from within pmap is my usual application.

What do you think about this (but it adds an implicit dependency on AR, feels wrong too):

def pmap_with_pool size
  pmap size do |object|
    ActiveRecord::Base.connection_pool.with_connection { yield object }
  end 
end

Maybe we could add another optional parameter which passes in a wrapper method to get the AR dependency out of pmap:

def pmap size, wrapper = nil
  ...
  wrapper.call { yield object } if wrapper
end

then call with:

some_ar_objects.pmap(4, ActiveRecord::Base.connection_pool.method(:with_connection)) { ... }

Not sure about the syntax but you get the idea...

[jwo]

My thinking is add this to the instructions. That way people can still make http calls (or otherwise do whatever in parallel), and active record calls. 

This way, it would still be used outside of rails. Thoughts?

On Sun, May 25, 2014 at 5:53 PM, Kai Krakow [email protected]
wrote:

I need your advise. I tried to fiddle around with this a little more. Your implementation easily leaks lingering database connection for activerecord if not handled explicitly. I've wrapper my pmap workers with additional code like so:
some_ar_objects.pmap(4) do |object|
ActiveRecord::Base.connection_pool.with_connection { object.do_hefty_stuff! }
end
And guess what: The pool exhaustion issues and connection timeouts are gone!
I think this wrapping should somehow be integrated into your pmap implementation but it look quite wrong to just put it right into your pmap method because one may not even touch ActiveRecord there. Well, in my case it could patch it in because using AR from within pmap is my usual application.
What do you think about this (but it adds an implicit dependency on AR, feels wrong too):
def pmap_with_pool size
pmap size do |object|
ActiveRecord::Base.connection_pool.with_connection { yield object }
end
end
Maybe we could add another optional parameter which passes in a wrapper method to get the AR dependency out of pmap:
def pmap size, wrapper = nil
...
wrapper.call { yield object } if wrapper
end
then call with:
some_ar_objects.pmap(4, &ActiveRecord::Base.connection_pool.with_connection) { ... }
Not sure about the syntax but you get the idea...

---

Reply to this email directly or view it on GitHub:
#2 (comment)

[jwo]

Thanks for your help on this @kakra. I'm hopeful the instructions will help devs like you in the future.

Further: I'm open to helping create a new gem plugin, celluloid-pmap_rails or similar that has the code we discussed built in.