Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow configuration of upstream connection limits in Envoy #6829

Merged
merged 6 commits into from
Dec 3, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions agent/xds/clusters.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,9 @@ func (s *Server) makeUpstreamClusterForPreparedQuery(upstream structs.Upstream,
},
},
},
CircuitBreakers: &envoycluster.CircuitBreakers{
Thresholds: makeThresholdsIfNeeded(cfg.Limits),
},
// Having an empty config enables outlier detection with default config.
OutlierDetection: &envoycluster.OutlierDetection{},
}
Expand Down Expand Up @@ -339,6 +342,9 @@ func (s *Server) makeUpstreamClustersForDiscoveryChain(
},
},
},
CircuitBreakers: &envoycluster.CircuitBreakers{
Thresholds: makeThresholdsIfNeeded(cfg.Limits),
},
// Having an empty config enables outlier detection with default config.
OutlierDetection: &envoycluster.OutlierDetection{},
}
Expand Down Expand Up @@ -449,3 +455,27 @@ func (s *Server) makeMeshGatewayCluster(clusterName string, cfgSnap *proxycfg.Co
OutlierDetection: &envoycluster.OutlierDetection{},
}, nil
}

func makeThresholdsIfNeeded(limits UpstreamLimits) []*envoycluster.CircuitBreakers_Thresholds {
var empty UpstreamLimits
// Make sure to not create any thresholds when passed the zero-value in order
// to rely on Envoy defaults
if limits == empty {
return nil
}

threshold := &envoycluster.CircuitBreakers_Thresholds{}
// Likewise, make sure to not set any threshold values on the zero-value in
// order to rely on Envoy defaults
if limits.MaxConnections != 0 {
crhino marked this conversation as resolved.
Show resolved Hide resolved
threshold.MaxConnections = makeUint32Value(limits.MaxConnections)
}
if limits.MaxPendingRequests != 0 {
threshold.MaxPendingRequests = makeUint32Value(limits.MaxPendingRequests)
}
if limits.MaxConcurrentRequests != 0 {
threshold.MaxRequests = makeUint32Value(limits.MaxConcurrentRequests)
}

return []*envoycluster.CircuitBreakers_Thresholds{threshold}
}
42 changes: 42 additions & 0 deletions agent/xds/clusters_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,42 @@ func TestClustersFromSnapshot(t *testing.T) {
snap.Proxy.Upstreams[0].Config["connect_timeout_ms"] = 2345
},
},
{
name: "custom-limits-max-connections-only",
create: proxycfg.TestConfigSnapshot,
setup: func(snap *proxycfg.ConfigSnapshot) {
for i := range snap.Proxy.Upstreams {
// We check if Config is nil because the prepared_query upstream is
// initialized without a Config map. Use Upstreams[i] syntax to
// modify the actual ConfigSnapshot instead of copying the Upstream
// in the range.
if snap.Proxy.Upstreams[i].Config == nil {
snap.Proxy.Upstreams[i].Config = map[string]interface{}{}
}

snap.Proxy.Upstreams[i].Config["limits"] = map[string]interface{}{
"max_connections": 500,
}
}
},
},
{
name: "custom-limits",
create: proxycfg.TestConfigSnapshot,
setup: func(snap *proxycfg.ConfigSnapshot) {
for i := range snap.Proxy.Upstreams {
if snap.Proxy.Upstreams[i].Config == nil {
snap.Proxy.Upstreams[i].Config = map[string]interface{}{}
}

snap.Proxy.Upstreams[i].Config["limits"] = map[string]interface{}{
"max_connections": 500,
"max_pending_requests": 600,
"max_concurrent_requests": 700,
}
}
},
},
{
name: "connect-proxy-with-chain",
create: proxycfg.TestConfigSnapshotDiscoveryChain,
Expand Down Expand Up @@ -312,6 +348,9 @@ func expectClustersJSONResources(t *testing.T, snap *proxycfg.ConfigSnapshot, to
},
"outlierDetection": {

},
"circuitBreakers": {

},
"altStatName": "db.default.dc1.internal.11111111-2222-3333-4444-555555555555.consul",
"commonLbConfig": {
Expand All @@ -334,6 +373,9 @@ func expectClustersJSONResources(t *testing.T, snap *proxycfg.ConfigSnapshot, to
},
"outlierDetection": {

},
"circuitBreakers": {

},
"connectTimeout": "5s",
"tlsContext": ` + expectedUpstreamTLSContextJSON(t, snap, "geo-cache.default.dc1.query.11111111-2222-3333-4444-555555555555.consul") + `
Expand Down
23 changes: 23 additions & 0 deletions agent/xds/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,25 @@ func ParseMeshGatewayConfig(m map[string]interface{}) (MeshGatewayConfig, error)
return cfg, err
}

// UpstreamLimits describes the limits that are associated with a specific
// upstream of a service instance.
type UpstreamLimits struct {
// MaxConnections is the maximum number of connections the local proxy can
// make to the upstream service.
MaxConnections int `mapstructure:"max_connections"`

// MaxPendingRequests is the maximum number of requests that will be queued
// waiting for an available connection. This is mostly applicable to HTTP/1.1
// clusters since all HTTP/2 requests are streamed over a single
// connection.
MaxPendingRequests int `mapstructure:"max_pending_requests"`

// MaxConcurrentRequests is the maximum number of in-flight requests that will be allowed
// to the upstream cluster at a point in time. This is mostly applicable to HTTP/2
// clusters since all HTTP/1.1 requests are limited by MaxConnections.
MaxConcurrentRequests int `mapstructure:"max_concurrent_requests"`
}

// UpstreamConfig describes the keys we understand from
// Connect.Proxy.Upstream[*].Config.
type UpstreamConfig struct {
Expand Down Expand Up @@ -131,6 +150,10 @@ type UpstreamConfig struct {
// ConnectTimeoutMs is the number of milliseconds to timeout making a new
// connection to this upstream. Defaults to 5000 (5 seconds) if not set.
ConnectTimeoutMs int `mapstructure:"connect_timeout_ms"`

// Limits are the set of limits that are applied to the proxy for a specific upstream of a
// service instance.
Limits UpstreamLimits `mapstructure:"limits"`
}

func ParseUpstreamConfigNoDefaults(m map[string]interface{}) (UpstreamConfig, error) {
Expand Down
19 changes: 19 additions & 0 deletions agent/xds/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,25 @@ func TestParseUpstreamConfig(t *testing.T) {
Protocol: "tcp",
},
},
{
name: "connect limits map",
input: map[string]interface{}{
"limits": map[string]interface{}{
"max_connections": 50,
"max_pending_requests": 60,
"max_concurrent_requests": 70,
},
},
want: UpstreamConfig{
ConnectTimeoutMs: 5000,
Protocol: "tcp",
Limits: UpstreamLimits{
MaxConnections: 50,
MaxPendingRequests: 60,
MaxConcurrentRequests: 70,
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
}
},
"connectTimeout": "33s",
"circuitBreakers": {

},
"tlsContext": {
"commonTlsContext": {
"tlsParams": {
Expand Down Expand Up @@ -58,6 +61,9 @@
}
},
"connectTimeout": "5s",
"circuitBreakers": {

},
"tlsContext": {
"commonTlsContext": {
"tlsParams": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
}
},
"connectTimeout": "66s",
"circuitBreakers": {

},
"tlsContext": {
"commonTlsContext": {
"tlsParams": {
Expand Down Expand Up @@ -61,6 +64,9 @@
}
},
"connectTimeout": "5s",
"circuitBreakers": {

},
"tlsContext": {
"commonTlsContext": {
"tlsParams": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
}
},
"connectTimeout": "33s",
"circuitBreakers": {

},
"tlsContext": {
"commonTlsContext": {
"tlsParams": {
Expand Down Expand Up @@ -58,6 +61,9 @@
}
},
"connectTimeout": "5s",
"circuitBreakers": {

},
"tlsContext": {
"commonTlsContext": {
"tlsParams": {
Expand Down
6 changes: 6 additions & 0 deletions agent/xds/testdata/clusters/connect-proxy-with-chain.golden
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
}
},
"connectTimeout": "33s",
"circuitBreakers": {

},
"tlsContext": {
"commonTlsContext": {
"tlsParams": {
Expand Down Expand Up @@ -58,6 +61,9 @@
}
},
"connectTimeout": "5s",
"circuitBreakers": {

},
"tlsContext": {
"commonTlsContext": {
"tlsParams": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
}
},
"connectTimeout": "33s",
"circuitBreakers": {

},
"tlsContext": {
"commonTlsContext": {
"tlsParams": {
Expand Down Expand Up @@ -58,6 +61,9 @@
}
},
"connectTimeout": "5s",
"circuitBreakers": {

},
"tlsContext": {
"commonTlsContext": {
"tlsParams": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
}
},
"connectTimeout": "33s",
"circuitBreakers": {

},
"tlsContext": {
"commonTlsContext": {
"tlsParams": {
Expand Down Expand Up @@ -58,6 +61,9 @@
}
},
"connectTimeout": "5s",
"circuitBreakers": {

},
"tlsContext": {
"commonTlsContext": {
"tlsParams": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
}
},
"connectTimeout": "33s",
"circuitBreakers": {

},
"tlsContext": {
"commonTlsContext": {
"tlsParams": {
Expand Down Expand Up @@ -58,6 +61,9 @@
}
},
"connectTimeout": "5s",
"circuitBreakers": {

},
"tlsContext": {
"commonTlsContext": {
"tlsParams": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
}
},
"connectTimeout": "33s",
"circuitBreakers": {

},
"tlsContext": {
"commonTlsContext": {
"tlsParams": {
Expand Down Expand Up @@ -58,6 +61,9 @@
}
},
"connectTimeout": "5s",
"circuitBreakers": {

},
"tlsContext": {
"commonTlsContext": {
"tlsParams": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
}
},
"connectTimeout": "33s",
"circuitBreakers": {

},
"tlsContext": {
"commonTlsContext": {
"tlsParams": {
Expand Down Expand Up @@ -58,6 +61,9 @@
}
},
"connectTimeout": "5s",
"circuitBreakers": {

},
"tlsContext": {
"commonTlsContext": {
"tlsParams": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
}
},
"connectTimeout": "33s",
"circuitBreakers": {

},
"tlsContext": {
"commonTlsContext": {
"tlsParams": {
Expand Down Expand Up @@ -58,6 +61,9 @@
}
},
"connectTimeout": "5s",
"circuitBreakers": {

},
"tlsContext": {
"commonTlsContext": {
"tlsParams": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@
}
},
"connectTimeout": "33s",
"circuitBreakers": {

},
"tlsContext": {
"commonTlsContext": {
"tlsParams": {
Expand Down Expand Up @@ -58,6 +61,9 @@
}
},
"connectTimeout": "5s",
"circuitBreakers": {

},
"tlsContext": {
"commonTlsContext": {
"tlsParams": {
Expand Down
Loading