Skip to content

Commit

Permalink
sql: fix TestCreateStatisticsCanBeCancelled hang
Browse files Browse the repository at this point in the history
Previously, this test could hang if there was an automatic
stats came in concurrently with a manual stats collection,
where the request filter would end up hanging and being called twice.
To address this patch will disable automatic stats collections
on the table.

Fixes: #109007
Release note: None
  • Loading branch information
fqazi committed Aug 21, 2023
1 parent 104ed81 commit a8bb526
Showing 1 changed file with 17 additions and 8 deletions.
25 changes: 17 additions & 8 deletions pkg/sql/stats/create_stats_job_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/testutils"
"github.com/cockroachdb/cockroach/pkg/testutils/jobutils"
"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
"github.com/cockroachdb/cockroach/pkg/testutils/skip"
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
"github.com/cockroachdb/cockroach/pkg/testutils/testcluster"
"github.com/cockroachdb/cockroach/pkg/util/encoding"
Expand Down Expand Up @@ -128,7 +127,6 @@ func TestCreateStatsControlJob(t *testing.T) {
func TestCreateStatisticsCanBeCancelled(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)
skip.UnderStress(t, "test can be slow to quiesce because of filter")

var allowRequest chan struct{}

Expand All @@ -145,7 +143,7 @@ func TestCreateStatisticsCanBeCancelled(t *testing.T) {
sqlDB := sqlutils.MakeSQLRunner(conn)

sqlDB.Exec(t, `CREATE DATABASE d`)
sqlDB.Exec(t, `CREATE TABLE d.t (x INT PRIMARY KEY)`)
sqlDB.Exec(t, `CREATE TABLE d.t (x INT PRIMARY KEY) WITH (sql_stats_automatic_collection_enabled = false)`)
sqlDB.Exec(t, `INSERT INTO d.t SELECT generate_series(1,1000)`)
var tID descpb.ID
sqlDB.QueryRow(t, `SELECT 'd.t'::regclass::int`).Scan(&tID)
Expand All @@ -159,6 +157,7 @@ func TestCreateStatisticsCanBeCancelled(t *testing.T) {
errCh <- err
}()
allowRequest <- struct{}{}
setTableID(descpb.InvalidID)
testutils.SucceedsSoon(t, func() error {
row := conn.QueryRow("SELECT query_id FROM [SHOW CLUSTER STATEMENTS] WHERE query LIKE 'CREATE STATISTICS%';")
var queryID string
Expand All @@ -168,9 +167,20 @@ func TestCreateStatisticsCanBeCancelled(t *testing.T) {
_, err := conn.Exec("CANCEL QUERIES VALUES ((SELECT query_id FROM [SHOW CLUSTER STATEMENTS] WHERE query LIKE 'CREATE STATISTICS%'));")
return err
})
err := <-errCh
allowRequest <- struct{}{}

// Allow the filter to pass everything an error is received.
var err error
testutils.SucceedsSoon(t, func() error {
// Assume something will fail.
err = errors.AssertionFailedf("failed for create stats to cancel")
for {
select {
case err = <-errCh:
return nil
case allowRequest <- struct{}{}:
}
}
})
close(allowRequest)
require.ErrorContains(t, err, "pq: query execution canceled")
}

Expand Down Expand Up @@ -214,7 +224,6 @@ func TestAtMostOneRunningCreateStats(t *testing.T) {
case err := <-errCh:
t.Fatal(err)
}

autoStatsRunShouldFail := func() {
_, err := conn.Exec(`CREATE STATISTICS __auto__ FROM d.t`)
expected := "another CREATE STATISTICS job is already running"
Expand Down Expand Up @@ -534,7 +543,7 @@ func createStatsRequestFilter(
if req, ok := ba.GetArg(kvpb.Scan); ok {
_, tableID, _ := encoding.DecodeUvarintAscending(req.(*kvpb.ScanRequest).Key)
// Ensure that the tableID is what we expect it to be.
if tableID > 0 && descpb.ID(tableID) == tableToBlock.Load().(descpb.ID) {
if tableID > 0 && descpb.ID(tableID) == tableToBlock.Load() {
// Read from the channel twice to allow jobutils.RunJob to complete
// even though there is only one ScanRequest.
<-*allowProgressIota
Expand Down

0 comments on commit a8bb526

Please sign in to comment.