pkg/sql/sqlstats/insights: data race when running TestConsumeJoinToken
#85988
Assignees
Labels
A-sql-observability
Related to observability of the SQL layer
C-bug
Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior.
Comments
renatolabs
added
the
C-bug
Merged
matthewtodd
added
A-sql-observability
|
This may be related to #83080. |
renatolabs
added a commit
to renatolabs/cockroach
that referenced
this issue
Aug 11, 2022
Related issue: cockroachdb#85988. Release note: None.
|
I think the bug here is that the methods on |
|
Give this a shot: --- a/pkg/sql/sqlstats/insights/ingester.go
+++ b/pkg/sql/sqlstats/insights/ingester.go
@@ -55,7 +55,7 @@ type event struct {
statement *Statement
}
-func (i concurrentBufferIngester) Start(ctx context.Context, stopper *stop.Stopper) {
+func (i *concurrentBufferIngester) Start(ctx context.Context, stopper *stop.Stopper) {
// This task pulls buffers from the channel and forwards them along to the
// underlying Registry.
_ = stopper.RunAsyncTask(ctx, "insights-ingester", func(ctx context.Context) {
@@ -86,7 +86,7 @@ func (i concurrentBufferIngester) Start(ctx context.Context, stopper *stop.Stopp
})
}
-func (i concurrentBufferIngester) ingest(events *eventBuffer) {
+func (i *concurrentBufferIngester) ingest(events *eventBuffer) {
for _, e := range events {
// Because an eventBuffer is a fixed-size array, rather than a slice,
// we do not know how full it is until we hit a nil entry.
@@ -101,7 +101,7 @@ func (i concurrentBufferIngester) ingest(events *eventBuffer) {
}
}
-func (i concurrentBufferIngester) ObserveStatement(
+func (i *concurrentBufferIngester) ObserveStatement(
sessionID clusterunique.ID, statement *Statement,
) {
if !i.enabled() {
@@ -115,7 +115,7 @@ func (i concurrentBufferIngester) ObserveStatement(
})
}
-func (i concurrentBufferIngester) ObserveTransaction(
+func (i *concurrentBufferIngester) ObserveTransaction(
sessionID clusterunique.ID, transaction *Transaction,
) {
if !i.enabled() {
@@ -129,13 +129,13 @@ func (i concurrentBufferIngester) ObserveTransaction(
})
}
-func (i concurrentBufferIngester) IterateInsights(
+func (i *concurrentBufferIngester) IterateInsights(
ctx context.Context, visitor func(context.Context, *Insight),
) {
i.delegate.IterateInsights(ctx, visitor)
}
-func (i concurrentBufferIngester) enabled() bool {
+func (i *concurrentBufferIngester) enabled() bool {
return i.delegate.enabled()
} |
craig bot
pushed a commit
that referenced
this issue
Aug 16, 2022
86090: CODEOWNERS: stop dev-inf PR review requests for gen file list changes r=rickystewart a=jlinder Changes to most of the generated pkg/gen/*.bzl files happen for many PRs and they don't need to be reviewed by dev inf. Let's stop getting PR review requests for them. Release note: None Release justification: non-production code change 86146: insights: call ingester methods by reference r=matthewtodd a=matthewtodd Fixes the data race observed in #85988. Passing this data structure by value meant that the `fullHandler` was working with a different object than the external callers. Release justification: Category 3: Fixes for high-priority or high-severity bugs in existing functionality Release note: None 86191: sql/schemachanger/sctest: run the Backup test over all stages r=ajwerner a=ajwerner Before this change, we'd only run them over the non-revertible stages. Release justification: testing only change Release note: None Co-authored-by: James H. Linder <[email protected]> Co-authored-by: Matthew Todd <[email protected]> Co-authored-by: Andrew Werner <[email protected]>
|
This is fixed by #86146. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
#85350 introduced a data race observed when running the
TestConsumeJoinTokentest. The test has been failing pretty consistently since then (see TC history).Stack:
Jira issue: CRDB-18514
The text was updated successfully, but these errors were encountered: