Skip to content

Commit

Permalink
fix(tests): fixed data race issue on checkIdelStmt
Browse files Browse the repository at this point in the history
  • Loading branch information
cnlangzi committed Apr 9, 2024
1 parent d6868f1 commit dd1f9ee
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 43 deletions.
4 changes: 3 additions & 1 deletion context.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"database/sql"
"log"
"sync"
"time"
)

type Context struct {
Expand All @@ -15,7 +16,8 @@ type Context struct {
stmts map[string]*Stmt
stmtsMutex sync.Mutex

Index int
stmtMaxIdleTime time.Duration
Index int
}

func (db *Context) Query(query string, args ...any) (*Rows, error) {
Expand Down
4 changes: 2 additions & 2 deletions context_stmt.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (db *Context) closeStaleStmt() {
db.stmtsMutex.Lock()
defer db.stmtsMutex.Unlock()

lastActive := time.Now().Add(-StmtMaxIdleTime)
lastActive := time.Now().Add(-db.stmtMaxIdleTime)
for k, s := range db.stmts {
s.mu.Lock()
if !s.isUsing && s.lastUsed.Before(lastActive) {
Expand All @@ -66,7 +66,7 @@ func (db *Context) closeStaleStmt() {

func (db *Context) checkIdleStmt() {
for {
<-time.After(StmtMaxIdleTime)
<-time.After(db.stmtMaxIdleTime)

db.closeStaleStmt()
}
Expand Down
45 changes: 7 additions & 38 deletions context_stmt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ func TestStmt(t *testing.T) {

db := Open(d)

db.stmtMaxIdleTime = 1 * time.Second

tests := []struct {
name string
run func(t *testing.T)
Expand Down Expand Up @@ -125,9 +127,7 @@ func TestStmt(t *testing.T) {
require.True(t, ok)
require.False(t, s.isUsing)

StmtMaxIdleTime = 1 * time.Second
time.Sleep(StmtMaxIdleTime)

time.Sleep(2 * time.Second)
db.closeStaleStmt()

// stmt should be closed and released
Expand All @@ -142,14 +142,6 @@ func TestStmt(t *testing.T) {
{
name: "stmt_reuse_should_work_in_rows_scan",
run: func(t *testing.T) {

stmtMaxIdleTime := StmtMaxIdleTime
defer func() {
StmtMaxIdleTime = stmtMaxIdleTime
}()

StmtMaxIdleTime = 1 * time.Second

var id int
q := "SELECT id, 'rows_scan' as reuse FROM rows WHERE id = ?"
rows, err := db.Query(q, 200)
Expand All @@ -159,7 +151,7 @@ func TestStmt(t *testing.T) {
require.True(t, ok)
require.True(t, s.isUsing)

time.Sleep(StmtMaxIdleTime + 1*time.Second)
time.Sleep(2 * time.Second)
db.closeStaleStmt()

// stmt that is in using should not be closed
Expand All @@ -185,13 +177,6 @@ func TestStmt(t *testing.T) {
ID int
}

stmtMaxIdleTime := StmtMaxIdleTime
defer func() {
StmtMaxIdleTime = stmtMaxIdleTime
}()

StmtMaxIdleTime = 1 * time.Second

q := "SELECT id, 'rows_bind' as reuse FROM rows WHERE id = ?"
rows, err := db.Query(q, 200)
require.NoError(t, err)
Expand All @@ -200,7 +185,7 @@ func TestStmt(t *testing.T) {
require.True(t, ok)
require.True(t, s.isUsing)

time.Sleep(StmtMaxIdleTime + 1*time.Second)
time.Sleep(2 * time.Second)
db.closeStaleStmt()

// stmt that is in using should not be closed
Expand All @@ -222,14 +207,6 @@ func TestStmt(t *testing.T) {
{
name: "stmt_reuse_should_work_in_row_scan",
run: func(t *testing.T) {

stmtMaxIdleTime := StmtMaxIdleTime
defer func() {
StmtMaxIdleTime = stmtMaxIdleTime
}()

StmtMaxIdleTime = 1 * time.Second

var id int
q := "SELECT id, 'row_scan' as reuse FROM rows WHERE id = ?"
row := db.QueryRow(q, 200)
Expand All @@ -239,7 +216,7 @@ func TestStmt(t *testing.T) {
require.True(t, ok)
require.True(t, s.isUsing)

time.Sleep(StmtMaxIdleTime + 1*time.Second)
time.Sleep(2 * time.Second)
db.closeStaleStmt()

// stmt that is in using should not be closed
Expand All @@ -264,14 +241,6 @@ func TestStmt(t *testing.T) {
var r struct {
ID int
}

stmtMaxIdleTime := StmtMaxIdleTime
defer func() {
StmtMaxIdleTime = stmtMaxIdleTime
}()

StmtMaxIdleTime = 1 * time.Second

q := "SELECT id, 'row_bind' as reuse FROM rows WHERE id = ?"
row, err := db.Query(q, 200)
require.NoError(t, err)
Expand All @@ -280,7 +249,7 @@ func TestStmt(t *testing.T) {
require.True(t, ok)
require.True(t, s.isUsing)

time.Sleep(StmtMaxIdleTime + 1*time.Second)
time.Sleep(2 * time.Second)
db.closeStaleStmt()

// stmt that is in using should not be closed
Expand Down
6 changes: 4 additions & 2 deletions db.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,10 @@ type DB struct {
func Open(dbs ...*sql.DB) *DB {
d := &DB{
Context: &Context{
DB: dbs[0],
stmts: make(map[string]*Stmt),
DB: dbs[0],
stmts: make(map[string]*Stmt),
Index: 0,
stmtMaxIdleTime: StmtMaxIdleTime,
},
dhts: make(map[string]*shardid.DHT),
}
Expand Down

0 comments on commit dd1f9ee

Please sign in to comment.