feat(streaming): follow server.connection_pool_size config for exchange service

[BugenZhao]

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

A connection_pool_size can be specified to establish multiple connections to the same server. We didn't pass this configuration field to the client pool used by streaming exchange, which is the issue to address in this PR.

This can help to mitigate potential streaming stuck due to the deadlock caused by HTTP/2 streaming vs connection-level flow control.

Remove Default impl to make specifying the connection_pool_size explicit to caller. Use adhoc() or new(1) if the caller doesn't care about the pool size.

Checklist

• I have written necessary rustdoc comments
• I have added necessary unit tests and integration tests
• All checks passed in ./risedev check (or alias, ./risedev c)

Documentation

• My PR needs documentation updates. (Please use the Release note section below to summarize the impact on users)

Release note

If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.

[chenzl25]

LGTM. The reason we introduced a connection pool previously is that a single connection became a bottleneck for batch queries between frontend nodes and compute nodes. If the bottleneck appears in the streaming exchange channel, using a connection pool seems promising to resolve deadlocks.

[kwannoel]

Generally LGTM. But I think an improvement would be to use a separate configuration for batch and streaming connection pools.

Currently both use the same connection_pool_size config, so batch and stream are not isolated for each other's configuration change for this var.

By default our connection pool size for stream is 1. Not sure if the same default can apply to batch.

[BugenZhao]

But I think an improvement would be to use a separate configuration for batch and streaming connection pools.

Currently both use the same connection_pool_size config, so batch and stream are not isolated for each other's configuration change for this var.

Agree. Since connection_pool_size was only used for batch exchange, It would be better if it was put in batch.developer from the very beginning. Then we can introduce another configuration for streaming exchange, naturally and without any compatibility issue.

By default our connection pool size for stream is 1. Not sure if the same default can apply to batch.

After this PR, it will be 16, but I don't think it's harmful.

[BugenZhao]

• refactor: use separate configuration for exchange connection pool for batch and streaming #17768
• feat(streaming): follow server.connection_pool_size config for exchange service #17755 👈
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @BugenZhao and the rest of your teammates on Graphite