Skip to content

configure buffers from target Params#2173

Merged
olix0r merged 6 commits intomainfrom
eliza/queue-params
Jan 21, 2023
Merged

configure buffers from target Params#2173
olix0r merged 6 commits intomainfrom
eliza/queue-params

Conversation

@hawkw
Copy link
Contributor

@hawkw hawkw commented Jan 20, 2023

Buffer services in the proxy are constructed with a queue capacity and a
failfast timeout. Currently, these parameters are configured based on
the proxy's config structs (parsed from environment variables), and each
buffer Layer is created with a fixed buffer configuration depending on
which stack it is used in, so all buffers constructed by that stack will
have the same configuration. In the future, we would like to allow these
parameters to be configured per-target by the control plane.

In order to support configuring buffer parameters dynamically, this
branch introduces a NewQueue type, which implements NewService and
produces Buffer + FailFast services configured using a QueueConfig
Param extracted from the target. This will allow target types to
implement Param<QueueConfig> to dynamically configure the buffer
parameters.

In order to continue supporting the current state of the world (i.e.,
fixed buffer parameters) without adding them to every target type,
NewQueue's NewService implementation is generic over an
ExtractParam implementation to control how the QueueConfig params
are extracted from targets. Convenience constructors are provided for
() as the ExtractParam type, expecting that T: Param<QueueConfig>,
and for CloneParam as the ExtractParam type, to clone the same set
of params every time a buffer is constructed. Currently, all NewQueue
layers constructed in the proxy use CloneParam, to continue
implementing the existing fixed configuration behavior.

Notes

  • This branch also removes the name field passed when constructing
    buffers. The name is no longer passed to FailFast and doesn't
    currently do anything, but is still part of the push_buffer methods.
    Since this PR changed how buffers are constructed anyway, it didn't
    make sense to continue taking a name field. This may cause a small
    merge conflict with add target metadata to error contexts #2162, but it shouldn't be a huge deal.

  • A note on naming: I decided to name the NewService type NewQueue
    rather than NewBuffer, and renamed the configuration params from
    BufferConfig to QueueConfig. I thought it was clearer to refer to
    these services as queues, rather than as buffers (despite the fact
    that Tower calls the service Buffer), because the naming
    BufferConfig could be interpreted as configuring the byte buffers
    that bytes from the network are read into, especially for readers who
    aren't familiar with Tower's naming scheme. Also, @olix0r's draft PR
    start unifying policy types for outbound #2151 named the client policy configuration type for buffer layer
    configs Queue rather than Buffer, so I thought I'd go with that
    naming here as well.

    However, I'm not super attached to this naming scheme, and am open to
    bikeshed over it. I can rename the uses of "queue" to "buffer" if
    consistency with Tower's naming scheme is important to us.

@hawkw hawkw requested a review from olix0r January 20, 2023 19:07
@hawkw hawkw self-assigned this Jan 20, 2023
@hawkw hawkw requested a review from a team as a code owner January 20, 2023 19:07
Copy link
Member

@olix0r olix0r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll push fixes for these suggestions.

pub failfast_timeout: Duration,
}

pub struct NewQueue<N, Req, X = ()> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I generally prefer to keep the service types last (for type readability in debug output). I don't think the default parameter is really worth it in this case.

}
}

impl<T: Param<QueueConfig>, Req> NewQueue<T, Req> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This type signature doesn't look right to me. T here is the NewService type. We don't want any type constraints here. In general, we should use N for the primary inner new service.


impl<T, Req, X> NewQueue<T, Req, X>
where
X: ExtractParam<QueueConfig, T> + Clone,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly

@olix0r olix0r merged commit 5434242 into main Jan 21, 2023
@olix0r olix0r deleted the eliza/queue-params branch January 21, 2023 06:41
olix0r added a commit to linkerd/linkerd2 that referenced this pull request Feb 16, 2023
This release includes many internal changes to prepare for the new
client policy API. Stack metric label values have changed to reflect the
new shape of the outbound proxy.

---

