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: data race in (*Replica).recordImpactOnRateLimiter #95345

Closed
knz opened this issue Jan 17, 2023 · 2 comments · Fixed by #95524
Closed

kv: data race in (*Replica).recordImpactOnRateLimiter #95345

knz opened this issue Jan 17, 2023 · 2 comments · Fixed by #95524
Assignees
@pav-kv
Labels
A-kv-replication Relating to Raft, consensus, and coordination. C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. T-kv KV Team

Comments

@knz
Copy link
Contributor

knz commented Jan 17, 2023 •
edited by cockroach-jira-scripts
Loading

Found here: https://teamcity.cockroachdb.com/buildConfiguration/Cockroach_BazelExtendedCi/8334227?showRootCauses=false&expandBuildChangesSection=true&expandBuildProblemsSection=true&expandBuildTestsSection=true

==================
WARNING: DATA RACE
Read at 0x00c00bfd8550 by goroutine 582130:
  github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*Replica).recordImpactOnRateLimiter()
      github.com/cockroachdb/cockroach/pkg/kv/kvserver/pkg/kv/kvserver/replica_rate_limit.go:39 +0x58
  github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*Replica).SendWithWriteBytes()
      github.com/cockroachdb/cockroach/pkg/kv/kvserver/pkg/kv/kvserver/replica_send.go:213 +0x8a4
  github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*Store).SendWithWriteBytes()
      github.com/cockroachdb/cockroach/pkg/kv/kvserver/pkg/kv/kvserver/store_send.go:205 +0xb51
  github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*Stores).SendWithWriteBytes()
      github.com/cockroachdb/cockroach/pkg/kv/kvserver/pkg/kv/kvserver/stores.go:203 +0x252
  github.com/cockroachdb/cockroach/pkg/server.(*Node).batchInternal()
      github.com/cockroachdb/cockroach/pkg/server/node.go:1121 +0x712
  github.com/cockroachdb/cockroach/pkg/server.(*Node).Batch()
      github.com/cockroachdb/cockroach/pkg/server/node.go:1173 +0x275
  github.com/cockroachdb/cockroach/pkg/rpc.makeInternalClientAdapter.func1()
      github.com/cockroachdb/cockroach/pkg/rpc/pkg/rpc/context.go:741 +0x7c
[...]
Previous write at 0x00c00bfd8550 by goroutine 567167:
        github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*Replica).postDestroyRaftMuLocked()
            github.com/cockroachdb/cockroach/pkg/kv/kvserver/pkg/kv/kvserver/replica_destroy.go:135 +0x21b
        github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*Replica).handleChangeReplicasResult()
            github.com/cockroachdb/cockroach/pkg/kv/kvserver/pkg/kv/kvserver/replica_application_result.go:340 +0x2ef
        github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*replicaStateMachine).handleNonTrivialReplicatedEvalResult()
            github.com/cockroachdb/cockroach/pkg/kv/kvserver/pkg/kv/kvserver/replica_application_state_machine.go:338 +0xd44
        github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*replicaStateMachine).ApplySideEffects()
            github.com/cockroachdb/cockroach/pkg/kv/kvserver/pkg/kv/kvserver/replica_application_state_machine.go:186 +0x633
        github.com/cockroachdb/cockroach/pkg/kv/kvserver/apply.StateMachine.ApplySideEffects-fm()
            <autogenerated>:1 +0x85
        github.com/cockroachdb/cockroach/pkg/kv/kvserver/apply.mapCheckedCmdIter()
            github.com/cockroachdb/cockroach/pkg/kv/kvserver/apply/cmd.go:210 +0x1da
        github.com/cockroachdb/cockroach/pkg/kv/kvserver/apply.(*Task).applyOneBatch()
            github.com/cockroachdb/cockroach/pkg/kv/kvserver/apply/task.go:304 +0x266
        github.com/cockroachdb/cockroach/pkg/kv/kvserver/apply.(*Task).ApplyCommittedEntries()
            github.com/cockroachdb/cockroach/pkg/kv/kvserver/apply/task.go:260 +0xd5
        github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*Replica).handleRaftReadyRaftMuLocked()
            github.com/cockroachdb/cockroach/pkg/kv/kvserver/pkg/kv/kvserver/replica_raft.go:984 +0x1df1
        github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*Replica).handleRaftReady()
            github.com/cockroachdb/cockroach/pkg/kv/kvserver/pkg/kv/kvserver/replica_raft.go:677 +0x14f
        github.com/cockroachdb/cockroach/pkg/kv/kvserver.(*Store).processReady()

cc @tbg @nvanbenschoten for triage.

Jira issue: CRDB-23467
@knz knz added C-bug Code not up to spec/doc, specs & docs deemed correct. Solution expected to change code/behavior. A-kv-replication Relating to Raft, consensus, and coordination. T-kv-replication labels Jan 17, 2023
@blathers-crl
Copy link

blathers-crl bot commented Jan 17, 2023

cc @cockroachdb/replication

@blathers-crl blathers-crl bot added the T-kv KV Team label Jan 17, 2023
@erikgrinaker
Copy link
Contributor

We'll need to synchronize access to Replica.tenantLimiter.

@pavelkalinnikov This is related to the replica lifecycle work y'all are working on, can you pick this up?

@pav-kv pav-kv mentioned this issue Jan 19, 2023
Merged
@craig craig bot closed this as completed in b23943c Jan 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pav-kv pav-kv

Labels
A-kv-replication Relating to Raft, consensus, and coordination. 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.

kvserver: fix race on Replica.tenantLimiter pav-kv/cockroach
3 participants
@knz @erikgrinaker @pav-kv