Skip to content

Commit 6dd2579

Browse files
committed
check that the context deadline hasn't expired before running a query
If the context deadline expires before the query has a chance to execute, the query killer will terminate the connection immediately after sending the query to mysql. Instead, add a check at the start of dbConn.execOnce and fail the query immediately if the context is already past the deadline. Signed-off-by: Michael Demmer <mdemmer@slack-corp.com>
1 parent 27d0c70 commit 6dd2579

File tree

3 files changed

+83
-0
lines changed

3 files changed

+83
-0
lines changed

go/mysql/fakesqldb/server.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ type DB struct {
7070
name string
7171
// isConnFail trigger a panic in the connection handler.
7272
isConnFail bool
73+
// connDelay causes a sleep in the connection handler
74+
connDelay time.Duration
7375
// shouldClose, if true, tells ComQuery() to close the connection when
7476
// processing the next query. This will trigger a MySQL client error with
7577
// errno 2013 ("server lost").
@@ -288,6 +290,10 @@ func (db *DB) NewConnection(c *mysql.Conn) {
288290
panic(fmt.Errorf("simulating a connection failure"))
289291
}
290292

293+
if db.connDelay != 0 {
294+
time.Sleep(db.connDelay)
295+
}
296+
291297
if conn, ok := db.connections[c.ConnectionID]; ok {
292298
db.t.Fatalf("BUG: connection with id: %v is already active. existing conn: %v new conn: %v", c.ConnectionID, conn, c)
293299
}
@@ -517,6 +523,13 @@ func (db *DB) DisableConnFail() {
517523
db.isConnFail = false
518524
}
519525

526+
// SetConnDelay delays connections to this fake DB for the given duration
527+
func (db *DB) SetConnDelay(d time.Duration) {
528+
db.mu.Lock()
529+
defer db.mu.Unlock()
530+
db.connDelay = d
531+
}
532+
520533
// EnableShouldClose closes the connection when processing the next query.
521534
func (db *DB) EnableShouldClose() {
522535
db.mu.Lock()

go/vt/vttablet/tabletserver/connpool/dbconn.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,16 @@ func (dbc *DBConn) execOnce(ctx context.Context, query string, maxrows int, want
132132
dbc.current.Set(query)
133133
defer dbc.current.Set("")
134134

135+
// Check if the context is already past its deadline before
136+
// trying to execute the query.
137+
if ctx.Done() != nil {
138+
select {
139+
case <-ctx.Done():
140+
return nil, fmt.Errorf("%v before execution started", ctx.Err())
141+
default:
142+
}
143+
}
144+
135145
done, wg := dbc.setDeadline(ctx)
136146
if done != nil {
137147
defer func() {

go/vt/vttablet/tabletserver/connpool/dbconn_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,66 @@ func TestDBConnExec(t *testing.T) {
117117
compareTimingCounts(t, "Exec", 1, startCounts, tabletenv.MySQLStats.Counts())
118118
}
119119

120+
func TestDBConnDeadline(t *testing.T) {
121+
db := fakesqldb.New(t)
122+
defer db.Close()
123+
startCounts := tabletenv.MySQLStats.Counts()
124+
sql := "select * from test_table limit 1000"
125+
expectedResult := &sqltypes.Result{
126+
Fields: []*querypb.Field{
127+
{Type: sqltypes.VarChar},
128+
},
129+
RowsAffected: 1,
130+
Rows: [][]sqltypes.Value{
131+
{sqltypes.NewVarChar("123")},
132+
},
133+
}
134+
db.AddQuery(sql, expectedResult)
135+
136+
connPool := newPool()
137+
connPool.Open(db.ConnParams(), db.ConnParams(), db.ConnParams())
138+
defer connPool.Close()
139+
140+
db.SetConnDelay(100 * time.Millisecond)
141+
ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(50*time.Millisecond))
142+
defer cancel()
143+
144+
dbConn, err := NewDBConn(connPool, db.ConnParams())
145+
defer dbConn.Close()
146+
if err != nil {
147+
t.Fatalf("should not get an error, err: %v", err)
148+
}
149+
150+
_, err = dbConn.Exec(ctx, sql, 1, false)
151+
want := "context deadline exceeded before execution started"
152+
if err == nil || !strings.Contains(err.Error(), want) {
153+
t.Errorf("Exec: %v, want %s", err, want)
154+
}
155+
156+
compareTimingCounts(t, "Connect", 1, startCounts, tabletenv.MySQLStats.Counts())
157+
compareTimingCounts(t, "ConnectError", 0, startCounts, tabletenv.MySQLStats.Counts())
158+
compareTimingCounts(t, "Exec", 0, startCounts, tabletenv.MySQLStats.Counts())
159+
160+
startCounts = tabletenv.MySQLStats.Counts()
161+
162+
ctx, cancel = context.WithDeadline(context.Background(), time.Now().Add(10*time.Second))
163+
defer cancel()
164+
165+
result, err := dbConn.Exec(ctx, sql, 1, false)
166+
if err != nil {
167+
t.Fatalf("should not get an error, err: %v", err)
168+
}
169+
expectedResult.Fields = nil
170+
if !reflect.DeepEqual(expectedResult, result) {
171+
t.Errorf("Exec: %v, want %v", expectedResult, result)
172+
}
173+
174+
compareTimingCounts(t, "Connect", 0, startCounts, tabletenv.MySQLStats.Counts())
175+
compareTimingCounts(t, "ConnectError", 0, startCounts, tabletenv.MySQLStats.Counts())
176+
compareTimingCounts(t, "Exec", 1, startCounts, tabletenv.MySQLStats.Counts())
177+
178+
}
179+
120180
func TestDBConnKill(t *testing.T) {
121181
db := fakesqldb.New(t)
122182
defer db.Close()

0 commit comments

Comments
 (0)