Skip to content

Commit

Permalink
pool: fix race condition at GetNextConnection()
Browse files Browse the repository at this point in the history
The `r.current` value can be changed by concurrent threads because
the change happens under read-lock. We could use the atomic counter
for a current connection number to avoid the race condition.

Closes #309

(cherry picked from dbfaab5)
  • Loading branch information
oleg-jukovec committed Aug 3, 2023
1 parent 2aafc13 commit cc19a61
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 7 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ Versioning](http://semver.org/spec/v2.0.0.html) except to the first release.

### Fixed

- Race condition at roundRobinStrategy.GetNextConnection() (#309)

## [1.12.0] - 2023-06-07

The release introduces the ability to gracefully close Connection
Expand Down
29 changes: 29 additions & 0 deletions connection_pool/connection_pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2065,6 +2065,35 @@ func TestDo(t *testing.T) {
require.NotNilf(t, resp, "response is nil after Ping")
}

func TestDo_concurrent(t *testing.T) {
roles := []bool{true, true, false, true, false}

err := test_helpers.SetClusterRO(servers, connOpts, roles)
require.Nilf(t, err, "fail to set roles for cluster")

connPool, err := connection_pool.Connect(servers, connOpts)
require.Nilf(t, err, "failed to connect")
require.NotNilf(t, connPool, "conn is nil after Connect")

defer connPool.Close()

req := tarantool.NewPingRequest()
const concurrency = 100
var wg sync.WaitGroup
wg.Add(concurrency)

for i := 0; i < concurrency; i++ {
go func() {
defer wg.Done()

_, err := connPool.Do(req, connection_pool.ANY).Get()
assert.Nil(t, err)
}()
}

wg.Wait()
}

func TestNewPrepared(t *testing.T) {
test_helpers.SkipIfSQLUnsupported(t)

Expand Down
14 changes: 7 additions & 7 deletions connection_pool/round_robin.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package connection_pool

import (
"sync"
"sync/atomic"

"github.com/tarantool/go-tarantool"
)
Expand All @@ -10,8 +11,8 @@ type RoundRobinStrategy struct {
conns []*tarantool.Connection
indexByAddr map[string]uint
mutex sync.RWMutex
size uint
current uint
size uint64
current uint64
}

func NewEmptyRoundRobin(size int) *RoundRobinStrategy {
Expand Down Expand Up @@ -98,13 +99,12 @@ func (r *RoundRobinStrategy) AddConn(addr string, conn *tarantool.Connection) {
r.conns[idx] = conn
} else {
r.conns = append(r.conns, conn)
r.indexByAddr[addr] = r.size
r.indexByAddr[addr] = uint(r.size)
r.size += 1
}
}

func (r *RoundRobinStrategy) nextIndex() uint {
ret := r.current % r.size
r.current++
return ret
func (r *RoundRobinStrategy) nextIndex() uint64 {
next := atomic.AddUint64(&r.current, 1)
return (next - 1) % r.size
}

0 comments on commit cc19a61

Please sign in to comment.