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.

Sign up for GitHub

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

Jump to bottom

kv/kvserver: TestFlowControlBlockedAdmission failed #137493

Closed
cockroach-teamcity opened this issue Dec 14, 2024 · 4 comments
Closed

kv/kvserver: TestFlowControlBlockedAdmission failed #137493

cockroach-teamcity opened this issue Dec 14, 2024 · 4 comments
Assignees
@arjunmahishi
Labels
branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-robot Originated from a bot. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. T-observability

Comments

@cockroach-teamcity
Copy link
Member

cockroach-teamcity commented Dec 14, 2024 •
edited by cockroach-jira-scripts
Loading

kv/kvserver.TestFlowControlBlockedAdmission failed on master @ 49cff91f3501494deaf038671bc643c194a0e3ca:

      pkg/server/node.go:1483 +0x1ec
  github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunAsyncTaskEx.func2()
      pkg/util/stop/stopper.go:498 +0x338
  github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunAsyncTaskEx.gowrap1()
      pkg/util/stop/stopper.go:499 +0x4f

Goroutine 37119 (running) created at:
  github.com/cockroachdb/cockroach/pkg/sql/pgwire.(*Server).serveImpl()
      pkg/sql/pgwire/server.go:1194 +0xa4f
  github.com/cockroachdb/cockroach/pkg/sql/pgwire.(*Server).ServeConn()
      pkg/sql/pgwire/server.go:958 +0xe64
  github.com/cockroachdb/cockroach/pkg/server.(*systemServerWrapper).serveConn()
      pkg/server/server_controller_sql.go:170 +0x1d7
  github.com/cockroachdb/cockroach/pkg/server.(*serverController).sqlMux()
      pkg/server/server_controller_sql.go:90 +0x50b
  github.com/cockroachdb/cockroach/pkg/server.(*serverController).sqlMux-fm()
      <autogenerated>:1 +0xc4
  github.com/cockroachdb/cockroach/pkg/server.startServeSQL.func1.1()
      pkg/server/server_sql.go:1991 +0x39b
  github.com/cockroachdb/cockroach/pkg/util/netutil.(*TCPServer).ServeWith.func1()
      pkg/util/netutil/net.go:186 +0x1c6
  github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunAsyncTaskEx.func2()
      pkg/util/stop/stopper.go:498 +0x338
  github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunAsyncTaskEx.gowrap1()
      pkg/util/stop/stopper.go:499 +0x4f

Goroutine 32946 (running) created at:
  github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunAsyncTaskEx()
      pkg/util/stop/stopper.go:488 +0x72f
  github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunAsyncTask()
      pkg/util/stop/stopper.go:359 +0x253
  github.com/cockroachdb/cockroach/pkg/server.(*Node).startWriteNodeStatus()
      pkg/server/node.go:1464 +0x138
  github.com/cockroachdb/cockroach/pkg/server.(*topLevelServer).PreStart()
      pkg/server/server.go:1986 +0x52b0
  github.com/cockroachdb/cockroach/pkg/server.(*testServer).PreStart()
      pkg/server/testserver.go:794 +0x104
  github.com/cockroachdb/cockroach/pkg/testutils/serverutils.(*wrap).PreStart()
      bazel-out/k8-fastbuild/bin/pkg/testutils/serverutils/ts_control_forwarder_generated.go:19 +0x72
  github.com/cockroachdb/cockroach/pkg/testutils/testcluster.(*TestCluster).Start()
      pkg/testutils/testcluster/testcluster.go:442 +0x63e
  github.com/cockroachdb/cockroach/pkg/testutils/testcluster.StartTestCluster()
      pkg/testutils/testcluster/testcluster.go:259 +0xa4
  github.com/cockroachdb/cockroach/pkg/kv/kvserver_test.TestFlowControlBlockedAdmission()
      pkg/kv/kvserver/flow_control_integration_test.go:355 +0x511
  testing.tRunner()
      GOROOT/src/testing/testing.go:1689 +0x21e
  testing.(*T).Run.gowrap1()
      GOROOT/src/testing/testing.go:1742 +0x44
==================

