Skip to content

Commit 118aaf8

Browse files
craig[bot]fqazi
andcommitted
Merge #153971
153971: sql: deflake TestReadCommittedImplicitPartitionUpsert r=fqazi a=fqazi The test TestReadCommittedImplicitPartitionUpsert started flaking after we switched the implementation of TRUNCATE to the declarative schema changer. This happened because the testing knobs were incorrectly picking up internal queries. To address this, this patch modifies the testing knobs to include the SQL statement, so they can skip unrelated queries. Fixes: #153644 Release note: None Co-authored-by: Faizan Qazi <[email protected]>
2 parents 2b2511f + e984af4 commit 118aaf8

File tree

5 files changed

+14
-6
lines changed

5 files changed

+14
-6
lines changed

pkg/sql/exec_util.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2017,7 +2017,7 @@ type ExecutorTestingKnobs struct {
20172017

20182018
// AfterArbiterRead, if set, will be called after each row read from an arbiter index
20192019
// for an UPSERT or INSERT.
2020-
AfterArbiterRead func()
2020+
AfterArbiterRead func(query string)
20212021

20222022
// BeforeIndexSplitAndScatter is invoked with the split and scatter of an index
20232023
// occurs.

pkg/sql/insert.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,7 @@ func (n *insertNode) BatchedNext(params runParams) (bool, error) {
326326
if buildutil.CrdbTestBuild {
327327
// This testing knob allows us to suspend execution to force a race condition.
328328
if fn := params.ExecCfg().TestingKnobs.AfterArbiterRead; fn != nil {
329-
fn()
329+
fn(params.p.stmt.SQL)
330330
}
331331
}
332332

pkg/sql/insert_fast_path.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -476,7 +476,7 @@ func (n *insertFastPathNode) BatchedNext(params runParams) (bool, error) {
476476
if buildutil.CrdbTestBuild {
477477
// This testing knob allows us to suspend execution to force a race condition.
478478
if fn := params.ExecCfg().TestingKnobs.AfterArbiterRead; fn != nil {
479-
fn()
479+
fn(params.p.stmt.SQL)
480480
}
481481
}
482482

pkg/sql/mutation_test.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
gosql "database/sql"
1111
"fmt"
1212
"reflect"
13+
"strings"
1314
"sync"
1415
"testing"
1516

@@ -133,13 +134,20 @@ func TestReadCommittedImplicitPartitionUpsert(t *testing.T) {
133134
mu.l.Unlock()
134135
}
135136

137+
peekState := func() State {
138+
mu.l.Lock()
139+
defer mu.l.Unlock()
140+
return mu.state
141+
}
142+
136143
ctx := context.Background()
137144
params, _ := createTestServerParamsAllowTenants()
138145
// If test is in Ready state, transition to ReadDone and wait for conflict.
139146
params.Knobs = base.TestingKnobs{
140147
SQLExecutor: &sql.ExecutorTestingKnobs{
141-
AfterArbiterRead: func() {
142-
if mu.state != Ready {
148+
AfterArbiterRead: func(query string) {
149+
// Only wait for arbiter operations on the upsert table.
150+
if peekState() != Ready || !strings.Contains(query, "d.upsert") {
143151
return
144152
}
145153
setState(ReadDone)

pkg/sql/upsert.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ func (r *upsertRun) processSourceRow(params runParams, rowVals tree.Datums) erro
207207
if buildutil.CrdbTestBuild {
208208
// This testing knob allows us to suspend execution to force a race condition.
209209
if fn := params.ExecCfg().TestingKnobs.AfterArbiterRead; fn != nil {
210-
fn()
210+
fn(params.p.stmt.SQL)
211211
}
212212
}
213213

0 commit comments

Comments
 (0)