changefeedccl: deflake TestChangefeedRetryableSinkError #30172
Conversation
|
This change is |
There was a problem hiding this comment.
Excellent, thanks for tracking that down so quickly. LGTM.
Reviewable status:
complete! 0 of 0 LGTMs obtained
Me too.
Neither says definitively that |
|
Yeah, looks pretty unsynchronized to me: https://github.com/golang/go/blob/de28555c0b33fcaa02779d55ea9289135280ae9f/src/runtime/chan.go#L671 |
|
Er, I guess that's just the code used by reflect. The actual code is always inlined by the compiler, but it again looks pretty darn unsynchronized: https://github.com/golang/go/blob/de28555c0b33fcaa02779d55ea9289135280ae9f/src/cmd/compile/internal/gc/ssa.go#L4421 |
TestChangefeedRetryableSinkError was performing many rounds of bcrypt, which is excruciatingly slow when the race detector is on. Put its test cluster into insecure mode to skip bcrypt hashing. Fix cockroachdb#30156. Fix cockroachdb#30161. Release note: None
The previous pattern used to drain a channel
if len(ch) > 0 {
<-ch
}
is considered a race by the race detector if close(ch) occurs at the
same time. Adjust the channel-draining code to be race-safe by instead
perfomring repeated nonblocking reads.
Fix cockroachdb#30159.
Release note: None
There was a problem hiding this comment.
Nice catch! LGTM.
Reviewed 2 of 2 files at r1, 1 of 1 files at r2.
Reviewable status:
pkg/storage/store.go, line 1751 at r2 (raw file):
case <-ch: // Drain all notifications from the channel. loop:
TIL! Thanks.
|
bors r=nvanbenschoten,arjunravinarayan,mrtracy |
30172: changefeedccl: deflake TestChangefeedRetryableSinkError r=nvanbenschoten,arjunravinarayan,mrtracy a=benesch TestChangefeedRetryableSinkError was performing many rounds of bcrypt, which is excruciatingly slow when the race detector is on. Put its test cluster into insecure mode to skip bcrypt hashing. Fix #30156. Fix #30161. Release note: None Co-authored-by: Nikhil Benesch <nikhil.benesch@gmail.com>
Build succeeded |
|
The |
TestChangefeedRetryableSinkError was performing many rounds of bcrypt,
which is excruciatingly slow when the race detector is on. Put its test
cluster into insecure mode to skip bcrypt hashing.
Fix #30156.
Fix #30161.
Release note: None