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

ccl/kvccl/kvfollowerreadsccl: TestFollowerReadsWithStaleDescriptor failed #108087

Closed
cockroach-teamcity opened this issue Aug 3, 2023 · 1 comment · Fixed by #108118
Closed
Assignees
Labels
A-testing Testing tools and infrastructure branch-master Failures and bugs on the master branch. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. C-test-failure Broken test (automatically or manually discovered). O-robot Originated from a bot. T-kv KV Team
Milestone

Comments

@cockroach-teamcity
Copy link
Member

cockroach-teamcity commented Aug 3, 2023

ccl/kvccl/kvfollowerreadsccl.TestFollowerReadsWithStaleDescriptor failed with artifacts on master @ 4fe2a80d81c6fc5a3da3c7c44c5fc38da67e0367:

      github.com/cockroachdb/cockroach/pkg/testutils/testcluster/testcluster.go:345 +0xf90
  github.com/cockroachdb/cockroach/pkg/testutils/testcluster.StartTestCluster()
      github.com/cockroachdb/cockroach/pkg/testutils/testcluster/testcluster.go:234 +0x8b
  github.com/cockroachdb/cockroach/pkg/ccl/kvccl/kvfollowerreadsccl.TestFollowerReadsWithStaleDescriptor()
      github.com/cockroachdb/cockroach/pkg/ccl/kvccl/kvfollowerreadsccl/followerreads_test.go:703 +0x5f6
  testing.tRunner()
      GOROOT/src/testing/testing.go:1446 +0x216
  testing.(*T).Run.func1()
      GOROOT/src/testing/testing.go:1493 +0x47

Goroutine 8879 (running) created at:
  github.com/cockroachdb/cockroach/pkg/sql.(*InternalExecutor).runWithEx()
      github.com/cockroachdb/cockroach/pkg/sql/internal.go:221 +0x345
  github.com/cockroachdb/cockroach/pkg/sql.(*InternalExecutor).execInternal()
      github.com/cockroachdb/cockroach/pkg/sql/internal.go:1076 +0x138b
  github.com/cockroachdb/cockroach/pkg/sql.(*InternalExecutor).QueryIteratorEx()
      github.com/cockroachdb/cockroach/pkg/sql/internal.go:780 +0x297
  github.com/cockroachdb/cockroach/pkg/sql.(*InternalExecutor).QueryIterator()
      github.com/cockroachdb/cockroach/pkg/sql/internal.go:766 +0x13e
  github.com/cockroachdb/cockroach/pkg/server.(*serverController).getExpectedRunningTenants()
      github.com/cockroachdb/cockroach/pkg/server/server_controller_orchestration.go:195 +0x1ea
  github.com/cockroachdb/cockroach/pkg/server.(*serverController).scanTenantsForRunnableServices()
      github.com/cockroachdb/cockroach/pkg/server/server_controller_orchestration.go:106 +0xa8
  github.com/cockroachdb/cockroach/pkg/server.(*serverController).start.func1()
      github.com/cockroachdb/cockroach/pkg/server/server_controller_orchestration.go:60 +0x21a
  github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunAsyncTaskEx.func2()
      github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go:484 +0x1f6

Goroutine 2551 (running) created at:
  testing.(*T).Run()
      GOROOT/src/testing/testing.go:1493 +0x75d
  testing.runTests.func1()
      GOROOT/src/testing/testing.go:1846 +0x99
  testing.tRunner()
      GOROOT/src/testing/testing.go:1446 +0x216
  testing.runTests()
      GOROOT/src/testing/testing.go:1844 +0x7ec
  testing.(*M).Run()
      GOROOT/src/testing/testing.go:1726 +0xa84
  github.com/cockroachdb/cockroach/pkg/ccl/kvccl/kvfollowerreadsccl.TestMain()
      github.com/cockroachdb/cockroach/pkg/ccl/kvccl/kvfollowerreadsccl/main_test.go:30 +0x18e
  main.main()
      main/bazel-out/k8-fastbuild/bin/pkg/ccl/kvccl/kvfollowerreadsccl/kvfollowerreadsccl_test_/testmain.go:128 +0x748
  runtime.main()
      GOROOT/src/runtime/proc.go:250 +0x211
  github.com/cockroachdb/cockroach/pkg/util/parquet.box2DDecoder.decode()
      github.com/cockroachdb/cockroach/pkg/util/parquet/decoders.go:174 +0x44
  github.com/cockroachdb/cockroach/pkg/util/parquet.init.0()
      github.com/cockroachdb/cockroach/pkg/util/parquet/decoders.go:336 +0x117
==================

Parameters: TAGS=bazel,gss,race , stress=true

Help

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

/cc @cockroachdb/kv

This test on roachdash | Improve this report!

Jira issue: CRDB-30309

@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. T-kv KV Team labels Aug 3, 2023
@cockroach-teamcity cockroach-teamcity added this to the 23.2 milestone Aug 3, 2023
@nvanbenschoten
Copy link
Member

Looks like a classic data race between a variable mutated in the test's main goroutine and read in an execution hook.