* build(deps): bump try-lock from 0.2.3 to 0.2.4 (linkerd/linkerd2-proxy#2139)
* build(deps): bump regex from 1.7.0 to 1.7.1 (linkerd/linkerd2-proxy#2145)
* build(deps): bump tokio from 1.24.0 to 1.24.1 (linkerd/linkerd2-proxy#2144)
* Parameterize the load balancer stack (linkerd/linkerd2-proxy#2142)
* build(deps): bump prost-types from 0.11.5 to 0.11.6 (linkerd/linkerd2-proxy#2147)
* build(deps): bump prost from 0.11.5 to 0.11.6 (linkerd/linkerd2-proxy#2148)
* build(deps): bump prost-build from 0.11.5 to 0.11.6 (linkerd/linkerd2-proxy#2149)
* stack: add `AnnotateError` middleware (linkerd/linkerd2-proxy#2158)
* Fix proxy-core dependencies (linkerd/linkerd2-proxy#2163)
* build(deps): bump tj-actions/changed-files from 35.3.1 to 35.4.1 (linkerd/linkerd2-proxy#2153)
* orig_proto: don't set `connection: close` on errors (linkerd/linkerd2-proxy#2171)
* build(deps): bump bumpalo from 3.11.1 to 3.12.0 (linkerd/linkerd2-proxy#2166)
* build(deps): bump proc-macro2 from 1.0.49 to 1.0.50 (linkerd/linkerd2-proxy#2165)
* build(deps): bump tj-actions/changed-files from 35.4.1 to 35.4.4 (linkerd/linkerd2-proxy#2172)
* build(deps): bump windows_x86_64_msvc from 0.42.0 to 0.42.1 (linkerd/linkerd2-proxy#2164)
* configure buffers from target `Param`s (linkerd/linkerd2-proxy#2173)
* Simplify profile discovery (linkerd/linkerd2-proxy#2170)
* stack: Unify AnnotateError and MapErr (linkerd/linkerd2-proxy#2180)
* build(deps): bump windows_aarch64_msvc from 0.42.0 to 0.42.1 (linkerd/linkerd2-proxy#2176)
* build(deps): bump async-trait from 0.1.61 to 0.1.63 (linkerd/linkerd2-proxy#2177)
* build(deps): bump tokio from 1.24.1 to 1.24.2 (linkerd/linkerd2-proxy#2178)
* Add the `meshtls-boring-fips` feature flag (linkerd/linkerd2-proxy#2168)
* Update HTTP error responder to log version info (linkerd/linkerd2-proxy#2182)
* build(deps): bump which from 4.3.0 to 4.4.0 (linkerd/linkerd2-proxy#2185)
* build(deps): bump derive_arbitrary from 1.2.2 to 1.2.3 (linkerd/linkerd2-proxy#2184)
* build(deps): bump unicode-bidi from 0.3.8 to 0.3.10 (linkerd/linkerd2-proxy#2183)
* build(deps): bump linkerd/dev from 38 to 39 (linkerd/linkerd2-proxy#2175)
* add target metadata to error contexts (linkerd/linkerd2-proxy#2162)
* build(deps): bump rustls from 0.20.7 to 0.20.8 (linkerd/linkerd2-proxy#2187)
* build(deps): bump ahash from 0.8.2 to 0.8.3 (linkerd/linkerd2-proxy#2188)
* build(deps): bump matches from 0.1.9 to 0.1.10 (linkerd/linkerd2-proxy#2189)
* Rename MakeThunk to NewThunk (linkerd/linkerd2-proxy#2197)
* Add `NewQueueWithoutTimeout` (linkerd/linkerd2-proxy#2196)
* Cache discovery results independently of proxy stacks  (linkerd/linkerd2-proxy#2195)
* core: Rename Stack utilities for clarity (linkerd/linkerd2-proxy#2199)
* gateway: Unify discovery for HTTP & opaque stacks (linkerd/linkerd2-proxy#2198)
* build(deps): bump libfuzzer-sys from 0.4.5 to 0.4.6 (linkerd/linkerd2-proxy#2193)
* build(deps): bump arbitrary from 1.2.2 to 1.2.3 (linkerd/linkerd2-proxy#2191)
* http: Remove `Clone` requirement in servers (linkerd/linkerd2-proxy#2200)
* build(deps): bump either from 1.8.0 to 1.8.1 (linkerd/linkerd2-proxy#2192)
* build(deps): bump bytes from 1.3.0 to 1.4.0 (linkerd/linkerd2-proxy#2202)
* build(deps): bump cc from 1.0.78 to 1.0.79 (linkerd/linkerd2-proxy#2203)
* build(deps): bump tj-actions/changed-files from 35.4.4 to 35.5.1 (linkerd/linkerd2-proxy#2211)
* build(deps): bump tokio from 1.24.2 to 1.25.0 (linkerd/linkerd2-proxy#2206)
* build(deps): bump miniz_oxide from 0.6.2 to 0.6.4 (linkerd/linkerd2-proxy#2207)
* build(deps): bump async-trait from 0.1.63 to 0.1.64 (linkerd/linkerd2-proxy#2205)
* Split outbound test modules into files (linkerd/linkerd2-proxy#2213)
* Disable broken tests (linkerd/linkerd2-proxy#2214)
* Add traceparent header parsing for w3c tracecontext (linkerd/linkerd2-proxy#2179)
* Simplify the `Resolve` trait alias (linkerd/linkerd2-proxy#2218)
* downgrade `miniz_oxide` from yanked 0.6.4 to 0.6.2 (linkerd/linkerd2-proxy#2219)
* build(deps): bump heck from 0.4.0 to 0.4.1 (linkerd/linkerd2-proxy#2215)
* ci: Fix check-each workflow (linkerd/linkerd2-proxy#2222)
* Use a cascading stack with protocol detection (linkerd/linkerd2-proxy#2221)
* ci: Fix quotation in list-crates (linkerd/linkerd2-proxy#2225)
* outbound: Split out separate 'opaq' modules (linkerd/linkerd2-proxy#2224)
* Update `linkerd2-proxy-api` to v0.8.0 (linkerd/linkerd2-proxy#2223)
* outbound: Lint stack target types (linkerd/linkerd2-proxy#2226)
* outbound: Split sidecar and ingress stack modules (linkerd/linkerd2-proxy#2227)
* gateway: Split 'http' and 'opaq' modules (linkerd/linkerd2-proxy#2230)
* test: Disable tap::rejects_incorrect_identity_when_identity_is_expected (linkerd/linkerd2-proxy#2231)
* outbound: Improve discovery cache test (linkerd/linkerd2-proxy#2233)
* integration: add destination update builders (linkerd/linkerd2-proxy#2232)
* Rename linkerd-server-policy to linkerd-proxy-server-policy (linkerd/linkerd2-proxy#2235)
* integration: add test for direct HTTP connections (linkerd/linkerd2-proxy#2234)
* outbound: Refactor stack target types (linkerd/linkerd2-proxy#2210)
* Add client-policy types (linkerd/linkerd2-proxy#2236)

Signed-off-by: Oliver Gould <ver@buoyant.io>
olix0r added a commit to linkerd/linkerd2 that referenced this pull request Feb 17, 2023
This release includes many internal changes to prepare for the new
client policy API. Stack metric label values have changed to reflect the
new shape of the outbound proxy.

This change also includes some test improvements that helped debug an
issue while merging this.

---

* build(deps): bump try-lock from 0.2.3 to 0.2.4 (linkerd/linkerd2-proxy#2139)
* build(deps): bump regex from 1.7.0 to 1.7.1 (linkerd/linkerd2-proxy#2145)
* build(deps): bump tokio from 1.24.0 to 1.24.1 (linkerd/linkerd2-proxy#2144)
* Parameterize the load balancer stack (linkerd/linkerd2-proxy#2142)
* build(deps): bump prost-types from 0.11.5 to 0.11.6 (linkerd/linkerd2-proxy#2147)
* build(deps): bump prost from 0.11.5 to 0.11.6 (linkerd/linkerd2-proxy#2148)
* build(deps): bump prost-build from 0.11.5 to 0.11.6 (linkerd/linkerd2-proxy#2149)
* stack: add `AnnotateError` middleware (linkerd/linkerd2-proxy#2158)
* Fix proxy-core dependencies (linkerd/linkerd2-proxy#2163)
* build(deps): bump tj-actions/changed-files from 35.3.1 to 35.4.1 (linkerd/linkerd2-proxy#2153)
* orig_proto: don't set `connection: close` on errors (linkerd/linkerd2-proxy#2171)
* build(deps): bump bumpalo from 3.11.1 to 3.12.0 (linkerd/linkerd2-proxy#2166)
* build(deps): bump proc-macro2 from 1.0.49 to 1.0.50 (linkerd/linkerd2-proxy#2165)
* build(deps): bump tj-actions/changed-files from 35.4.1 to 35.4.4 (linkerd/linkerd2-proxy#2172)
* build(deps): bump windows_x86_64_msvc from 0.42.0 to 0.42.1 (linkerd/linkerd2-proxy#2164)
* configure buffers from target `Param`s (linkerd/linkerd2-proxy#2173)
* Simplify profile discovery (linkerd/linkerd2-proxy#2170)
* stack: Unify AnnotateError and MapErr (linkerd/linkerd2-proxy#2180)
* build(deps): bump windows_aarch64_msvc from 0.42.0 to 0.42.1 (linkerd/linkerd2-proxy#2176)
* build(deps): bump async-trait from 0.1.61 to 0.1.63 (linkerd/linkerd2-proxy#2177)
* build(deps): bump tokio from 1.24.1 to 1.24.2 (linkerd/linkerd2-proxy#2178)
* Add the `meshtls-boring-fips` feature flag (linkerd/linkerd2-proxy#2168)
* Update HTTP error responder to log version info (linkerd/linkerd2-proxy#2182)
* build(deps): bump which from 4.3.0 to 4.4.0 (linkerd/linkerd2-proxy#2185)
* build(deps): bump derive_arbitrary from 1.2.2 to 1.2.3 (linkerd/linkerd2-proxy#2184)
* build(deps): bump unicode-bidi from 0.3.8 to 0.3.10 (linkerd/linkerd2-proxy#2183)
* build(deps): bump linkerd/dev from 38 to 39 (linkerd/linkerd2-proxy#2175)
* add target metadata to error contexts (linkerd/linkerd2-proxy#2162)
* build(deps): bump rustls from 0.20.7 to 0.20.8 (linkerd/linkerd2-proxy#2187)
* build(deps): bump ahash from 0.8.2 to 0.8.3 (linkerd/linkerd2-proxy#2188)
* build(deps): bump matches from 0.1.9 to 0.1.10 (linkerd/linkerd2-proxy#2189)
* Rename MakeThunk to NewThunk (linkerd/linkerd2-proxy#2197)
* Add `NewQueueWithoutTimeout` (linkerd/linkerd2-proxy#2196)
* Cache discovery results independently of proxy stacks  (linkerd/linkerd2-proxy#2195)
* core: Rename Stack utilities for clarity (linkerd/linkerd2-proxy#2199)
* gateway: Unify discovery for HTTP & opaque stacks (linkerd/linkerd2-proxy#2198)
* build(deps): bump libfuzzer-sys from 0.4.5 to 0.4.6 (linkerd/linkerd2-proxy#2193)
* build(deps): bump arbitrary from 1.2.2 to 1.2.3 (linkerd/linkerd2-proxy#2191)
* http: Remove `Clone` requirement in servers (linkerd/linkerd2-proxy#2200)
* build(deps): bump either from 1.8.0 to 1.8.1 (linkerd/linkerd2-proxy#2192)
* build(deps): bump bytes from 1.3.0 to 1.4.0 (linkerd/linkerd2-proxy#2202)
* build(deps): bump cc from 1.0.78 to 1.0.79 (linkerd/linkerd2-proxy#2203)
* build(deps): bump tj-actions/changed-files from 35.4.4 to 35.5.1 (linkerd/linkerd2-proxy#2211)
* build(deps): bump tokio from 1.24.2 to 1.25.0 (linkerd/linkerd2-proxy#2206)
* build(deps): bump miniz_oxide from 0.6.2 to 0.6.4 (linkerd/linkerd2-proxy#2207)
* build(deps): bump async-trait from 0.1.63 to 0.1.64 (linkerd/linkerd2-proxy#2205)
* Split outbound test modules into files (linkerd/linkerd2-proxy#2213)
* Disable broken tests (linkerd/linkerd2-proxy#2214)
* Add traceparent header parsing for w3c tracecontext (linkerd/linkerd2-proxy#2179)
* Simplify the `Resolve` trait alias (linkerd/linkerd2-proxy#2218)
* downgrade `miniz_oxide` from yanked 0.6.4 to 0.6.2 (linkerd/linkerd2-proxy#2219)
* build(deps): bump heck from 0.4.0 to 0.4.1 (linkerd/linkerd2-proxy#2215)
* ci: Fix check-each workflow (linkerd/linkerd2-proxy#2222)
* Use a cascading stack with protocol detection (linkerd/linkerd2-proxy#2221)
* ci: Fix quotation in list-crates (linkerd/linkerd2-proxy#2225)
* outbound: Split out separate 'opaq' modules (linkerd/linkerd2-proxy#2224)
* Update `linkerd2-proxy-api` to v0.8.0 (linkerd/linkerd2-proxy#2223)
* outbound: Lint stack target types (linkerd/linkerd2-proxy#2226)
* outbound: Split sidecar and ingress stack modules (linkerd/linkerd2-proxy#2227)
* gateway: Split 'http' and 'opaq' modules (linkerd/linkerd2-proxy#2230)
* test: Disable tap::rejects_incorrect_identity_when_identity_is_expected (linkerd/linkerd2-proxy#2231)
* outbound: Improve discovery cache test (linkerd/linkerd2-proxy#2233)
* integration: add destination update builders (linkerd/linkerd2-proxy#2232)
* Rename linkerd-server-policy to linkerd-proxy-server-policy (linkerd/linkerd2-proxy#2235)
* integration: add test for direct HTTP connections (linkerd/linkerd2-proxy#2234)
* outbound: Refactor stack target types (linkerd/linkerd2-proxy#2210)
* Add client-policy types (linkerd/linkerd2-proxy#2236)
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.

2 participants