Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: data race introduced in #94438 #95937

Closed
cockroach-teamcity opened this issue Jan 26, 2023 · 7 comments · Fixed by #96041
Closed

sql: data race introduced in #94438 #95937

cockroach-teamcity opened this issue Jan 26, 2023 · 7 comments · Fixed by #96041
Assignees
Labels
A-sql-execution Relating to SQL execution. branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-robot Originated from a bot. regression Regression from a release. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked.
Milestone

Comments

@cockroach-teamcity
Copy link
Member

cockroach-teamcity commented Jan 26, 2023

cli/clisqlshell.Example_sql_config failed with artifacts on master @ 2ad8df3df3272110705984efc32f1453631ce602:

WARNING: DATA RACE
Write at 0x00c0038c8d00 by goroutine 20635:
  github.com/cockroachdb/cockroach/pkg/sql/physicalplan.releaseTableReaderSpec()
      github.com/cockroachdb/cockroach/pkg/sql/physicalplan/specs.go:63 +0xf7
  github.com/cockroachdb/cockroach/pkg/sql/physicalplan.ReleaseFlowSpec()
      github.com/cockroachdb/cockroach/pkg/sql/physicalplan/specs.go:40 +0x145
  github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).Run.func3()
      github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:673 +0x39
  runtime.deferreturn()
      GOROOT/src/runtime/panic.go:476 +0x32
  github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).PlanAndRun()
      github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:1793 +0x2ec
  github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).PlanAndRunAll()
      github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:1544 +0x34b
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execWithDistSQLEngine()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:1638 +0xa94
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).dispatchToExecutionEngine()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:1259 +0x1624
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmtInOpenState()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:744 +0x3a91
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmt.func1()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:130 +0x18e
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execWithProfiling()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:2527 +0x50f
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmt()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:129 +0x6cd
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execPortal()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:219 +0x217
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execCmd.func2()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:2038 +0xad7
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execCmd()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:2040 +0xbd6
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).run()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1882 +0x424
  github.com/cockroachdb/cockroach/pkg/sql.(*InternalExecutor).runWithEx.func1()
      github.com/cockroachdb/cockroach/pkg/sql/internal.go:173 +0x117

Previous read at 0x00c0038c8d04 by goroutine 20206:
  reflect.Value.Uint()
      GOROOT/src/reflect/value.go:2584 +0x195
  encoding/json.uintEncoder()
      GOROOT/src/encoding/json/encode.go:562 +0x45
  encoding/json.structEncoder.encode()
      GOROOT/src/encoding/json/encode.go:760 +0x2ba
  encoding/json.structEncoder.encode-fm()
      <autogenerated>:1 +0xdb
  encoding/json.ptrEncoder.encode()
      GOROOT/src/encoding/json/encode.go:944 +0x3c2
  encoding/json.ptrEncoder.encode-fm()
      <autogenerated>:1 +0x90
  encoding/json.structEncoder.encode()
      GOROOT/src/encoding/json/encode.go:760 +0x2ba
  encoding/json.structEncoder.encode-fm()
      <autogenerated>:1 +0xdb
  encoding/json.structEncoder.encode()
      GOROOT/src/encoding/json/encode.go:760 +0x2ba
  encoding/json.structEncoder.encode-fm()
      <autogenerated>:1 +0xdb
  encoding/json.ptrEncoder.encode()
      GOROOT/src/encoding/json/encode.go:944 +0x3c2
  encoding/json.ptrEncoder.encode-fm()
      <autogenerated>:1 +0x90
  encoding/json.(*encodeState).reflectValue()
      GOROOT/src/encoding/json/encode.go:359 +0x88
  encoding/json.(*encodeState).marshal()
      GOROOT/src/encoding/json/encode.go:331 +0x20b
  encoding/json.(*Encoder).Encode()
      GOROOT/src/encoding/json/stream.go:206 +0xcf
  github.com/cockroachdb/cockroach/pkg/kv/kvclient/kvcoord.GRPCTransportFactory.func1()
      github.com/cockroachdb/cockroach/pkg/kv/kvclient/kvcoord/transport_race.go:120 +0x2c4
  github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunAsyncTaskEx.func2()
      github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go:470 +0x1f6