WARNING: DATA RACE
Read at 0x00c003f7c5f0 by goroutine 8879:
  github.com/cockroachdb/cockroach/pkg/ccl/kvccl/kvfollowerreadsccl.TestFollowerReadsWithStaleDescriptor.func2()
      github.com/cockroachdb/cockroach/pkg/ccl/kvccl/kvfollowerreadsccl/followerreads_test.go:735 +0x56
  github.com/cockroachdb/cockroach/pkg/sql.(*instrumentationHelper).Finish()
      github.com/cockroachdb/cockroach/pkg/sql/instrumentation.go:396 +0x1fd
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmtInOpenState.func11.1()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:669 +0x4d7
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmtInOpenState.func3()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:305 +0x3aa
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmtInOpenState.func11()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:658 +0x3e1
  runtime.deferreturn()
      GOROOT/src/runtime/panic.go:476 +0x32
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmt.func1()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:142 +0x18e
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execWithProfiling()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:2989 +0x50f
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execStmt()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:141 +0x724
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execPortal()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor_exec.go:247 +0x365
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execCmd.func2()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:2325 +0xf70
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).execCmd()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:2327 +0xbe4
  github.com/cockroachdb/cockroach/pkg/sql.(*connExecutor).run()
      github.com/cockroachdb/cockroach/pkg/sql/conn_executor.go:2142 +0x397
  github.com/cockroachdb/cockroach/pkg/sql.(*InternalExecutor).runWithEx.func1()
      github.com/cockroachdb/cockroach/pkg/sql/internal.go:222 +0x117

Previous write at 0x00c003f7c5f0 by goroutine 2551:
  github.com/cockroachdb/cockroach/pkg/ccl/kvccl/kvfollowerreadsccl.TestFollowerReadsWithStaleDescriptor()
      github.com/cockroachdb/cockroach/pkg/ccl/kvccl/kvfollowerreadsccl/followerreads_test.go:858 +0x1959
  github.com/cockroachdb/cockroach/pkg/testutils/testcluster.NewTestCluster()
      github.com/cockroachdb/cockroach/pkg/testutils/testcluster/testcluster.go:345 +0xf90
  github.com/cockroachdb/cockroach/pkg/testutils/testcluster.NewTestCluster()
      github.com/cockroachdb/cockroach/pkg/testutils/testcluster/testcluster.go:345 +0xf90
  github.com/cockroachdb/cockroach/pkg/testutils/testcluster.StartTestCluster()
      github.com/cockroachdb/cockroach/pkg/testutils/testcluster/testcluster.go:234 +0x8b
  github.com/cockroachdb/cockroach/pkg/ccl/kvccl/kvfollowerreadsccl.TestFollowerReadsWithStaleDescriptor()
      github.com/cockroachdb/cockroach/pkg/ccl/kvccl/kvfollowerreadsccl/followerreads_test.go:703 +0x5f6
  testing.tRunner()
      GOROOT/src/testing/testing.go:1446 +0x216
  testing.(*T).Run.func1()
      GOROOT/src/testing/testing.go:1493 +0x47

@nvanbenschoten nvanbenschoten self-assigned this Aug 3, 2023
craig bot pushed a commit that referenced this issue Aug 3, 2023
107756: roachtest: make path to default cockroach public r=srosenberg a=renatolabs

The roachtest test runner already uploads the `cockroach` binary passed with `--cockroach` to every node in the cluster. By making the path to that file public, we allow tests to use that binary, stopping avoidable uploads.

Concretely, this commit removes a step from `mixedversion` tests where the same logic of uploading the current binary took place. Tests can use the new constant in the `test` package instead.

Epic: none

Release note: None

108082: sql: remove slow functions from TestRandomSyntaxFunctions r=fqazi a=fqazi

Previously, we could easily timeout running slower,
functions in the random syntax functions test. We
attempted to minimize this risk with resettable timeouts
which helped, but libpq has limited support for cancellation,
so we need to fully pull these out. To address this,
this patch will remove:
crdb_internal.revalidate_unique_constraints_in_all_tables
and crdb_internal.validate_ttl_scheduled_jobs from testing.

Fixes: #107929
Release note: None

108118: kv: deflake TestFollowerReadsWithStaleDescriptor r=nvanbenschoten a=nvanbenschoten

Fixes #108087.

This fix avoids a data race in the test. The race was harmless, but could cause the test to fail when run with the race detector.

Release note: None

Co-authored-by: Renato Costa <[email protected]>
Co-authored-by: Faizan Qazi <[email protected]>
Co-authored-by: Nathan VanBenschoten <[email protected]>
@craig craig bot closed this as completed in 6db50f7 Aug 3, 2023
blathers-crl bot pushed a commit that referenced this issue Aug 3, 2023
Fixes #108087.

This fix avoids a data race in the test. The race was harmless, but
could cause the test to fail when run with the race detector.

Release note: None
@nvanbenschoten nvanbenschoten added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-testing Testing tools and infrastructure labels Aug 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testing Testing tools and infrastructure branch-master Failures and bugs on the master branch. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. C-test-failure Broken test (automatically or manually discovered). O-robot Originated from a bot. T-kv KV Team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants