changefeedccl: use correct channel size in parallelio

[jayshrivastava]

Previously, the channels used for sending requests and receiving results were too small. This meant that a caller could block on sending a request even after acquiring quota. This change ensures that the size of the channels is large enough so that this blocking does not occur.

Closes: #118463
Closes: #118462
Closes: #118461
Closes: #118460
Closes: #118459
Closes: #118458
Epic: none

[blathers-crl]

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

_{🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.}

[cockroach-teamcity]

This change is 

[jayshrivastava]

I stress tested TestChangefeedEndTimeWithCursor. It would fail consistently before this change. Now --stress passes.

[rharding6373]

Good find, thanks for the quick fix! I have a question about memory accounting

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @jayshrivastava and @miretskiy)

---

pkg/ccl/changefeedccl/parallel_io.go line 97 at r1 (raw file):

		// NB: The size of these channels should not be less than the quota. This prevents the producer from
		// blocking on sending requests which have been admitted.
		requestCh: make(chan AdmittedIORequest, quota),

Do we do memory accounting for the channel size?

[jayshrivastava]

Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @miretskiy and @rharding6373)

---

pkg/ccl/changefeedccl/parallel_io.go line 97 at r1 (raw file):

Previously, rharding6373 (Rachael Harding) wrote…

Do we do memory accounting for the channel size?

We don't need to memory account because the requests are batches which contain cdc events and these cdc events are accounted for. The overhead is considered negligible. They get freed by batch.alloc.Release after the result comes back:

cockroach/pkg/ccl/changefeedccl/batching_sink.go

Lines 346 to 367 in cb674c2

	handleResult := func(result IOResult) {
	req, err := result.Consume()
	batch, _ := req.(*sinkBatch)
	if err != nil {
	s.handleError(err)
	} else {
	s.metrics.recordEmittedBatch(
	batch.bufferTime, batch.numMessages, batch.mvcc, batch.numKVBytes, sinkDoesNotCompress,
	)
	}
	inflight -= batch.numMessages
	if (err != nil || inflight == 0) && sinkFlushWaiter != nil {
	close(sinkFlushWaiter)
	sinkFlushWaiter = nil
	}
	batch.alloc.Release(ctx)
	freeSinkBatchEvent(batch)
	}

Event allocs are created here here

cockroach/pkg/ccl/changefeedccl/kvevent/blocking_buffer.go

Lines 271 to 275 in 3595935

	alloc, err := b.AcquireMemory(ctx, n)
	if err != nil {
	return err
	}
	e.alloc = alloc

[jayshrivastava]

bors r+

[craig]

Build succeeded:

• Bazel Essential CI (Cockroach)