outbound: Avoid building balancers when no concrete name #890
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The outbound balancers are always built, even when there is no resolveable concrete address. This change modifies the outbound `Concrete` target to require a resolveable address. When no address is present, the balancer stack is skipped in favor of an endpoint stack.
hawkw
approved these changes
Feb 1, 2021
| .push_map_target(Endpoint::from_logical( | ||
| tls::NoClientTls::NotProvidedByServiceDiscovery, | ||
| )) | ||
| .check_new_service::<Logical, http::Request<_>>() |
There was a problem hiding this comment.
WDYT about sticking a separate tracing span on this to indicate that the address was not resolved?
| @@ -68,59 +64,14 @@ where | |||
| } | |||
|
|
|||
| #[derive(Clone, Debug)] | |||
| pub struct AllowResolve; | |||
| pub struct ToAddr; | |||
There was a problem hiding this comment.
Maybe worth a comment explaining what this does?
Comment on lines
+87
to
+96
| svc::stack(endpoint.clone()) | ||
| .push_on_response( | ||
| svc::layers() | ||
| .push(http::BoxRequest::layer()) | ||
| .push(http::BoxResponse::layer()), | ||
| ) | ||
| .push_map_target(Endpoint::from_logical( | ||
| tls::NoClientTls::NotProvidedByServiceDiscovery, | ||
| )) | ||
| .into_inner(), |
There was a problem hiding this comment.
TIOLI: should this stack have its own debug span? no_concrete or something?
There was a problem hiding this comment.
The endpoint stack is annotated, so it should be pretty obvious if the spans are just logical>endpoint without a concrete span
olix0r
added a commit
to linkerd/linkerd2
that referenced
this pull request
Feb 3, 2021
This release changes HTTP protocol detection to prevent timeout errors in two ways: 1. HTTP detection no longer blocks until a newline is read. We've reverted to relying on a single read to make a determination. 2. Detection timeouts are no longer terminal. When a timeout is encountered, we continue forwarding the connection as an opaque TCP connection. These changes may lead to false-negatives--we may fail to detect some HTTP streams--but it should prevent many avoidable detection errors. This release also makes improvements for multicluster gateways, improving caching so that profile lookups are only performed once per target service. Diagnostic `stack_*` metrics have been moved so that they track underlying services, ignoring fail-fast. This should help us get better insights into services that are in failfast. Finally, the opencensus exporter has been improved to ensure that trace events are flushed if the trace buffer is not filled within a timeout. --- * actions: Update actions to use full SHAs (linkerd/linkerd2-proxy#885) * http: Parameterize normalize_ur::DefaultAuthority (linkerd/linkerd2-proxy#886) * http: Parameterize the HTTP server (linkerd/linkerd2-proxy#887) * opencensus: rewrite span exporter using async/await (linkerd/linkerd2-proxy#789) * Update http::Insert to use `Param` (linkerd/linkerd2-proxy#889) * Update crate dependencies (linkerd/linkerd2-proxy#892) * stack: Make the router fallible (linkerd/linkerd2-proxy#888) * Track stack metrics within failfast (linkerd/linkerd2-proxy#891) * outbound: Avoid building balancers when no concrete name (linkerd/linkerd2-proxy#890) * inbound: Cache HTTP gateways per destination (linkerd/linkerd2-proxy#893) * Reorganize the gateway crate (linkerd/linkerd2-proxy#897) * Bias HTTP detection towards availability (linkerd/linkerd2-proxy#894) * inbound: Use ALPN to determine transport header (linkerd/linkerd2-proxy#895) * detect: Return unknown protocol on detection timeout (linkerd/linkerd2-proxy#896) * Extract protocol detection into the gateway crate (linkerd/linkerd2-proxy#898)
olix0r
added a commit
to linkerd/linkerd2
that referenced
this pull request
Feb 3, 2021
This release changes HTTP protocol detection to prevent timeout errors in two ways: 1. HTTP detection no longer blocks until a newline is read. We've reverted to relying on a single read to make a determination. 2. Detection timeouts are no longer terminal. When a timeout is encountered, we continue forwarding the connection as an opaque TCP connection. These changes may lead to false-negatives--we may fail to detect some HTTP streams--but it should prevent many avoidable detection errors. This release also makes improvements for multicluster gateways, improving caching so that profile lookups are only performed once per target service. Diagnostic `stack_*` metrics have been moved so that they track underlying services, ignoring fail-fast. This should help us get better insights into services that are in failfast. Finally, the opencensus exporter has been improved to ensure that trace events are flushed if the trace buffer is not filled within a timeout. --- * actions: Update actions to use full SHAs (linkerd/linkerd2-proxy#885) * http: Parameterize normalize_ur::DefaultAuthority (linkerd/linkerd2-proxy#886) * http: Parameterize the HTTP server (linkerd/linkerd2-proxy#887) * opencensus: rewrite span exporter using async/await (linkerd/linkerd2-proxy#789) * Update http::Insert to use `Param` (linkerd/linkerd2-proxy#889) * Update crate dependencies (linkerd/linkerd2-proxy#892) * stack: Make the router fallible (linkerd/linkerd2-proxy#888) * Track stack metrics within failfast (linkerd/linkerd2-proxy#891) * outbound: Avoid building balancers when no concrete name (linkerd/linkerd2-proxy#890) * inbound: Cache HTTP gateways per destination (linkerd/linkerd2-proxy#893) * Reorganize the gateway crate (linkerd/linkerd2-proxy#897) * Bias HTTP detection towards availability (linkerd/linkerd2-proxy#894) * inbound: Use ALPN to determine transport header (linkerd/linkerd2-proxy#895) * detect: Return unknown protocol on detection timeout (linkerd/linkerd2-proxy#896) * Extract protocol detection into the gateway crate (linkerd/linkerd2-proxy#898)
jijeesh
pushed a commit
to jijeesh/linkerd2
that referenced
this pull request
Mar 23, 2021
This release changes HTTP protocol detection to prevent timeout errors in two ways: 1. HTTP detection no longer blocks until a newline is read. We've reverted to relying on a single read to make a determination. 2. Detection timeouts are no longer terminal. When a timeout is encountered, we continue forwarding the connection as an opaque TCP connection. These changes may lead to false-negatives--we may fail to detect some HTTP streams--but it should prevent many avoidable detection errors. This release also makes improvements for multicluster gateways, improving caching so that profile lookups are only performed once per target service. Diagnostic `stack_*` metrics have been moved so that they track underlying services, ignoring fail-fast. This should help us get better insights into services that are in failfast. Finally, the opencensus exporter has been improved to ensure that trace events are flushed if the trace buffer is not filled within a timeout. --- * actions: Update actions to use full SHAs (linkerd/linkerd2-proxy#885) * http: Parameterize normalize_ur::DefaultAuthority (linkerd/linkerd2-proxy#886) * http: Parameterize the HTTP server (linkerd/linkerd2-proxy#887) * opencensus: rewrite span exporter using async/await (linkerd/linkerd2-proxy#789) * Update http::Insert to use `Param` (linkerd/linkerd2-proxy#889) * Update crate dependencies (linkerd/linkerd2-proxy#892) * stack: Make the router fallible (linkerd/linkerd2-proxy#888) * Track stack metrics within failfast (linkerd/linkerd2-proxy#891) * outbound: Avoid building balancers when no concrete name (linkerd/linkerd2-proxy#890) * inbound: Cache HTTP gateways per destination (linkerd/linkerd2-proxy#893) * Reorganize the gateway crate (linkerd/linkerd2-proxy#897) * Bias HTTP detection towards availability (linkerd/linkerd2-proxy#894) * inbound: Use ALPN to determine transport header (linkerd/linkerd2-proxy#895) * detect: Return unknown protocol on detection timeout (linkerd/linkerd2-proxy#896) * Extract protocol detection into the gateway crate (linkerd/linkerd2-proxy#898) Signed-off-by: Jijeesh <[email protected]>
jijeesh
pushed a commit
to jijeesh/linkerd2
that referenced
this pull request
Apr 21, 2021
This release changes HTTP protocol detection to prevent timeout errors in two ways: 1. HTTP detection no longer blocks until a newline is read. We've reverted to relying on a single read to make a determination. 2. Detection timeouts are no longer terminal. When a timeout is encountered, we continue forwarding the connection as an opaque TCP connection. These changes may lead to false-negatives--we may fail to detect some HTTP streams--but it should prevent many avoidable detection errors. This release also makes improvements for multicluster gateways, improving caching so that profile lookups are only performed once per target service. Diagnostic `stack_*` metrics have been moved so that they track underlying services, ignoring fail-fast. This should help us get better insights into services that are in failfast. Finally, the opencensus exporter has been improved to ensure that trace events are flushed if the trace buffer is not filled within a timeout. --- * actions: Update actions to use full SHAs (linkerd/linkerd2-proxy#885) * http: Parameterize normalize_ur::DefaultAuthority (linkerd/linkerd2-proxy#886) * http: Parameterize the HTTP server (linkerd/linkerd2-proxy#887) * opencensus: rewrite span exporter using async/await (linkerd/linkerd2-proxy#789) * Update http::Insert to use `Param` (linkerd/linkerd2-proxy#889) * Update crate dependencies (linkerd/linkerd2-proxy#892) * stack: Make the router fallible (linkerd/linkerd2-proxy#888) * Track stack metrics within failfast (linkerd/linkerd2-proxy#891) * outbound: Avoid building balancers when no concrete name (linkerd/linkerd2-proxy#890) * inbound: Cache HTTP gateways per destination (linkerd/linkerd2-proxy#893) * Reorganize the gateway crate (linkerd/linkerd2-proxy#897) * Bias HTTP detection towards availability (linkerd/linkerd2-proxy#894) * inbound: Use ALPN to determine transport header (linkerd/linkerd2-proxy#895) * detect: Return unknown protocol on detection timeout (linkerd/linkerd2-proxy#896) * Extract protocol detection into the gateway crate (linkerd/linkerd2-proxy#898) Signed-off-by: Jijeesh <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The outbound balancers are always built, even when there is no
resolveable concrete address.
This change modifies the outbound
Concretetarget to require aresolveable address. When no address is present, the balancer stack is
skipped in favor of an endpoint stack.