Parameters: TAGS=bazel,gss,race

Help

See also: How To Investigate a Go Test Failure (internal)

/cc @cockroachdb/sql-sessions @cockroachdb/server

This test on roachdash | Improve this report!

Jira issue: CRDB-23834

@cockroach-teamcity cockroach-teamcity added branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-robot Originated from a bot. labels Jan 26, 2023
@cockroach-teamcity cockroach-teamcity added this to the 23.1 milestone Jan 26, 2023
@blathers-crl blathers-crl bot added the T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) label Jan 26, 2023
@knz knz added regression Regression from a release. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. labels Jan 26, 2023
@blathers-crl blathers-crl bot added the T-sql-schema-deprecated Use T-sql-foundations instead label Jan 26, 2023
@knz knz added A-telemetry and removed T-sql-foundations SQL Foundations Team (formerly SQL Schema + SQL Sessions) labels Jan 26, 2023
@knz

This comment was marked as outdated.

@knz

This comment was marked as resolved.

@knz knz added A-sql-execution Relating to SQL execution. T-sql-execution and removed A-telemetry T-sql-schema-deprecated Use T-sql-foundations instead labels Jan 26, 2023
@blathers-crl blathers-crl bot added the T-sql-queries SQL Queries Team label Jan 26, 2023
@knz
Copy link
Contributor

knz commented Jan 26, 2023

cc @yuzefovich there's a data race in distsql physical planning

@knz
Copy link
Contributor

knz commented Jan 26, 2023

Another one here #95916

WARNING: DATA RACE
Write at 0x00c002c2da00 by goroutine 4339:
  github.com/cockroachdb/cockroach/pkg/sql.initTableReaderSpecTemplate()
      github.com/cockroachdb/cockroach/pkg/sql/distsql_physical_planner.go:1530 +0x3c4
  github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).createTableReaders()
      github.com/cockroachdb/cockroach/pkg/sql/distsql_physical_planner.go:1554 +0xe8
  github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).createPhysPlanForPlanNode()
      github.com/cockroachdb/cockroach/pkg/sql/distsql_physical_planner.go:3350 +0x304
  github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).createPhysPlan()
      github.com/cockroachdb/cockroach/pkg/sql/distsql_physical_planner.go:3254 +0xcd
  github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).PlanAndRun()
      github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:1785 +0x16e
  github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).PlanAndRunAll()
      github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:1544 +0x34b
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execWithDistSQLEngine()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:1638 +0xa94
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).dispatchToExecutionEngine()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:1259 +0x1624
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmtInOpenState()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:744 +0x3a91
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmt.func1()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:130 +0x18e
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execWithProfiling()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:2527 +0x50f
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmt()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:129 +0x6cd
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execCmd.func1()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1959 +0x7c4
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execCmd()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1964 +0x1cba
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).run()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1882 +0x424
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).run()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1882 +0x424
  github.com/cockroachdb/cockroach/pkg/sql.(*Server).ServeConn()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:836 +0x1be
  github.com/cockroachdb/cockroach/pkg/sql/pgwire.(*conn).processCommandsAsync.func1()
      github.com/cockroachdb/cockroach/pkg/sql/pgwire/conn.go:731 +0x572

