Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Commit 7a138af

Browse files
authored
event_logs, telemetry_export: do not interrupt insert on context cancel (#57552)
When a request context is cancelled, the [corresponding event database insert can fail on context cancellation](https://sourcegraph.slack.com/archives/C03V51C8K53/p1697072391851359) - but this is highly unlikely to be desirable behaviour, since we probably want to proceed with persisting the event regardless. This change introduces a new helper in `internal/trace`, `BackgroundContext`, which aliases `CopyContext(context.Background(), from)` with some more docstrings on when to use it and how to use it correctly. We use this new background context in the most inner part of `(TelemetryEventsExportQueueStore).QueueForExport` and `(EventLogStore).BulkInsert` to ensure we don't interrupt database inserts.
1 parent afd3156 commit 7a138af

File tree

3 files changed

+25
-2
lines changed

3 files changed

+25
-2
lines changed

internal/database/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@ go_library(
149149
"//internal/trace",
150150
"//internal/types",
151151
"//internal/version",
152+
"//internal/xcontext",
152153
"//lib/errors",
153154
"//lib/pointers",
154155
"//schema",

internal/database/event_logs.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/sourcegraph/sourcegraph/internal/trace"
2727
"github.com/sourcegraph/sourcegraph/internal/types"
2828
"github.com/sourcegraph/sourcegraph/internal/version"
29+
"github.com/sourcegraph/sourcegraph/internal/xcontext"
2930
"github.com/sourcegraph/sourcegraph/lib/errors"
3031
)
3132

@@ -45,6 +46,10 @@ type EventLogStore interface {
4546
// AggregatedSearchEvents calculates SearchAggregatedEvent for each every unique event type related to search.
4647
AggregatedSearchEvents(ctx context.Context, now time.Time) ([]types.SearchAggregatedEvent, error)
4748

49+
// BulkInsert inserts a set of events.
50+
//
51+
// It does NOT respect context cancellation, as it is assumed that we never
52+
// drop events once we attempt to queue it for export.
4853
BulkInsert(ctx context.Context, events []*Event) error
4954

5055
// CodeIntelligenceCrossRepositoryWAUs returns the WAU (current week) with any (precise or search-based) cross-repository code intelligence event.
@@ -289,8 +294,14 @@ func (l *eventLogStore) BulkInsert(ctx context.Context, events []*Event) error {
289294
}
290295
close(rowValues)
291296

297+
// Create a cancel-free context to avoid interrupting the insert when
298+
// the parent context is cancelled, and add our own timeout on the insert
299+
// to make sure things don't get stuck in an unbounded manner.
300+
insertCtx, cancel := context.WithTimeout(xcontext.Detach(ctx), 5*time.Minute)
301+
defer cancel()
302+
292303
return batch.InsertValues(
293-
ctx,
304+
insertCtx,
294305
l.Handle(),
295306
"event_logs",
296307
batch.MaxNumPostgresParameters,

internal/database/telemetry_export_store.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/sourcegraph/sourcegraph/internal/telemetry/sensitivemetadataallowlist"
2020
telemetrygatewayv1 "github.com/sourcegraph/sourcegraph/internal/telemetrygateway/v1"
2121
"github.com/sourcegraph/sourcegraph/internal/trace"
22+
"github.com/sourcegraph/sourcegraph/internal/xcontext"
2223
"github.com/sourcegraph/sourcegraph/lib/errors"
2324
)
2425

@@ -40,6 +41,9 @@ type TelemetryEventsExportQueueStore interface {
4041
// feature-flagged, such that if the flag is not enabled for the given
4142
// context, we do not cache the event for export.
4243
//
44+
// It does NOT respect context cancellation, as it is assumed that we never
45+
// drop events once we attempt to queue it for export.
46+
//
4347
// 🚨 SECURITY: The implementation strips out sensitive contents from events
4448
// that are not in sensitivemetadataallowlist.AllowedEventTypes().
4549
QueueForExport(context.Context, []*telemetrygatewayv1.Event) error
@@ -91,7 +95,14 @@ func (s *telemetryEventsExportQueueStore) QueueForExport(ctx context.Context, ev
9195
return nil
9296
}
9397

94-
err := batch.InsertValues(ctx,
98+
// Create a cancel-free context to avoid interrupting the insert when
99+
// the parent context is cancelled, and add our own timeout on the insert
100+
// to make sure things don't get stuck in an unbounded manner.
101+
insertCtx, cancel := context.WithTimeout(xcontext.Detach(ctx), 5*time.Minute)
102+
defer cancel()
103+
104+
err := batch.InsertValues(
105+
insertCtx,
95106
s.Handle(),
96107
"telemetry_events_export_queue",
97108
batch.MaxNumPostgresParameters,

0 commit comments

Comments
 (0)