Skip to content

Commit

Permalink
diagnostics: honor stopper quiescence when reporting diagnostics
Browse files Browse the repository at this point in the history
Previously, we would block on the HTTP diagnostics request until the
preset timeout regardless of stopper quiescence. This change ensures
that the `ctx` passed to the diagnostics reporter and the HTTP client
is cancelled if the stopper is quiescing.

Resolves: #136739

Release note: None
  • Loading branch information
dhartunian committed Dec 20, 2024
1 parent ff0001d commit f573cae
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 2 deletions.
33 changes: 32 additions & 1 deletion pkg/ccl/serverccl/diagnosticsccl/reporter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,14 @@ import (

const elemName = "somestring"

var setTelemetryHttpTimeout = func(newVal time.Duration) func() {
prior := diagnostics.TelemetryHttpTimeout
diagnostics.TelemetryHttpTimeout = newVal
return func() {
diagnostics.TelemetryHttpTimeout = prior
}
}

func TestTenantReport(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)
Expand Down Expand Up @@ -297,7 +305,7 @@ func TestTelemetry_SuccessfulTelemetryPing(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

diagnostics.TelemetryHttpTimeout = 3 * time.Second
defer setTelemetryHttpTimeout(3 * time.Second)()
rt := startReporterTest(t, base.TestIsSpecificToStorageLayerAndNeedsASystemTenant)
defer rt.Close()

Expand Down Expand Up @@ -366,6 +374,29 @@ func TestTelemetry_SuccessfulTelemetryPing(t *testing.T) {

}

// This test will block on `stopper.Stop` if the diagnostics reporter
// doesn't honor stopper quiescence when making its HTTP request.
func TestTelemetryQuiesce(t *testing.T) {
defer leaktest.AfterTest(t)()
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)

defer setTelemetryHttpTimeout(100 * time.Second)()
rt := startReporterTest(t, base.TestIsSpecificToStorageLayerAndNeedsASystemTenant)
defer rt.Close()

ctx := context.Background()
setupCluster(t, rt.serverDB)

defer rt.diagServer.SetWaitSeconds(200)()
dr := rt.server.DiagnosticsReporter().(*diagnostics.Reporter)
stopper := rt.server.Stopper()

dr.PeriodicallyReportDiagnostics(ctx, stopper)
stopper.Stop(ctx)
<-stopper.IsStopped()
}

func TestUsageQuantization(t *testing.T) {
defer leaktest.AfterTest(t)()
defer log.Scope(t).Close(t)
Expand Down
6 changes: 5 additions & 1 deletion pkg/server/diagnostics/reporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,11 @@ func (r *Reporter) PeriodicallyReportDiagnostics(ctx context.Context, stopper *s
// Consider something like rand.Float() > resetFreq/reportFreq here to sample
// stat reset periods for reporting.
if shouldReportDiagnostics(ctx, r.Settings) {
r.ReportDiagnostics(ctx)
func() {
ctx, cancel := stopper.WithCancelOnQuiesce(ctx)
defer cancel()
r.ReportDiagnostics(ctx)
}()
}

nextReport = nextReport.Add(reportFrequency.Get(&r.Settings.SV))
Expand Down

0 comments on commit f573cae

Please sign in to comment.