Previous read at 0x00c002c2da00 by goroutine 2276:
  reflect.Value.Uint()
      GOROOT/src/reflect/value.go:2584 +0x195
  encoding/json.uintEncoder()
      GOROOT/src/encoding/json/encode.go:562 +0x45
  encoding/json.structEncoder.encode()
      GOROOT/src/encoding/json/encode.go:760 +0x2ba
  encoding/json.structEncoder.encode-fm()
      <autogenerated>:1 +0xdb
  encoding/json.ptrEncoder.encode()
      GOROOT/src/encoding/json/encode.go:944 +0x3c2
  encoding/json.ptrEncoder.encode-fm()
      <autogenerated>:1 +0x90
  encoding/json.structEncoder.encode()
      GOROOT/src/encoding/json/encode.go:760 +0x2ba
  encoding/json.structEncoder.encode-fm()
      <autogenerated>:1 +0xdb
  encoding/json.structEncoder.encode()
      GOROOT/src/encoding/json/encode.go:760 +0x2ba
  encoding/json.structEncoder.encode-fm()
      <autogenerated>:1 +0xdb
  encoding/json.ptrEncoder.encode()
      GOROOT/src/encoding/json/encode.go:944 +0x3c2
  encoding/json.ptrEncoder.encode-fm()
      <autogenerated>:1 +0x90
  encoding/json.(*encodeState).reflectValue()
      GOROOT/src/encoding/json/encode.go:359 +0x88
  encoding/json.(*encodeState).marshal()
      GOROOT/src/encoding/json/encode.go:331 +0x20b
  encoding/json.(*Encoder).Encode()
      GOROOT/src/encoding/json/stream.go:206 +0xcf
  github.com/cockroachdb/cockroach/pkg/kv/kvclient/kvcoord.GRPCTransportFactory.func1()
      github.com/cockroachdb/cockroach/pkg/kv/kvclient/kvcoord/transport_race.go:120 +0x2c4
  github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunAsyncTaskEx.func2()
      github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go:470 +0x1f6

@knz
Copy link
Contributor

knz commented Jan 26, 2023

Another one here #95917

==================
WARNING: DATA RACE
Read at 0x00c00164bc00 by goroutine 8528:
  reflect.Value.Uint()
      GOROOT/src/reflect/value.go:2584 +0x195
  encoding/json.uintEncoder()
      GOROOT/src/encoding/json/encode.go:562 +0x45
  encoding/json.structEncoder.encode()
      GOROOT/src/encoding/json/encode.go:760 +0x2ba
  encoding/json.structEncoder.encode-fm()
      <autogenerated>:1 +0xdb
  encoding/json.ptrEncoder.encode()
      GOROOT/src/encoding/json/encode.go:944 +0x3c2
  encoding/json.ptrEncoder.encode-fm()
      <autogenerated>:1 +0x90
  encoding/json.structEncoder.encode()
      GOROOT/src/encoding/json/encode.go:760 +0x2ba
  encoding/json.structEncoder.encode-fm()
      <autogenerated>:1 +0xdb
  encoding/json.structEncoder.encode()
      GOROOT/src/encoding/json/encode.go:760 +0x2ba
  encoding/json.structEncoder.encode-fm()
      <autogenerated>:1 +0xdb
  encoding/json.ptrEncoder.encode()
      GOROOT/src/encoding/json/encode.go:944 +0x3c2
  encoding/json.ptrEncoder.encode-fm()
      <autogenerated>:1 +0x90
  encoding/json.(*encodeState).reflectValue()
      GOROOT/src/encoding/json/encode.go:359 +0x88
  encoding/json.(*encodeState).marshal()
      GOROOT/src/encoding/json/encode.go:331 +0x20b
  encoding/json.(*Encoder).Encode()
      GOROOT/src/encoding/json/stream.go:206 +0xcf
  github.com/cockroachdb/cockroach/pkg/kv/kvclient/kvcoord.GRPCTransportFactory.func1()
      github.com/cockroachdb/cockroach/pkg/kv/kvclient/kvcoord/transport_race.go:120 +0x2c4
  github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunAsyncTaskEx.func2()
      github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go:470 +0x1f6

