Skip to content

Commit

Permalink
sql: deflake TestStatusAPIStatements and TestStatusAPICombinedStatements
Browse files Browse the repository at this point in the history
Fixes: cockroachdb#69557

As mentioned in cockroachdb#69533, we have a race condition in tests
where we request statements stats with start=now. In these
tests we expect to see no results, but because in-memory
stats have the aggregated_ts field set on iterator return,
depending on the time the test is run we might see
results returned.

For example, suppose we insert stats at 15:45. If we then request
stats at 16:00, requesting only stats aggregated after or at the
current time, i.e. start=16:00, the aggregated_ts for in-memory
stats will be set to 16:00 and thus we will see results
returned.

To deflake these tests, we stub the aggregated_ts field to
a predetermined value.

Release justification: non-production code changes
Release note: None
  • Loading branch information
xinhaoz committed Aug 30, 2021
1 parent b78f3fc commit c724737
Showing 1 changed file with 28 additions and 11 deletions.
39 changes: 28 additions & 11 deletions pkg/server/status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1813,9 +1813,17 @@ func TestStatusAPIStatements(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

params, _ := tests.CreateTestServerParams()
// Aug 30 2021 19:50:00 GMT+0000
aggregatedTs := int64(1630353000)
testCluster := serverutils.StartNewTestCluster(t, 3, base.TestClusterArgs{
ServerArgs: params,
ServerArgs: base.TestServerArgs{
Knobs: base.TestingKnobs{
SQLStatsKnobs: &sqlstats.TestingKnobs{
AOSTClause: "AS OF SYSTEM TIME '-1us'",
StubTimeNow: func() time.Time { return timeutil.Unix(aggregatedTs, 0) },
},
},
},
})
defer testCluster.Stopper().Stop(context.Background())

Expand Down Expand Up @@ -1897,17 +1905,24 @@ func TestStatusAPIStatements(t *testing.T) {
// Test no params
testPath("statements", expectedStatements)
// Test combined=true forwards to CombinedStatements
nowInSecs := timeutil.Now().Unix()
testPath(fmt.Sprintf("statements?combined=true&start=%d", nowInSecs), nil)
testPath(fmt.Sprintf("statements?combined=true&start=%d", aggregatedTs+60), nil)
}

func TestStatusAPICombinedStatements(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

params, _ := tests.CreateTestServerParams()
// Aug 30 2021 19:50:00 GMT+0000
aggregatedTs := int64(1630353000)
testCluster := serverutils.StartNewTestCluster(t, 3, base.TestClusterArgs{
ServerArgs: params,
ServerArgs: base.TestServerArgs{
Knobs: base.TestingKnobs{
SQLStatsKnobs: &sqlstats.TestingKnobs{
AOSTClause: "AS OF SYSTEM TIME '-1us'",
StubTimeNow: func() time.Time { return timeutil.Unix(aggregatedTs, 0) },
},
},
},
})
defer testCluster.Stopper().Stop(context.Background())

Expand Down Expand Up @@ -1989,11 +2004,13 @@ func TestStatusAPICombinedStatements(t *testing.T) {
// Test with no query params
testPath("combinedstmts", expectedStatements)

nowInSecs := timeutil.Now().Unix()
// Test with end = now; should give the same results as get all.
testPath(fmt.Sprintf("combinedstmts?end=%d", nowInSecs), expectedStatements)
// Test with start = 1 hour ago end = now; should give the same results as get all.
testPath(fmt.Sprintf("combinedstmts?start=%d&end=%d", nowInSecs-3600, nowInSecs), expectedStatements)
oneMinAfterAggregatedTs := aggregatedTs + 60
// Test with end = 1 min after aggregatedTs; should give the same results as get all.
testPath(fmt.Sprintf("combinedstmts?end=%d", oneMinAfterAggregatedTs), expectedStatements)
// Test with start = 1 hour before aggregatedTs end = 1 min after aggregatedTs; should give same results as get all.
testPath(fmt.Sprintf("combinedstmts?start=%d&end=%d", aggregatedTs-3600, oneMinAfterAggregatedTs), expectedStatements)
// Test with start = 1 min after aggregatedTs; should give no results
testPath(fmt.Sprintf("combinedstmts?start=%d", oneMinAfterAggregatedTs), nil)
}

func TestListSessionsSecurity(t *testing.T) {
Expand Down

0 comments on commit c724737

Please sign in to comment.