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

kvserver: TestMergeQueueSeesLearnerOrJointConfig data race #105381

Closed
Tracked by #98703
erikgrinaker opened this issue Jun 22, 2023 · 1 comment · Fixed by #105642
Closed
Tracked by #98703

kvserver: TestMergeQueueSeesLearnerOrJointConfig data race #105381

erikgrinaker opened this issue Jun 22, 2023 · 1 comment · Fixed by #105642
Assignees
@irfansharif
Labels
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).

Comments

@erikgrinaker
Copy link
Contributor

erikgrinaker commented Jun 22, 2023 •
edited by cockroach-jira-scripts
Loading

Seen in a CI failure. Related to the recent replication AC work.

      WARNING: DATA RACE
      Write at 0x00c0086e4300 by goroutine 1927829:
        github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*RaftTransport).RaftMessageBatch.func2.1()
            github.com/cockroachdb/cockroach/pkg/kv/kvserver/pkg/kv/kvserver/raft_transport.go:478 +0x1c5
        github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*RaftTransport).RaftMessageBatch.func2()
            github.com/cockroachdb/cockroach/pkg/kv/kvserver/pkg/kv/kvserver/raft_transport.go:497 +0x84
        github.com/cockroachdb/cockroach/pkg/util/stop.(*Stopper).RunAsyncTaskEx.func2()
            github.com/cockroachdb/cockroach/pkg/util/stop/stopper.go:484 +0x1f6
      
      Previous read at 0x00c0086e4300 by goroutine 1927828:
        github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*RaftTransport).RaftMessageBatch.func1()
            github.com/cockroachdb/cockroach/pkg/kv/kvserver/pkg/kv/kvserver/raft_transport.go:455 +0x164
        runtime.deferreturn()
            GOROOT/src/runtime/panic.go:476 +0x32
        github.com/cockroachdb/cockroach/pkg/kv/kvserver._MultiRaft_RaftMessageBatch_Handler()
            github.com/cockroachdb/cockroach/pkg/kv/kvserver/bazel-out/k8-fastbuild/bin/pkg/kv/kvserver/kvserver_go_proto_/github.com/cockroachdb/cockroach/pkg/kv/kvserver/storage_services.pb.go:237 +0xc2
        github.com/cockroachdb/cockroach/pkg/util/tracing/grpcinterceptor.StreamServerInterceptor.func1()
            github.com/cockroachdb/cockroach/pkg/util/tracing/grpcinterceptor/grpc_interceptor.go:164 +0x679
        google.golang.org/grpc.chainStreamInterceptors.func1.1()
            google.golang.org/grpc/external/org_golang_google_grpc/server.go:1482 +0x105
        github.com/cockroachdb/cockroach/pkg/rpc.NewServerEx.func4()
            github.com/cockroachdb/cockroach/pkg/rpc/pkg/rpc/context.go:176 +0xe8
        google.golang.org/grpc.chainStreamInterceptors.func1.1()
            google.golang.org/grpc/external/org_golang_google_grpc/server.go:1485 +0x1e9
        github.com/cockroachdb/cockroach/pkg/rpc.kvAuth.streamInterceptor()
            github.com/cockroachdb/cockroach/pkg/rpc/pkg/rpc/auth.go:167 +0x556
        github.com/cockroachdb/cockroach/pkg/rpc.kvAuth.streamInterceptor-fm()
            <autogenerated>:1 +0xda
        google.golang.org/grpc.chainStreamInterceptors.func1.1()
            google.golang.org/grpc/external/org_golang_google_grpc/server.go:1485 +0x1e9
        github.com/cockroachdb/cockroach/pkg/rpc.NewServerEx.func2.1()
            github.com/cockroachdb/cockroach/pkg/rpc/pkg/rpc/context.go:143 +0x6f
        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.func2()
            github.com/cockroachdb/cockroach/pkg/rpc/pkg/rpc/context.go:142 +0x14c
        google.golang.org/grpc.chainStreamInterceptors.func1.1()
            google.golang.org/grpc/external/org_golang_google_grpc/server.go:1485 +0x1e9
        google.golang.org/grpc.chainStreamInterceptors.func1()
            google.golang.org/grpc/external/org_golang_google_grpc/server.go:1487 +0x275
        google.golang.org/grpc.(*Server).processStreamingRPC()
            google.golang.org/grpc/external/org_golang_google_grpc/server.go:1636 +0x1ef5
        google.golang.org/grpc.(*Server).handleStream()
            google.golang.org/grpc/external/org_golang_google_grpc/server.go:1717 +0xfae
        google.golang.org/grpc.(*Server).serveStreams.func1.2()
            google.golang.org/grpc/external/org_golang_google_grpc/server.go:965 +0xec

Jira issue: CRDB-29008
@erikgrinaker erikgrinaker added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-kv-replication labels Jun 22, 2023
@blathers-crl
Copy link

blathers-crl bot commented Jun 22, 2023

cc @cockroachdb/replication

@erikgrinaker erikgrinaker added the C-test-failure Broken test (automatically or manually discovered). label Jun 22, 2023
@irfansharif irfansharif mentioned this issue Jun 25, 2023
Closed
15 tasks
@irfansharif irfansharif mentioned this issue Jun 27, 2023
Merged
craig bot pushed a commit that referenced this issue Jun 29, 2023
@irfansharif
Merge #105642
c1d73cd 
105642: kvserver: de-flake TestMergeQueueSeesLearnerOrJointConfig r=irfansharif a=irfansharif

Fixes #105381. It was possible for us to receive client-side store IDs concurrently with the server-side stopper being quiesced, at which point we read the variable storing the client-side store IDs without a mutex (which triggered the data race detector). The data race was benign, but instead of adding a synchronization primitive, we recognize that there was no reason for the store ID handling to happen on separate threads.

Release note: None

Co-authored-by: irfan sharif <[email protected]>
@craig craig bot closed this as completed in b514d3d Jun 29, 2023
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. C-test-failure Broken test (automatically or manually discovered).
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

kvserver: de-flake TestMergeQueueSeesLearnerOrJointConfig irfansharif/cockroach
2 participants
@erikgrinaker @irfansharif