From b45ba50816f3c9e3eba52b080f4f9fe622ec5f9c Mon Sep 17 00:00:00 2001 From: maryliag Date: Wed, 13 Sep 2023 23:44:02 -0300 Subject: [PATCH] sql: fix TestCaptureIndexUsageStats 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 #110617 Release note: None --- .../captured_index_usage_stats_test.go | 39 +------------------ 1 file changed, 1 insertion(+), 38 deletions(-) diff --git a/pkg/sql/scheduledlogging/captured_index_usage_stats_test.go b/pkg/sql/scheduledlogging/captured_index_usage_stats_test.go index 27e6e6950392..bdab0ad5066c 100644 --- a/pkg/sql/scheduledlogging/captured_index_usage_stats_test.go +++ b/pkg/sql/scheduledlogging/captured_index_usage_stats_test.go @@ -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{}) @@ -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 @@ -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 @@ -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