2
0

Define close resource not to return an error

There is nothing the pool can do on an error. If the client code
wishes to handle or log errors it can do so in the close resource
function.

This removes the need for background error reporting.
This commit is contained in:
Jack Christensen
2018-12-25 14:17:17 -06:00
parent d4f6b3dbba
commit 7c5f3f0446
2 changed files with 16 additions and 109 deletions
+13 -53
View File
@@ -21,12 +21,7 @@ const maxInt = int(maxUint >> 1)
var ErrClosedPool = errors.New("cannot get from closed pool") var ErrClosedPool = errors.New("cannot get from closed pool")
type CreateFunc func(ctx context.Context) (res interface{}, err error) type CreateFunc func(ctx context.Context) (res interface{}, err error)
type CloseFunc func(res interface{}) (err error) type CloseFunc func(res interface{})
// BackgroundErrorHandler is the type of function that handles background
// errors. It may be called while the pool is locked. Therefore it must not call
// any pool methods and should not perform any lengthy operations.
type BackgroundErrorHandler func(err error)
type resourceWrapper struct { type resourceWrapper struct {
resource interface{} resource interface{}
@@ -47,21 +42,19 @@ type Pool struct {
maxResourceCheckouts uint64 maxResourceCheckouts uint64
closed bool closed bool
createRes CreateFunc createRes CreateFunc
closeRes CloseFunc closeRes CloseFunc
backgroundErrorHandler BackgroundErrorHandler
} }
func NewPool(createRes CreateFunc, closeRes CloseFunc) *Pool { func NewPool(createRes CreateFunc, closeRes CloseFunc) *Pool {
return &Pool{ return &Pool{
cond: sync.NewCond(new(sync.Mutex)), cond: sync.NewCond(new(sync.Mutex)),
allResources: make(map[interface{}]*resourceWrapper), allResources: make(map[interface{}]*resourceWrapper),
maxSize: maxInt, maxSize: maxInt,
maxResourceDuration: math.MaxInt64, maxResourceDuration: math.MaxInt64,
maxResourceCheckouts: math.MaxUint64, maxResourceCheckouts: math.MaxUint64,
createRes: createRes, createRes: createRes,
closeRes: closeRes, closeRes: closeRes,
backgroundErrorHandler: func(error) {},
} }
} }
@@ -72,10 +65,7 @@ func (p *Pool) Close() {
p.closed = true p.closed = true
for _, rw := range p.availableResources { for _, rw := range p.availableResources {
err := p.closeRes(rw.resource) p.closeRes(rw.resource)
if err != nil {
p.backgroundErrorHandler(err)
}
delete(p.allResources, rw.resource) delete(p.allResources, rw.resource)
} }
p.availableResources = nil p.availableResources = nil
@@ -166,17 +156,6 @@ func (p *Pool) SetMaxResourceCheckouts(n uint64) {
p.cond.L.Unlock() p.cond.L.Unlock()
} }
// SetBackgroundErrorHandler assigns a handler for errors that have no other
// place to be reported. For example, Get is called when no resources are
// available. Get begins creating a new resource (in a goroutine). Before the
// new resource is completed, the context passed to Get is canceled. Then the
// new resource creation fails. f will be called with that error.
func (p *Pool) SetBackgroundErrorHandler(f BackgroundErrorHandler) {
p.cond.L.Lock()
p.backgroundErrorHandler = f
p.cond.L.Unlock()
}
// Get gets a resource from the pool. If no resources are available and the pool // Get gets a resource from the pool. If no resources are available and the pool
// is not at maximum capacity it will create a new resource. If the pool is at // is not at maximum capacity it will create a new resource. If the pool is at
// maximum capacity it will block until a resource is available. ctx can be used // maximum capacity it will block until a resource is available. ctx can be used
@@ -258,17 +237,6 @@ func (p *Pool) Get(ctx context.Context) (interface{}, error) {
} }
} }
// func (p *Pool) backgroundReportError(errChan chan error) {
// go func() {
// err := <-errChan
// if err != nil {
// p.cond.L.Lock()
// p.backgroundErrorHandler(err)
// p.cond.L.Unlock()
// }
// }()
// }
// lockedAvailableGet gets the top resource from p.availableResources. p.cond.L // lockedAvailableGet gets the top resource from p.availableResources. p.cond.L
// must already be locked. len(p.availableResources) must be > 0. // must already be locked. len(p.availableResources) must be > 0.
func (p *Pool) lockedAvailableGet() interface{} { func (p *Pool) lockedAvailableGet() interface{} {
@@ -284,10 +252,7 @@ func (p *Pool) lockedAvailableGet() interface{} {
func (p *Pool) backgroundClose(res interface{}) { func (p *Pool) backgroundClose(res interface{}) {
go func() { go func() {
err := p.closeRes(res) p.closeRes(res)
if err != nil {
p.backgroundErrorHandler(err)
}
}() }()
} }
@@ -347,11 +312,6 @@ func (p *Pool) Remove(res interface{}) {
// close the resource in the background // close the resource in the background
go func() { go func() {
err := p.closeRes(res) p.closeRes(res)
if err != nil {
p.cond.L.Lock()
p.backgroundErrorHandler(err)
p.cond.L.Unlock()
}
}() }()
} }
+3 -56
View File
@@ -60,18 +60,16 @@ func createCreateResourceFuncWithNotifierChan() (puddle.CreateFunc, *Counter, ch
func createCloseResourceFuncWithNotifierChan() (puddle.CloseFunc, *Counter, chan int) { func createCloseResourceFuncWithNotifierChan() (puddle.CloseFunc, *Counter, chan int) {
ch := make(chan int) ch := make(chan int)
var c Counter var c Counter
f := func(interface{}) error { f := func(interface{}) {
n := c.Next() n := c.Next()
// Because the tests will not read from ch until after the close function f returns. // Because the tests will not read from ch until after the close function f returns.
go func() { ch <- n }() go func() { ch <- n }()
return nil
} }
return f, &c, ch return f, &c, ch
} }
func stubCloseRes(interface{}) error { return nil } func stubCloseRes(interface{}) {}
func waitForRead(ch chan int) bool { func waitForRead(ch chan int) bool {
select { select {
@@ -233,9 +231,8 @@ func TestPoolCloseClosesAllAvailableResources(t *testing.T) {
createFunc, _ := createCreateResourceFunc() createFunc, _ := createCreateResourceFunc()
var closeCalls Counter var closeCalls Counter
closeFunc := func(interface{}) error { closeFunc := func(interface{}) {
closeCalls.Next() closeCalls.Next()
return nil
} }
p := puddle.NewPool(createFunc, closeFunc) p := puddle.NewPool(createFunc, closeFunc)
@@ -356,56 +353,6 @@ func TestPoolGetReturnsErrorWhenPoolIsClosed(t *testing.T) {
assert.Nil(t, res) assert.Nil(t, res)
} }
func TestPoolCloseResourceCloseErrorIsReported(t *testing.T) {
createFunc, _ := createCreateResourceFunc()
errCloseFailed := errors.New("close failed")
closeFunc := func(res interface{}) error { return errCloseFailed }
pool := puddle.NewPool(createFunc, closeFunc)
asyncErrChan := make(chan error, 1)
pool.SetBackgroundErrorHandler(func(err error) { asyncErrChan <- err })
// Get and return a resource to put something in the pool
res, err := pool.Get(context.Background())
require.NoError(t, err)
assert.Equal(t, 1, res)
pool.Return(res)
pool.Close()
select {
case err = <-asyncErrChan:
assert.Equal(t, errCloseFailed, err)
default:
t.Fatal("error not reported")
}
}
func TestPoolReturnClosesResourcePoolIsAlreadyClosedErrorIsReported(t *testing.T) {
createFunc, _ := createCreateResourceFunc()
errCloseFailed := errors.New("close failed")
closeFunc := func(res interface{}) error { return errCloseFailed }
pool := puddle.NewPool(createFunc, closeFunc)
asyncErrChan := make(chan error, 1)
pool.SetBackgroundErrorHandler(func(err error) { asyncErrChan <- err })
// Get and return a resource to put something in the pool
res, err := pool.Get(context.Background())
require.NoError(t, err)
assert.Equal(t, 1, res)
pool.Close()
pool.Return(res)
select {
case err = <-asyncErrChan:
assert.Equal(t, errCloseFailed, err)
case <-time.NewTimer(time.Second).C:
t.Fatal("timed out waiting for async error")
}
}
func BenchmarkPoolGetAndReturn(b *testing.B) { func BenchmarkPoolGetAndReturn(b *testing.B) {
benchmarks := []struct { benchmarks := []struct {
poolSize int poolSize int