Parameters:

  • attempt=1
  • race=true
  • run=1
  • shard=4
Help

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

/cc @cockroachdb/kv-triage

This test on roachdash | Improve this report!

Jira issue: CRDB-45611
@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. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. T-kv KV Team labels Dec 14, 2024
@nvanbenschoten
Copy link
Member

The test failed with the following data race:

==================
WARNING: DATA RACE
Write at 0x00c010c46100 by goroutine 37119:
  github.com/cockroachdb/cockroach/pkg/util/metric.(*vector).recordLabels()
      pkg/util/metric/metric.go:1276 +0x328
  github.com/cockroachdb/cockroach/pkg/util/metric.(*HistogramVec).Observe()
      pkg/util/metric/metric.go:1513 +0x224
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).recordStatementLatencyMetrics()
      pkg/sql/executor_statement_metrics.go:329 +0x517
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).recordStatementSummary()
      pkg/sql/executor_statement_metrics.go:145 +0x70b
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).dispatchToExecutionEngine()
      pkg/sql/conn_executor_exec.go:2901 +0x31fa
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmtInOpenState()
      pkg/sql/conn_executor_exec.go:1021 +0x5d34
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmt.func2()
      pkg/sql/conn_executor_exec.go:164 +0x184
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execWithProfiling()
      pkg/sql/conn_executor_exec.go:4325 +0x539
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmt()
      pkg/sql/conn_executor_exec.go:163 +0xe3d
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execCmd.func1()
      pkg/sql/conn_executor.go:2380 +0x864
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execCmd()
      pkg/sql/conn_executor.go:2385 +0xfef
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).run()
      pkg/sql/conn_executor.go:2302 +0x3ea
  github.com/cockroachdb/cockroach/pkg/sql.(*Server).ServeConn()
      pkg/sql/conn_executor.go:998 +0x216
  github.com/cockroachdb/cockroach/pkg/sql/pgwire.(*conn).processCommands()
      pkg/sql/pgwire/conn.go:252 +0x706
  github.com/cockroachdb/cockroach/pkg/sql/pgwire.(*Server).serveImpl.func4()
      pkg/sql/pgwire/server.go:1197 +0x21c
Previous read at 0x00c010c46100 by goroutine 32946:
  github.com/cockroachdb/cockroach/pkg/util/metric.(*HistogramVec).ToPrometheusMetrics()
      pkg/util/metric/metric.go:1545 +0x4c
  github.com/cockroachdb/cockroach/pkg/server/status.extractValue()
      recorder.go:740 +0x177
  github.com/cockroachdb/cockroach/pkg/server/status.(*MetricsRecorder).GenerateNodeStatus.eachRecordableValue.func7()
      recorder.go:763 +0x71
  github.com/cockroachdb/cockroach/pkg/util/metric.(*Registry).Each.func1()
      pkg/util/metric/registry.go:197 +0x63
  github.com/cockroachdb/cockroach/pkg/util/metric.(*HistogramVec).Inspect()
      pkg/util/metric/metric.go:1531 +0x3a
  github.com/cockroachdb/cockroach/pkg/util/metric.(*Registry).Each()
      pkg/util/metric/registry.go:196 +0x103
  github.com/cockroachdb/cockroach/pkg/server/status.eachRecordableValue()
      recorder.go:762 +0xb1b
  github.com/cockroachdb/cockroach/pkg/server/status.(*MetricsRecorder).GenerateNodeStatus()
      recorder.go:604 +0x9f3
  github.com/cockroachdb/cockroach/pkg/server.(*Node).writeNodeStatus.func1()
      pkg/server/node.go:1503 +0xad
  github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunTask()
      pkg/util/stop/stopper.go:316 +0x246
  github.com/cockroachdb/cockroach/pkg/server.(*Node).writeNodeStatus()
      pkg/server/node.go:1502 +0x135
  github.com/cockroachdb/cockroach/pkg/server.(*Node).startWriteNodeStatus.func2()
      pkg/server/node.go:1483 +0x1ec
  github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunAsyncTaskEx.func2()
      pkg/util/stop/stopper.go:498 +0x338
  github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunAsyncTaskEx.gowrap1()
      pkg/util/stop/stopper.go:499 +0x4f
