admission: squash data race in accessing token bucket#88279
admission: squash data race in accessing token bucket#88279craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
sumeerbhola
left a comment
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @irfansharif)
pkg/util/admission/elastic_cpu_granter.go line 102 at r1 (raw file):
ctx context.Context mu struct { syncutil.Mutex
My understanding is that there is no lock ordering between this Mutex and the one in WorkQueue, since neither holds its own mutex when calling into the other. Is that correct?
Worth documenting this in a code comment, since this is different from the other granters: the granters that are used by GrantCoordinator don't have their own mutex, and rely on the one in GrantCoordinator, which is ordered before the one in WorkQueue, since requester.granted() is called while holding GrantCoordinator.mu.
Fixes cockroachdb#88269, fallout from cockroachdb#88031 -- sloppy assumption that the token bucket type is thread-safe. Instantly reproduced using: dev test pkg/ccl/changefeedccl \ -f=TestAlterChangefeedAddTarget/cloudstorage \ --race --stress --timeout 1m Release note: None
62f89d6 to
5862861
Compare
irfansharif
left a comment
There was a problem hiding this comment.
bors r+
Reviewable status:
pkg/util/admission/elastic_cpu_granter.go line 102 at r1 (raw file):
Previously, sumeerbhola wrote…
My understanding is that there is no lock ordering between this Mutex and the one in WorkQueue, since neither holds its own mutex when calling into the other. Is that correct?
Worth documenting this in a code comment, since this is different from the other granters: the granters that are used by GrantCoordinator don't have their own mutex, and rely on the one in GrantCoordinator, which is ordered before the one in WorkQueue, sincerequester.granted()is called while holdingGrantCoordinator.mu.
Done.
|
Build failed (retrying...): |
|
Build succeeded: |
Fixes #88269, fallout from #88031 -- sloppy assumption that the token bucket type is thread-safe. Instantly reproduced using:
Release note: None
Release justification: Fixes data race in non-production code