Skip to content

Commit 738c039

Browse files
craig[bot]spilchen
andcommitted
Merge #155358
155358: sql/inspect: serialize inspect progress metadata pushes r=spilchen a=spilchen Concurrent inspect processors were racing when calling DistSQLReceiver.Push, which is not concurrency-safe. This commit introduces a per-processor mutex to serialize all Push calls. Resolves: #155309, resolves #155311, resolves #155312, resolves #155313, resolves #155314, resolves #155315 Epic: CRDB-30356 Release note: none Co-authored-by: Matt Spilchen <matt.spilchen@cockroachlabs.com>
2 parents 199a469 + 63621e6 commit 738c039

File tree

1 file changed

+20
-2
lines changed

1 file changed

+20
-2
lines changed

pkg/sql/inspect/inspect_processor.go

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import (
2424
"github.com/cockroachdb/cockroach/pkg/util/ctxgroup"
2525
"github.com/cockroachdb/cockroach/pkg/util/hlc"
2626
"github.com/cockroachdb/cockroach/pkg/util/log"
27+
"github.com/cockroachdb/cockroach/pkg/util/syncutil"
2728
"github.com/cockroachdb/cockroach/pkg/util/tracing"
2829
"github.com/cockroachdb/errors"
2930
pbtypes "github.com/gogo/protobuf/types"
@@ -89,6 +90,12 @@ type inspectProcessor struct {
8990
logger inspectLogger
9091
concurrency int
9192
clock *hlc.Clock
93+
mu struct {
94+
// Guards calls to output.Push because DistSQLReceiver.Push is not
95+
// concurrency safe and progress metadata can be emitted from multiple
96+
// worker goroutines.
97+
syncutil.Mutex
98+
}
9299
}
93100

94101
var _ execinfra.Processor = (*inspectProcessor)(nil)
@@ -329,7 +336,7 @@ func (p *inspectProcessor) sendInspectProgress(
329336
},
330337
}
331338

332-
output.Push(nil, meta)
339+
p.pushProgressMeta(output, meta)
333340
return nil
334341
}
335342

@@ -369,10 +376,21 @@ func (p *inspectProcessor) sendSpanCompletionProgress(
369376
},
370377
}
371378

372-
output.Push(nil, meta)
379+
p.pushProgressMeta(output, meta)
373380
return nil
374381
}
375382

383+
// pushProgressMeta serializes metadata pushes so only one goroutine interacts
384+
// with the DistSQL receiver at a time (DistSQLReceiver.Push is not concurrency
385+
// safe).
386+
func (p *inspectProcessor) pushProgressMeta(
387+
output execinfra.RowReceiver, meta *execinfrapb.ProducerMetadata,
388+
) {
389+
p.mu.Lock()
390+
defer p.mu.Unlock()
391+
output.Push(nil, meta)
392+
}
393+
376394
// newInspectProcessor constructs a new inspectProcessor from the given InspectSpec.
377395
// It parses the spec to generate a set of inspectCheck factories, sets up the span source,
378396
// and wires in logging and concurrency controls.

0 commit comments

Comments
 (0)