Goroutine 37119 (running) created at:
  github.com/cockroachdb/cockroach/pkg/sql/pgwire.(*Server).serveImpl()
      pkg/sql/pgwire/server.go:1194 +0xa4f
  github.com/cockroachdb/cockroach/pkg/sql/pgwire.(*Server).ServeConn()
      pkg/sql/pgwire/server.go:958 +0xe64
  github.com/cockroachdb/cockroach/pkg/server.(*systemServerWrapper).serveConn()
      pkg/server/server_controller_sql.go:170 +0x1d7
  github.com/cockroachdb/cockroach/pkg/server.(*serverController).sqlMux()
      pkg/server/server_controller_sql.go:90 +0x50b
  github.com/cockroachdb/cockroach/pkg/server.(*serverController).sqlMux-fm()
      <autogenerated>:1 +0xc4
  github.com/cockroachdb/cockroach/pkg/server.startServeSQL.func1.1()
      pkg/server/server_sql.go:1991 +0x39b
  github.com/cockroachdb/cockroach/pkg/util/netutil.(*TCPServer).ServeWith.func1()
      pkg/util/netutil/net.go:186 +0x1c6
  github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunAsyncTaskEx.func2()
      pkg/util/stop/stopper.go:498 +0x338
  github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunAsyncTaskEx.gowrap1()
      pkg/util/stop/stopper.go:499 +0x4f
Goroutine 32946 (running) created at:
  github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunAsyncTaskEx()
      pkg/util/stop/stopper.go:488 +0x72f
  github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunAsyncTask()
      pkg/util/stop/stopper.go:359 +0x253
  github.com/cockroachdb/cockroach/pkg/server.(*Node).startWriteNodeStatus()
      pkg/server/node.go:1464 +0x138
  github.com/cockroachdb/cockroach/pkg/server.(*topLevelServer).PreStart()
      pkg/server/server.go:1986 +0x52b0
  github.com/cockroachdb/cockroach/pkg/server.(*testServer).PreStart()
      pkg/server/testserver.go:794 +0x104
  github.com/cockroachdb/cockroach/pkg/testutils/serverutils.(*wrap).PreStart()
      bazel-out/k8-fastbuild/bin/pkg/testutils/serverutils/ts_control_forwarder_generated.go:19 +0x72
  github.com/cockroachdb/cockroach/pkg/testutils/testcluster.(*TestCluster).Start()
      pkg/testutils/testcluster/testcluster.go:442 +0x63e
  github.com/cockroachdb/cockroach/pkg/testutils/testcluster.StartTestCluster()
      pkg/testutils/testcluster/testcluster.go:259 +0xa4
  github.com/cockroachdb/cockroach/pkg/kv/kvserver_test.TestFlowControlBlockedAdmission()
      pkg/kv/kvserver/flow_control_integration_test.go:355 +0x511
  testing.tRunner()
      GOROOT/src/testing/testing.go:1689 +0x21e
  testing.(*T).Run.gowrap1()
      GOROOT/src/testing/testing.go:1742 +0x44
==================

@nvanbenschoten
Copy link
Member

This code was just changed in b8c532e and in 64c9018. This appears to be fallout from those changes.

@nvanbenschoten
Copy link
Member

The problem seems to be that none of the ToPrometheusMetrics methods (CounterVec, GaugeVec, HistogramVec) are acquiring the read lock on metric.vector when accessing metric.vector.encounteredLabelValues.

This bug seems to have been introduced in 39033a3, so I'll route the issue that way.

@nvanbenschoten nvanbenschoten added T-observability and removed T-kv KV Team labels Dec 20, 2024
@arjunmahishi
Copy link
Contributor

Looks like the fix for this was merged last week: #137532

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@arjunmahishi arjunmahishi

Labels
branch-master Failures and bugs on the master branch. C-test-failure Broken test (automatically or manually discovered). O-robot Originated from a bot. release-blocker Indicates a release-blocker. Use with branch-release-2x.x label to denote which branch is blocked. T-observability
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@nvanbenschoten @arjunmahishi @cockroach-teamcity