workload/tpcc: fix --partition-affinity flag #39649
Conversation
|
This change is |
There was a problem hiding this comment.
Reviewed 1 of 1 files at r1, 1 of 1 files at r2.
Reviewable status:complete! 0 of 0 LGTMs obtained (waiting on @nvanbenschoten)
pkg/workload/tpcc/tpcc.go, line 570 at r2 (raw file):
db := dbs[warehouse%len(dbs)] ql.WorkerFns = append(ql.WorkerFns, nil)
This seems like now this can race on accessing the underlying array of ql.WorkerFns. I don't think it's a particularly problematic hazard but when the slice re-allocs you might race with a goroutine constructing a new worker below.
This was broken by b4e7796. Release note: None
This is required for all leaseholder preferences to work correctly when we run the benchmark with more than three regions. Release note: None
The commit also adds a new nightly roachtest that would have caught the regression fixed in the previous roachtest. Release note: None
nvanbenschoten
force-pushed
the
nvanbenschoten/fixTPCCGeo
branch
from
ce26a55 to
415bf29
Compare
There was a problem hiding this comment.
Reviewable status:
pkg/workload/tpcc/tpcc.go, line 570 at r2 (raw file):
Previously, ajwerner wrote…
This seems like now this can race on accessing the underlying array of
ql.WorkerFns. I don't think it's a particularly problematic hazard but when the slice re-allocs you might race with a goroutine constructing a new worker below.
Good point. Done.
There was a problem hiding this comment.
Reviewed 2 of 2 files at r3, 1 of 1 files at r4, 1 of 1 files at r5.
Reviewable status:
|
bors r=ajwerner |
39649: workload/tpcc: fix --partition-affinity flag r=ajwerner a=nvanbenschoten This was broken by b4e7796. The second commit adds a new roachtest that uses the flag and would have caught this regression. cc. @sploiselle Co-authored-by: Nathan VanBenschoten <[email protected]>
Build succeeded |
This was broken by b4e7796.
The second commit adds a new roachtest that uses the flag and would have caught this regression.
cc. @sploiselle