On Jan 22, 8:31 am, Jeremy Evans <[email protected]> wrote: > On Jan 22, 5:49 am, jan zimmek <[email protected]> wrote: > > > hi, > > > i just want to let you know, that i have started a project to bring > > together sequel, fibers, eventmachine and postgresql. > > >https://github.com/jzimmek/em-postgresql-sequel > > > the project is still in alpha stage, but we will use it for one of our > > upcoming projects, so it will hopefully mature pretty soon. > > > feel free to report bugs, feature request or send pull requests. > > Awesome. I'll try to review this within a few days. If you need any > help or have any questions, please let me know. If you want some good > general tests, try running the spec_postgres task with your adapter.
I looked over your implementation and it looks clean. Here's a few suggestions from my review of the code: = mutex.rb * Don't raise a string. * I would yield unconditionally in #synchronize, since calling #synchronize without a block should raise an error. = pgconn.rb * callback and errback do the same thing. If that is expected, use the same block for both. * There's no reason for the module Sequel module Postgres stuff if you are just reopening ::PGconn. = fibered_connection_pool.rb * Consider preallocating the disconnection mutex in #initialize, may fix potentaial race if 2 separate threads call disconnect simultaneously. * #size looks wrong, should probably be available.length + waiting.length. * You are preallocating connections in #initialize and in #hold if there is a disconnection, but #disconnect leaves @available empty. Looks like disconnect would break the pool. Three options here: * Just accept the current implementation, and add a #reconnect method for setting up connections. * Make disconnect also reconnect. This is bad as it breaks cases were you really want to disconnect (such as in a before fork hook for a multi-process webserver to make sure child processes use their own connections). * Switch to general Sequel style of connecting on demand (start with empty pool, make new connection if no available connections and number of waiting connections is less than max connections). I tried running Sequel's spec_postgres task with em-postgresql-sequel, but unfortunately most of it failed with "can't yield from root fiber" messages. You may want to look into that. My first guess is that the specs test disconnection, and your #disconnect implementation appears broken. Hope this helps. If you have any questions, please let me know. I don't have any experience with EM, and only a basic understanding of Fibers, but hopefully I'll be able to help. Thanks, Jeremy -- You received this message because you are subscribed to the Google Groups "sequel-talk" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/sequel-talk?hl=en.
