Skip to content

Commit

Permalink
sql: increase buffer on TestCaptureIndexUsageStats
Browse files Browse the repository at this point in the history
Tests can take longer to be completed, so a bigger time buffer
would be required to accomodate for the non-determinism in
the logging timings. This time buffer has been increasing every
couple of months and it would have to change to 5s now, which is
a big buffer compared to the actual value of 1s. The time it takes
is not really relevant for this test, as long as the schedules
are happening one after the other. So instead of increasing
the time buffer even more, this commit updates the test to confirm
that both the expected (which is calculated) and the actual value
are great or equal 0. This way the test is no longer flaky or would need
to be adjusted every few months, and we keep all the actual checks
important for the test.

Fixes #110617

Release note: None
  • Loading branch information
maryliag committed Sep 14, 2023
1 parent 0cde11b commit f774b6d
Showing 1 changed file with 3 additions and 9 deletions.
12 changes: 3 additions & 9 deletions pkg/sql/scheduledlogging/captured_index_usage_stats_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,6 @@ func TestCaptureIndexUsageStats(t *testing.T) {
stubScheduleCheckEnabledInterval := time.Second
stubLoggingDelay := 0 * time.Second

// timeBuffer is a short time buffer to account for non-determinism in the logging timings.
const timeBuffer = 4 * time.Second

schedulesFinishedChan := make(chan struct{})
logsVerifiedChan := make(chan struct{})

Expand All @@ -90,7 +87,7 @@ func TestCaptureIndexUsageStats(t *testing.T) {
getLoggingDuration: sd.getLoggingDuration,
getOverlapDuration: sd.getOverlapDuration,
onScheduleComplete: func() {
// Notify that the scheudle has completed.
// Notify that the schedule has completed.
schedulesFinishedChan <- struct{}{}
// Wait for logs to be verified before proceeding.
<-logsVerifiedChan
Expand Down Expand Up @@ -174,10 +171,8 @@ func TestCaptureIndexUsageStats(t *testing.T) {

// Enable capture of index usage stats.
telemetryCaptureIndexUsageStatsEnabled.Override(context.Background(), &tenantSettings.SV, true)
//t.Fatal(telemetryCaptureIndexUsageStatsEnabled.Get(&s.ClusterSettings().SV))

// Check that telemetry log file contains all the entries we're expecting, at the scheduled intervals.

expectedTotalNumEntriesInSingleInterval := 8
expectedNumberOfIndividualIndexEntriesInSingleInterval := 1

Expand Down Expand Up @@ -284,9 +279,8 @@ func TestCaptureIndexUsageStats(t *testing.T) {
}

actualDuration := time.Duration(currentTimestamp - previousTimestamp)
// Use a time window to afford some non-determinism in the test.
require.Greaterf(t, expectedDuration, actualDuration-timeBuffer, "%v <= %v-%v", expectedDuration, actualDuration, timeBuffer)
require.Greater(t, actualDuration+timeBuffer, expectedDuration, "%v+%v <= %v", expectedDuration, actualDuration, timeBuffer)
require.GreaterOrEqual(t, expectedDuration, 0*time.Second, "expectedDuration %v should be >= 0")
require.GreaterOrEqual(t, actualDuration, 0*time.Second, "actualDuration %v should be >= 0")
previousTimestamp = currentTimestamp
}

Expand Down

0 comments on commit f774b6d

Please sign in to comment.