Skip to content

upstream: allow configuration of connection pool limits#6298

Merged
mattklein123 merged 20 commits intoenvoyproxy:masterfrom
klarose:add_limit_config
Mar 26, 2019
Merged

upstream: allow configuration of connection pool limits#6298
mattklein123 merged 20 commits intoenvoyproxy:masterfrom
klarose:add_limit_config

Conversation

@klarose
Copy link
Copy Markdown
Contributor

@klarose klarose commented Mar 15, 2019

Description: We recently added a connection pool map class which maps keys to connection pools. A new key means a new connection pool. To prevent unbounded growth of connection pools, we placed a limit on the pool. Allocating beyond the limit causes the map to attempt to reclaim unused pools prior to allocating a new one. If none can be reclaimed, the map returns a failure.

We did not add the ability to configure the limit; its default was unlimited. This commit adds the ability to configure a limit by extending the cluster circuit breaker thresholds with a new optional configuration field: max_connection_pools.

Worth noting:

  1. The default if no configuration is set is still unlimited.
  2. The configuration is per priority, like the other thresholds.
  3. The per priority configuration required some small refactoring: I added a class which creates one connection pool map per priority, so that each connection pool map can track its resources independently.
  4. I added an extremely simple integration test which shows the behaviour when the limit is reached.

Risk Level: Medium
We're in the main code path for http. But, the default behaviour should not have changed, so there shouldn't be any major impact.

Testing:
Unit testing + integration testing
I spun up a local envoy instance, showing that:

  1. When the limit was reached, I got a 503 back
  2. If a pool was idle, even though I was at the limit, an old pool would be recycled and the request would go through.

Docs Changes: Protobuf docs + additions to the circuit breaking section in the overview.

Release Notes: Added a release not for the new circuit breaker.

Loading
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants