upstream: add per-host preconnect eligibility policy - #46153
Conversation
Adds an optional MetadataMatcher, Cluster.PreconnectPolicy.preconnect_enabled_metadata, that gates anticipatory upstream connections per host. Non-matching hosts are never preconnected but still get connections on demand when serving requests. Suppressed preconnects increment the new upstream_cx_preconnect_skipped counter. With no matcher configured, all hosts stay eligible, preserving backwards compatibility. Signed-off-by: Violeta Ilieva <vilieva@netflix.com>
|
Hi @vilieva, welcome and thank you for your contribution. We will try to review your Pull Request as quickly as possible. In the meantime, please take a look at the contribution guidelines if you have not done so already. |
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
|
@vilieva please merge main to resolve conflicts, so CI can run. |
|
/wait |
…igibility Co-authored-by: Cursor <cursoragent@cursor.com> # Conflicts: # api/envoy/config/cluster/v3/BUILD # api/envoy/config/cluster/v3/cluster.proto
|
main is merged, thanks for taking a look! |
|
Looks good. Will wait for @adisuissa to LGTM API. /wait-any |
|
Thanks! I used this field as inspiration for the API change - I hope my usage is consistent. |
|
Any other context I can provide to help sus this out? Or any use cases I can investigate? |
|
gently ping @yanavlasov |
|
Since @yanavlasov previously approved and it was only blocked on the API review, I think it should be okay to merge. |
Commit Message: upstream: add per-host preconnect eligibility policy
Additional Description:
Adds an optional MetadataMatcher, Cluster.PreconnectPolicy.preconnect_enabled_metadata,
that gates anticipatory upstream connections per host. Non-matching hosts are never
preconnected but still get connections on demand when serving requests. Suppressed
preconnects increment the new upstream_cx_preconnect_skipped counter. With no matcher
configured, all hosts stay eligible, preserving backwards compatibility.
Portions of the implementation and tests were developed with generative-AI assistance; I
fully understand and take ownership of the code.
Risk Level: Low; new behavior is opt-in. The only always-on change is that
shouldCreateNewConnectionconsults per-host eligibility, which is empty for clusters thathave not opted in.
Testing:
//test/common/conn_pool:conn_pool_base_test(eligible/ineligible preconnect,explicit preconnect, and skip accounting).
//test/common/upstream:cluster_manager_misc_test(shouldPreconnectwith andwithout a configured matcher).
//test/integration:integration_test(skips ineligible host, allowseligible host, verifies no over-count of the skip counter when no preconnect is wanted).
Docs Changes:
preconnect_enabled_metadatafield.upstream_cx_preconnect_skippedcounter documented in the cluster stats reference.Release Notes:
changelogs/current/new_features/upstream__added-preconnect-eligibility-matcher.rstPlatform Specific Features: None.
[Optional Runtime guard:] None. The change is opt-in and default behavior is unchanged; I
can add a runtime guard for the
shouldCreateNewConnectionpath if a reviewer prefers.[Optional Fixes #Issue:] Fixes #46151
[Optional API Considerations:] This PR modifies the
api/tree; the API Review Checklist(https://github.com/envoyproxy/envoy/blob/main/api/review_checklist.md) is addressed in
the linked issue. Summary: opt-in with backwards-compatible default, reuses the common
type.matcher.v3.MetadataMatcher, added as a first-classPreconnectPolicyfield ratherthan an extension point (the matcher already provides the needed flexibility), failure
mode falls back to on-demand connections, and per-decision cost is limited to clusters
that configure a matcher.