Debug concurrent streams test

[emcfarlane]

Locally increasing the number of concurrent streams and running with -race generates:

writeRequest io.ErrClosedPipe
2023/11/03 10:19:46 timeout waiting for SETTINGS frames from pipe
writeRequest io.ErrClosedPipe
--- FAIL: TestConcurrentStreams (35.69s)
    connect_ext_test.go:490: failed to send request: unknown: write envelope: EOF
    connect_ext_test.go:490: failed to send request: unknown: write envelope: EOF

Large number of go routines can timeout past the arbitrary limit of 2 seconds in the HTTP2 libraries. See comment here.
Bind the concurrency to runtime.GOMAXPROCS to reduce resource requirements for testing on different machines.

See golang/go#60359

Occurred in https://github.com/connectrpc/connect-go/actions/runs/6725037978/job/18278596900?pr=611

[jhump]

LGTM. But I'm worried that maybe now we're being too conservative with the number of concurrent operations. Any reason not to lift this a bit before merging?

[jhump]

In case GOMAXPROCS is very low, can we use a slightly higher multiplier? Like maybe 10? Since the test is meant to exercise concurrency, having only two concurrent streams (if GOMAXPROCS is 1) seems potentially ineffective. Also, this likely defaults to 8 or 10 on your MBP, which means we're only doing 16 or 20 requests, which seems quite hard to believe it could exceed 2 seconds. So the multiplier of 2 seems like it may be overly cautious.

Also, do we know the actual value of this in CI? It was previously resolving to 100 operations. How many does it do now, in practice? Maybe we also should t.Log the actual duration of the operations, so we can see if/when we're getting close to the 2 second limit?

[emcfarlane]

What's the value in increasing the concurrency further? *2 ensures theres some level of concurrency. If GOMAXPROCS is 1, 2 will still be testing concurrent execution.

[jhump]

I was hoping for stronger guarantee of concurrent activity -- the scheduler is non-deterministic, so it seems trivial for this to allow one goroutine to complete before the other actually starts. I suppose another way to force concurrency might be to call Send(nil) (to make sure streams are actually started) before calling start.Wait().

But I guess it also depends on what this is trying to test. The previous value of 100 is quite high, so I assumed it wanted to be a sort of stress test. And in that case, reducing all the way from 100 down to 2 streams seems rather insufficient. So, based on my interpretation of the intent, I would expect to see more than just 2 here.

[emcfarlane]

The windows runner has more cores. They are relatively low and maybe would catch more with more concurrency. Each stream does do 100 back on forth on the stream per go rountine. So I thought it would be good enough to just have some parallelism for the test. Will bump to 8 and can always lower later.

Windows:

    connect_ext_test.go:514: NUMCPU 4
    connect_ext_test.go:515: GOMAXPROCS 4

Linux:

    connect_ext_test.go:514: NUMCPU 2
    connect_ext_test.go:515: GOMAXPROCS 2