Previous write at 0x00c00164bc00 by goroutine 10679:
  github.com/cockroachdb/cockroach/pkg/sql/rowenc.InitIndexFetchSpec()
      github.com/cockroachdb/cockroach/pkg/sql/rowenc/index_fetch.go:38 +0x2c4
  github.com/cockroachdb/cockroach/pkg/sql.initTableReaderSpecTemplate()
      github.com/cockroachdb/cockroach/pkg/sql/distsql_physical_planner.go:1536 +0x4a8
  github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).createTableReaders()
      github.com/cockroachdb/cockroach/pkg/sql/distsql_physical_planner.go:1554 +0xe8
  github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).createPhysPlanForPlanNode()
      github.com/cockroachdb/cockroach/pkg/sql/distsql_physical_planner.go:3350 +0x304
  github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).createPhysPlanForPlanNode()
      github.com/cockroachdb/cockroach/pkg/sql/distsql_physical_planner.go:3340 +0x373
  github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).createPhysPlan()
      github.com/cockroachdb/cockroach/pkg/sql/distsql_physical_planner.go:3254 +0xcd
  github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).PlanAndRun()
      github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:1785 +0x16e
  github.com/cockroachdb/cockroach/pkg/sql.(*DistSQLPlanner).PlanAndRunAll()
      github.com/cockroachdb/cockroach/pkg/sql/distsql_running.go:1544 +0x34b
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execWithDistSQLEngine()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:1638 +0xa94
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).dispatchToExecutionEngine()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:1259 +0x1624
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmtInOpenState()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:744 +0x3a91
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmt.func1()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:130 +0x18e
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execWithProfiling()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:2527 +0x50f
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmt()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:129 +0x6cd
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execPortal()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:219 +0x217
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execCmd.func2()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:2038 +0xad7
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execCmd()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:2040 +0xbd6
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).run()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:1882 +0x424
  github.com/cockroachdb/cockroach/pkg/sql.(*InternalExecutor).runWithEx.func1()
      github.com/cockroachdb/cockroach/pkg/sql/internal.go:173 +0x117

craig bot pushed a commit that referenced this issue Jan 26, 2023
95839: colinfo: add missing type families into CanHaveCompositeKeyEncoding r=yuzefovich a=yuzefovich

This commit adds several missing type families into `CanHaveCompositeKeyEncoding` method. Some of these type families are internal and, probably, don't need to be handled, but we recently introduced TSVector and TSQuery types which were incorrectly marked as being composite since they were not mentioned explicitly in the type family switch. This commit also makes it so that we panic in this method if we forget to include a newly-introduced type into this switch.

Fixes: #95680.

Release note: None

95900: ui: add check for cpu usage r=maryliag a=maryliag

Add a check, for cases when the value might no be
returned (cluster with mixed versions).

Part Of #87213

Release note: None

96005: colfetcher: disable direct columnar scans for now r=yuzefovich a=yuzefovich

This commit disables direct columnar scans which are now randomly enabled in tests due to this feature having a data race at the moment.

Informs: #95937.

Release note: None

96008: roachtest/awsdms: fix race condition that can cause panics r=otan a=Jeremyyang920

In #95518, we added a new test case to check for a table error during the DMS process. However there is a race when the check happens depending on how quickly tasks have ran and the task ReplicationTaskStats can be nil and we tried to access field on the nil object that was returned from the API call.

This commit checks that the ReplicationTaskStat is not nil before accessing the TablesErrored property.

Fixes: #93305
Release note: None

96009: binfetcher: fix binary downloading for arm64 MacOS r=rail a=andyyang890

This patch updates the suffix used to fetch arm64 MacOS binaries
to match the naming scheme used to build releases.

Epic: None

Release note: None

96020: roachtest: use teardown log when creating GitHub issue r=herkolategan a=renatolabs

This is a follow up of #95831. The logger passed to the `githubIssues`
struct writes to the test runner logger, which is not ideal. This
changes the logger passed to use the `teardown` logger, so log entries
related to GitHub issue creation are in the same directory as the
failing test itself.

Epic: None

Release note: None

Co-authored-by: Yahor Yuzefovich <[email protected]>
Co-authored-by: maryliag <[email protected]>
Co-authored-by: Jeremy Yang <[email protected]>
Co-authored-by: Andy Yang <[email protected]>
Co-authored-by: Renato Costa <[email protected]>
@craig craig bot closed this as completed in 90e24ab Jan 27, 2023
@exalate-issue-sync exalate-issue-sync bot removed the T-sql-queries SQL Queries Team label Jan 27, 2023
@mgartner mgartner moved this to Done in SQL Queries Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sql-execution Relating to SQL execution. branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-robot Originated from a bot. regression Regression from a release. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants