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

kvflowcontrol: data race in TestRelocateNonVoters [kvflowhandle race] #105762

Closed
yuzefovich opened this issue Jun 28, 2023 · 3 comments · Fixed by #104855
Closed

kvflowcontrol: data race in TestRelocateNonVoters [kvflowhandle race] #105762

yuzefovich opened this issue Jun 28, 2023 · 3 comments · Fixed by #104855
Assignees
@irfansharif
Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-kv KV Team

Comments

@yuzefovich
Copy link
Member

yuzefovich commented Jun 28, 2023 •
edited by cockroach-jira-scripts
Loading

Seen on this build on likely unrelated PR (a couple of commits on top of 06a051f):

=== RUN   TestRelocateNonVoters/ALTER_RANGE_RELOCATE_NONVOTERS
==================
WARNING: DATA RACE
Write at 0x00c00cdda230 by goroutine 1573582:
  internal/reflectlite.Swapper.func3()
      GOROOT/src/internal/reflectlite/swapper.go:42 +0xf2
  sort.insertionSort_func()
      GOROOT/src/sort/zsortfunc.go:13 +0x94
  sort.pdqsort_func()
      GOROOT/src/sort/zsortfunc.go:73 +0x3cb
  sort.Slice()
      GOROOT/src/sort/slice.go:23 +0xc4
  github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvflowcontrol/kvflowhandle.(*Handle).connectStreamLocked()
      github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvflowcontrol/kvflowhandle/kvflowhandle.go:238 +0x3ca
  github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvflowcontrol/kvflowhandle.(*Handle).ConnectStream()
      github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvflowcontrol/kvflowhandle/kvflowhandle.go:228 +0x1a4
  github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*replicaFlowControlIntegrationImpl).tryReconnect()
      github.com/cockroachdb/cockroach/pkg/kv/kvserver/pkg/kv/kvserver/flow_control_replica_integration.go:423 +0x518
  github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*replicaFlowControlIntegrationImpl).refreshStreams()
      github.com/cockroachdb/cockroach/pkg/kv/kvserver/pkg/kv/kvserver/flow_control_replica_integration.go:289 +0x84
  github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*replicaFlowControlIntegrationImpl).onRaftTicked()
      github.com/cockroachdb/cockroach/pkg/kv/kvserver/pkg/kv/kvserver/flow_control_replica_integration.go:257 +0x8b
  github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*Replica).updateProposalQuotaRaftMuLocked()
      github.com/cockroachdb/cockroach/pkg/kv/kvserver/pkg/kv/kvserver/replica_proposal_quota.go:268 +0xe7c
  github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*Replica).handleRaftReadyRaftMuLocked()
      github.com/cockroachdb/cockroach/pkg/kv/kvserver/pkg/kv/kvserver/replica_raft.go:1151 +0x2344
  github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*Replica).handleRaftReady()
      github.com/cockroachdb/cockroach/pkg/kv/kvserver/pkg/kv/kvserver/replica_raft.go:751 +0x20f
  github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*Store).processReady()
      github.com/cockroachdb/cockroach/pkg/kv/kvserver/pkg/kv/kvserver/store_raft.go:646 +0x1cf
  github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*raftSchedulerShard).worker()
      github.com/cockroachdb/cockroach/pkg/kv/kvserver/pkg/kv/kvserver/scheduler.go:418 +0x2f5
  github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*raftScheduler).Start.func2()
      github.com/cockroachdb/cockroach/pkg/kv/kvserver/pkg/kv/kvserver/scheduler.go:321 +0x9a
  github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunAsyncTaskEx.func2()
      github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go:484 +0x1f6

Previous read at 0x00c00cdda230 by goroutine 1627371:
  github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvflowcontrol/kvflowhandle.(*Handle).Admit()
      github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvflowcontrol/kvflowhandle/kvflowhandle.go:110 +0x2a4
  github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvadmission.(*controllerImpl).AdmitKVWork()
      github.com/cockroachdb/cockroach/pkg/kv/kvserver/kvadmission/kvadmission.go:310 +0x5ec
  github.com/cockroachdb/cockroach/pkg/server.(*Node).batchInternal()
      github.com/cockroachdb/cockroach/pkg/server/node.go:1268 +0x4ac
  github.com/cockroachdb/cockroach/pkg/server.(*Node).Batch()
      github.com/cockroachdb/cockroach/pkg/server/node.go:1464 +0x3bd
  github.com/cockroachdb/cockroach/pkg/kv/kvpb._Internal_Batch_Handler.func1()
      github.com/cockroachdb/cockroach/pkg/kv/kvpb/bazel-out/k8-fastbuild/bin/pkg/kv/kvpb/kvpb_go_proto_/github.com/cockroachdb/cockroach/pkg/kv/kvpb/api.pb.go:10099 +0x88
  github.com/cockroachdb/cockroach/pkg/util/tracing/grpcinterceptor.ServerInterceptor.func1()
      github.com/cockroachdb/cockroach/pkg/util/tracing/grpcinterceptor/grpc_interceptor.go:96 +0x1d4
  google.golang.org/grpc.getChainUnaryHandler.func1()
      google.golang.org/grpc/external/org_golang_google_grpc/server.go:1163 +0x131
  github.com/cockroachdb/cockroach/pkg/rpc.NewServerEx.func3()
      github.com/cockroachdb/cockroach/pkg/rpc/pkg/rpc/context.go:167 +0xf8
  google.golang.org/grpc.getChainUnaryHandler.func1()
      google.golang.org/grpc/external/org_golang_google_grpc/server.go:1163 +0x131
  github.com/cockroachdb/cockroach/pkg/rpc.kvAuth.unaryInterceptor()
      github.com/cockroachdb/cockroach/pkg/rpc/pkg/rpc/auth.go:105 +0x322
  github.com/cockroachdb/cockroach/pkg/rpc.kvAuth.unaryInterceptor-fm()
      <autogenerated>:1 +0xe6
  google.golang.org/grpc.getChainUnaryHandler.func1()
      google.golang.org/grpc/external/org_golang_google_grpc/server.go:1163 +0x131
  github.com/cockroachdb/cockroach/pkg/rpc.NewServerEx.func1.1()
      github.com/cockroachdb/cockroach/pkg/rpc/pkg/rpc/context.go:134 +0x70
  github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunTaskWithErr()
      github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go:336 +0x147
  github.com/cockroachdb/cockroach/pkg/rpc.NewServerEx.func1()
      github.com/cockroachdb/cockroach/pkg/rpc/pkg/rpc/context.go:132 +0x137
  google.golang.org/grpc.chainUnaryInterceptors.func1()
      google.golang.org/grpc/external/org_golang_google_grpc/server.go:1154 +0xe2
  github.com/cockroachdb/cockroach/pkg/kv/kvpb._Internal_Batch_Handler()
      github.com/cockroachdb/cockroach/pkg/kv/kvpb/bazel-out/k8-fastbuild/bin/pkg/kv/kvpb/kvpb_go_proto_/github.com/cockroachdb/cockroach/pkg/kv/kvpb/api.pb.go:10101 +0x1dd
  google.golang.org/grpc.(*Server).processUnaryRPC()
      google.golang.org/grpc/external/org_golang_google_grpc/server.go:1336 +0x15da
  google.golang.org/grpc.(*Server).handleStream()
      google.golang.org/grpc/external/org_golang_google_grpc/server.go:1704 +0xff8
  google.golang.org/grpc.(*Server).serveStreams.func1.2()
      google.golang.org/grpc/external/org_golang_google_grpc/server.go:965 +0xec

Goroutine 1573582 (running) created at:
  github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunAsyncTaskEx()
      github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go:475 +0x619
  github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*raftScheduler).Start()
      github.com/cockroachdb/cockroach/pkg/kv/kvserver/pkg/kv/kvserver/scheduler.go:313 +0x406
  github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*Store).processRaft()
      github.com/cockroachdb/cockroach/pkg/kv/kvserver/pkg/kv/kvserver/store_raft.go:714 +0xc4
  github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*Store).Start()
      github.com/cockroachdb/cockroach/pkg/kv/kvserver/pkg/kv/kvserver/store.go:2065 +0x127a
  github.com/cockroachdb/cockroach/pkg/server.(*Node).start()
      github.com/cockroachdb/cockroach/pkg/server/node.go:605 +0x1539
  github.com/cockroachdb/cockroach/pkg/server.(*Server).PreStart()
      github.com/cockroachdb/cockroach/pkg/server/server.go:1809 +0x349a
  github.com/cockroachdb/cockroach/pkg/server.(*TestServer).Start()
      github.com/cockroachdb/cockroach/pkg/server/testserver.go:622 +0x8f
  github.com/cockroachdb/cockroach/pkg/testutils/testcluster.(*TestCluster).startServer()
      github.com/cockroachdb/cockroach/pkg/testutils/testcluster/testcluster.go:593 +0xd4
  github.com/cockroachdb/cockroach/pkg/testutils/testcluster.(*TestCluster).Start()
      github.com/cockroachdb/cockroach/pkg/testutils/testcluster/testcluster.go:389 +0x890
  github.com/cockroachdb/cockroach/pkg/testutils/serverutils.StartNewTestCluster()
      github.com/cockroachdb/cockroach/pkg/testutils/serverutils/test_cluster_shim.go:281 +0x116
  github.com/cockroachdb/cockroach/pkg/testutils/testcluster.NewTestCluster()
      github.com/cockroachdb/cockroach/pkg/testutils/testcluster/testcluster.go:329 +0xcec
  github.com/cockroachdb/cockroach/pkg/testutils/testcluster.NewTestCluster()
      github.com/cockroachdb/cockroach/pkg/testutils/testcluster/testcluster.go:329 +0xcec
  github.com/cockroachdb/cockroach/pkg/testutils/testcluster.testClusterFactoryImpl.NewTestCluster()
      github.com/cockroachdb/cockroach/pkg/testutils/testcluster/testcluster.go:1920 +0x7d
  github.com/cockroachdb/cockroach/pkg/testutils/testcluster.(*testClusterFactoryImpl).NewTestCluster()
      <autogenerated>:1 +0xa4
  github.com/cockroachdb/cockroach/pkg/testutils/serverutils.NewTestCluster()
      github.com/cockroachdb/cockroach/pkg/testutils/serverutils/test_cluster_shim.go:299 +0xea
  github.com/cockroachdb/cockroach/pkg/testutils/serverutils.StartNewTestCluster()
      github.com/cockroachdb/cockroach/pkg/testutils/serverutils/test_cluster_shim.go:280 +0x56
  github.com/cockroachdb/cockroach/pkg/sql_test.testCase.runTest()
      github.com/cockroachdb/cockroach/pkg/sql_test/pkg/sql/multitenant_admin_function_test.go:249 +0x1a4
  github.com/cockroachdb/cockroach/pkg/sql_test.TestRelocateNonVoters.func1()
      github.com/cockroachdb/cockroach/pkg/sql_test/pkg/sql/multitenant_admin_function_test.go:886 +0x1ef
  testing.tRunner()
      GOROOT/src/testing/testing.go:1446 +0x216
  testing.(*T).Run.func1()
      GOROOT/src/testing/testing.go:1493 +0x47

