Skip to content

Commit

Permalink
Fix the SingleThreadedPool's handling of disconnected connections
Browse files Browse the repository at this point in the history
This shows that while 100% code coverage may not fix all bugs, it
certainly helps find some.
  • Loading branch information
jeremyevans committed Dec 4, 2008
1 parent 9c52f62 commit 722ce5b
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 5 deletions.
8 changes: 4 additions & 4 deletions lib/sequel_core/connection_pool.rb
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ class Sequel::SingleThreadedPool
# to RuntimeError exceptions (default true)
def initialize(opts={}, &block)
@connection_proc = block
@disconnection_proc = opts[:disconnection_proc]
@conns = {}
@convert_exceptions = opts.include?(:pool_convert_exceptions) ? opts[:pool_convert_exceptions] : true
end
Expand All @@ -226,13 +227,12 @@ def conn(server=:default)
# This method simulates the ConnectionPool#hold API.
def hold(server=:default)
begin
yield(c = (@conns[server] ||= @connection_proc.call(server)))
rescue Sequel::DatabaseDisconnectError
begin
yield(c = (@conns[server] ||= @connection_proc.call(server)))
rescue Sequel::DatabaseDisconnectError => dde
@conns.delete(server)
@disconnection_proc.call(c) if @disconnection_proc
rescue Exception => e
raise(@convert_exceptions && !e.is_a?(StandardError) ? RuntimeError.new(e.message) : e)
raise
end
rescue Exception => e
# if the error is not a StandardError it is converted into RuntimeError.
Expand Down
24 changes: 23 additions & 1 deletion spec/sequel_core/connection_pool_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,8 @@ def value

context "A single threaded pool with multiple servers" do
setup do
@pool = Sequel::SingleThreadedPool.new(CONNECTION_POOL_DEFAULTS.merge(:servers=>{:read_only=>{}})){|server| server}
@max_size=2
@pool = Sequel::SingleThreadedPool.new(CONNECTION_POOL_DEFAULTS.merge(:disconnection_proc=>proc{|c| @max_size=3}, :servers=>{:read_only=>{}})){|server| server}
end

specify "should use the :default server by default" do
Expand Down Expand Up @@ -487,4 +488,25 @@ def value
@pool.conn.should == nil
@pool.conn(:read_only).should == nil
end

specify ":disconnection_proc option should set the disconnection proc to use" do
@max_size.should == 2
proc{@pool.hold{raise Sequel::DatabaseDisconnectError}}.should raise_error(Sequel::DatabaseDisconnectError)
@max_size.should == 3
end

specify "#disconnection_proc= should set the disconnection proc to use" do
a = 1
@pool.disconnection_proc = proc{|c| a += 1}
proc{@pool.hold{raise Sequel::DatabaseDisconnectError}}.should raise_error(Sequel::DatabaseDisconnectError)
a.should == 2
end

specify "#hold should remove the connection if a DatabaseDisconnectError is raised" do
@pool.instance_variable_get(:@conns).length.should == 0
@pool.hold{}
@pool.instance_variable_get(:@conns).length.should == 1
proc{@pool.hold{raise Sequel::DatabaseDisconnectError}}.should raise_error(Sequel::DatabaseDisconnectError)
@pool.instance_variable_get(:@conns).length.should == 0
end
end

0 comments on commit 722ce5b

Please sign in to comment.