Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
110639: sql: fix TestCaptureIndexUsageStats r=maryliag a=maryliag

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
removes those checks. 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 cockroachdb#110617

Release note: None

Co-authored-by: maryliag <[email protected]>
  • Loading branch information
craig[bot] and maryliag committed Sep 15, 2023
2 parents 28552c8 + b45ba50 commit c3e6bb2
Showing 1 changed file with 1 addition and 38 deletions.
39 changes: 1 addition & 38 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 @@ -258,38 +253,6 @@ func TestCaptureIndexUsageStats(t *testing.T) {
return entries[a].Time < entries[b].Time
})

testData := []time.Duration{
0 * time.Second,
// the difference in number of seconds between first and second schedule
stubScheduleInterval - firstScheduleLoggingDuration,
// the difference in number of seconds between second and third schedule
sd.getOverlapDuration(),
}

var (
previousTimestamp = int64(0)
currentTimestamp = int64(0)
)

// Check the timestamp differences between schedules.
for idx, expectedDuration := range testData {
entriesLowerBound := idx * expectedTotalNumEntriesInSingleInterval
entriesUpperBound := (idx + 1) * expectedTotalNumEntriesInSingleInterval
scheduleEntryBlock := entries[entriesLowerBound:entriesUpperBound]
// Take the first log entry from the schedule.
currentTimestamp = scheduleEntryBlock[0].Time
// If this is the first iteration, initialize the previous timestamp.
if idx == 0 {
previousTimestamp = currentTimestamp
}

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)
previousTimestamp = currentTimestamp
}

}

// checkNumTotalEntriesAndNumIndexEntries is a helper function that verifies that
Expand Down

0 comments on commit c3e6bb2

Please sign in to comment.