On Dec 2, 2009, at 2:32 PM, Jeremy Evans wrote:
>> In a threaded system the call to #disconnect might always orphan some
>> connections because they are in use by another thread elsewhere. A
>> new
>> call to #connect would (???) throw a Sequel::DatabaseConnectionError.
>> If I am wrong about this, I would love to be corrected.
>
> No. Only available connections are disconnected. Any active
> connections will not be disconnected, and will be returned to the pool
> when they are no longer active. So while disconnect is safe and
> doesn't orphan connections, it also only completely disconnects from
> the database if there are no active connections to it.
Okay, I understand. It's missing behavior equivalent to Thread#join so
we can confirm all threads/connections have exited.
>> If I were to create tests for this and submit as a patch, would this
>> kind of functionality be accepted? Is there a superior way to add a
>> new sharding server to the connection pool at runtime that I have
>> missed?
>
> No, your implementation is generally sound, but I see a few issues:
>
> 1) The add_server_key internals should be protected by a
> @mutex.synchronize block, otherwise it is vulnerable to a race
> condition.
>
> 2) You need to make sure that the :default server always exists.
>
> 3) You need to protect against adding an already present server,
> otherwise add_server with an already existing key will orphan
> connections. You can probably modify the database's options to have
> an existing server key use new options, but you can't modify the
> connection pool keys.
>
> 4) Database#add_server uses :server instead of :servers as the option
> key, and I would probably have it accept a hash instead of multiple
> options. I would also change ConnectionPool#add_server to add_servers
> and have it take an array of symbols.
>
> If you fix those issues and add documentation and specs, I'll include
> this as long as it passes the test suite.
I just forked the repository. I'll send you a pull request after I get
this implemented and documented.
> Your post got me thinking about changes to ConnectionPool#disconnect:
>
> 1) Start accepting an options hash, for a :servers option, for only
> disconnecting those servers.
If you provide a Database#remove_server then this suggestion would be
"sugar" for calling #delete_server and then #add_server with new
options. I like it.
> 2) Have it start checking allocated connections, marking the threads
> with a flag. When the connection is released, the flag is checked,
> and if present, instead of the connection being returned to the pool,
> it is disconnected. This would allow calling disconnect even when
> there are active connections, knowing that any active connections will
> be disconnected as soon as they are finished. This would also allow
> add_server to change existing servers, by disconnecting, and having
> new connections go to the new server.
I would suggest adding this behavior as optional parameters to
Database#disconnect.
:remove_all => boolean
Flag all connections to be disconnected and removed from the pool
:async => boolean
When true, perform this operation asynchronously. When false, block
until all connections are removed.
Failure to pass any options would give the existing default behavior.
class Database
def disconnection opts = {}
if opts[: remove_all] && opts[:async]
# flag all for asynchronous disconnect
elsif opts[: remove_all]
# flag all and block for a synchronous disconnect
else
# existing default behavior
end
end
cr
--
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.