Goroutine 1627371 (running) created at:
  google.golang.org/grpc.(*Server).serveStreams.func1()
      google.golang.org/grpc/external/org_golang_google_grpc/server.go:963 +0x4dd
  google.golang.org/grpc/internal/transport.(*http2Server).operateHeaders()
      google.golang.org/grpc/internal/transport/external/org_golang_google_grpc/internal/transport/http2_server.go:620 +0x357e
  google.golang.org/grpc/internal/transport.(*http2Server).HandleStreams()
      google.golang.org/grpc/internal/transport/external/org_golang_google_grpc/internal/transport/http2_server.go:662 +0x378
  google.golang.org/grpc.(*Server).serveStreams()
      google.golang.org/grpc/external/org_golang_google_grpc/server.go:949 +0x2d5
  google.golang.org/grpc.(*Server).handleRawConn.func1()
      google.golang.org/grpc/external/org_golang_google_grpc/server.go:891 +0x64
==================

Jira issue: CRDB-29178
@yuzefovich yuzefovich added the C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. label Jun 28, 2023
@blathers-crl blathers-crl bot added the T-kv KV Team label Jun 28, 2023
@irfansharif
Copy link
Contributor

Same as #104837, will be fixed by #104855.

@irfansharif irfansharif mentioned this issue Jul 4, 2023
Merged
@irfansharif irfansharif changed the title kvflowcontrol: data race in TestRelocateNonVoters kvflowcontrol: data race in TestRelocateNonVoters [kvflowhandle race] Jul 4, 2023
craig bot pushed a commit that referenced this issue Jul 4, 2023
@irfansharif
Merge #104855
c77d127 
104855: kvflowcontrol: squash data race r=irfansharif a=irfansharif

Fixes #104837.
Fixes #105762.

Release note: None

Co-authored-by: irfan sharif <[email protected]>
@craig craig bot closed this as completed in edd1e1b Jul 5, 2023
@chengxiong-ruan
Copy link
Contributor

chengxiong-ruan commented Aug 3, 2023 •
edited
Loading

@irfansharif TestRelocateNonVoters failed here. But It seems like a different failure. Do you want me to open a new issue or just reuse this one?

Failed
=== RUN   TestRelocateNonVoters/ALTER_RANGE_x_RELOCATE_NONVOTERS
    test_server_shim.go:98:
        Test server was configured to route SQL queries to a secondary tenant (virtual cluster).
        If you are only seeing a test failure when this message appears, there may be a problem
        specific to cluster virtualization or multi-tenancy.
        To investigate, consider using "COCKROACH_TEST_TENANT=true" to force-enable just
        the secondary tenant in all runs (or, alternatively, "false" to force-disable), or use
        "COCKROACH_INTERNAL_DISABLE_METAMORPHIC_TESTING=false" to disable all random test variables altogether.
    testcluster.go:416:
        Test server was configured to route SQL queries to a secondary tenant (virtual cluster).
        If you are only seeing a test failure when this message appears, there may be a problem
        specific to cluster virtualization or multi-tenancy.
        To investigate, consider using "COCKROACH_TEST_TENANT=true" to force-enable just
        the secondary tenant in all runs (or, alternatively, "false" to force-disable), or use
        "COCKROACH_INTERNAL_DISABLE_METAMORPHIC_TESTING=false" to disable all random test variables altogether.
    multitenant_admin_function_test.go:369: condition failed to evaluate within 3m45s: from multitenant_admin_function_test.go:392: expected "trying to add a non-voter to a store that already has a NON_VOTER" contains "ok" tenant=secondary query=`ALTER RANGE (SELECT min(range_id) FROM [SHOW RANGES FROM TABLE t]) RELOCATE NONVOTERS FROM 2 TO 3;` leaseholder=1 replicas=[1 2 4 5] voting_replicas=[1 4 5] non_voting_replicas=[2] fromReplica=2 toReplica=3 row=0 col=2
    --- FAIL: TestRelocateNonVoters/ALTER_RANGE_x_RELOCATE_NONVOTERS (433.62s)

@chengxiong-ruan
Copy link
Contributor

I filed a new one since this is quite different : #108081

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

@irfansharif irfansharif

Labels
C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-kv KV Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

kvflowcontrol: squash data race irfansharif/cockroach
3 participants
@yuzefovich @chengxiong-ruan @irfansharif