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

discover: replace linkerd-channel with tokio-util PollSender #969

Merged
merged 3 commits into from
Apr 8, 2021

Conversation

hawkw
Copy link
Contributor

@hawkw hawkw commented Apr 7, 2021

This commit replaces the use of linkerd-channel's pollable MPSC in the
linkerd_discover crate's buffer module with tokio-util's
sync::PollSender type, which also implements a pollable sender.
The PollSender type is likely more efficient than our channel
implementation, since it uses the ReusableBoxFuture type from
tokio-util, which avoids allocating a Box for every message sent
to the channel, and instead reuses a single allocation. It also wraps
the tokio::sync::mpsc Sender, rather than reimplementing the
capacity limit on top of the UnboundedSender type. This reduces
complexity and should hopefully be less error-prone.

Signed-off-by: Eliza Weisman eliza@buoyant.io

This commit replaces the use of `linkerd-channel`'s pollable MPSC in the
`linkerd_discover` crate's `buffer` module with `tokio-util`'s
[`sync::PollSender`][1] type, which also implements a pollable sender.
The `PollSender` type is likely more efficient than our channel
implementation, since it uses the [`ReusableBoxFuture`][2] type from
`tokio-util`, which avoids allocating a `Box` for _every_ message sent
to the channel, and instead reuses a single allocation. It also wraps
the `tokio::sync::mpsc` `Sender`, rather than reimplementing the
capacity limit on top of the `UnboundedSender` type. This reduces
complexity and should hopefully be less error-prone.

[1]: https://docs.rs/tokio-util/0.6.5/tokio_util/sync/struct.PollSender.html
[2]: https://docs.rs/tokio-util/0.6.5/tokio_util/sync/struct.ReusableBoxFuture.html

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
@hawkw hawkw requested review from olix0r and a team April 7, 2021 17:48
hawkw added 2 commits April 7, 2021 10:53
Signed-off-by: Eliza Weisman <eliza@buoyant.io>
Copy link
Contributor

@kleimkuhler kleimkuhler left a comment

Choose a reason for hiding this comment

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

👍

@hawkw hawkw merged commit bb671d5 into main Apr 8, 2021
@hawkw hawkw deleted the eliza/no-custom-chans branch April 8, 2021 18:52
hawkw added a commit that referenced this pull request Apr 8, 2021
Depends on #969.

It turns out that the trait impls for `linkerd_channel::Sender` in
`linkerd-trace-context` were just never being used at all. If we remove
them, we can remove the custom pollable channel implementation in
`linkerd_channel` entirely. Then, this crate will provide only stream
wrappers. Similar wrappers are also provided by `tokio-stream`. If we
replace the use of `linkerd-channel`'s stream wrappers with
`tokio-stream`, we can remove this crate entirely.

This branch completely removes the `linkerd-channel` crate and converts
all uses of its stream wrappers to use `tokio-stream` instead.

Signed-off-by: Eliza Weisman <eliza@buoyant.io>
olix0r added a commit to linkerd/linkerd2 that referenced this pull request Apr 21, 2021
This release primarily improves protocol detection error messages in the
admin server so that logs clearly indicate when the client expected a
different TLS server identity than that of the running proxy.

A number of internal improvements have been made, especially eliminating
some potential runtime panics detected by oss-fuzz. It is not expected
that these panics could be triggered in typical cluster configurations.

---

* discover: replace `linkerd-channel` with `tokio-util` `PollSender` (linkerd/linkerd2-proxy#969)
* replace `linkerd-channel` with `tokio-stream` (linkerd/linkerd2-proxy#970)
* concurrency-limit: use `tokio-util`'s `PollSemaphore` (linkerd/linkerd2-proxy#968)
* http: Do not fail fuzz tests when all IO is not read (linkerd/linkerd2-proxy#973)
* transport: Fix orig-dst compilation on non-Linux targets  (linkerd/linkerd2-proxy#974)
* Update trust-dns to fix possible panic (linkerd/linkerd2-proxy#975)
*  addr: fix `to_http_authority` panic with IPv6 (linkerd/linkerd2-proxy#976)
* outbound: skip logical stacks when no profile is discovered (linkerd/linkerd2-proxy#963)
* split: change traffic splits to require a profile (linkerd/linkerd2-proxy#964)
* inbound: only build profile route stacks when a profile is resolved (linkerd/linkerd2-proxy#966)
* profiles: make receiver param in `route_request` non-optional (linkerd/linkerd2-proxy#967)
* outbound: move target types into stack modules (linkerd/linkerd2-proxy#971)
* outbound: only build logical stacks for profiles with logical addrs (linkerd/linkerd2-proxy#972)
* app: inbound: add fuzzer (linkerd/linkerd2-proxy#977)
* admin: Fail connections when HTTP detection fails (linkerd/linkerd2-proxy#979)
* reduce error boilerplate with `thiserror` (linkerd/linkerd2-proxy#980)
* app: inbound: fuzzer: generalise fuzzers and solve coverage build (linkerd/linkerd2-proxy#978)
* admin: Assume meshed connections are HTTP/2 (linkerd/linkerd2-proxy#982)
* chore: Fix deprecations on nightly (linkerd/linkerd2-proxy#983)
olix0r added a commit to linkerd/linkerd2 that referenced this pull request May 12, 2021
This release simplifies internals so that endpoint-forwarding logic is
completely distinct from handling of load balanced services.

The ingress-mode outbound proxy has been simplified to *require* the
`l5d-dst-override` header and to fail non-HTTP communication. This
ensures that the ingress-mode proxy does not unexpectedly revert to
insecure communication.

Finally, a regression was recently introduced that caused all proxy logs
to be output with ANSI control characters. Logs are now output in
plaintext by default

---

* discover: replace `linkerd-channel` with `tokio-util` `PollSender` (linkerd/linkerd2-proxy#969)
* replace `linkerd-channel` with `tokio-stream` (linkerd/linkerd2-proxy#970)
* concurrency-limit: use `tokio-util`'s `PollSemaphore` (linkerd/linkerd2-proxy#968)
* http: Do not fail fuzz tests when all IO is not read (linkerd/linkerd2-proxy#973)
* transport: Fix orig-dst compilation on non-Linux targets  (linkerd/linkerd2-proxy#974)
* Update trust-dns to fix possible panic (linkerd/linkerd2-proxy#975)
*  addr: fix `to_http_authority` panic with IPv6 (linkerd/linkerd2-proxy#976)
* outbound: skip logical stacks when no profile is discovered (linkerd/linkerd2-proxy#963)
* split: change traffic splits to require a profile (linkerd/linkerd2-proxy#964)
* inbound: only build profile route stacks when a profile is resolved (linkerd/linkerd2-proxy#966)
* profiles: make receiver param in `route_request` non-optional (linkerd/linkerd2-proxy#967)
* outbound: move target types into stack modules (linkerd/linkerd2-proxy#971)
* outbound: only build logical stacks for profiles with logical addrs (linkerd/linkerd2-proxy#972)
* app: inbound: add fuzzer (linkerd/linkerd2-proxy#977)
* admin: Fail connections when HTTP detection fails (linkerd/linkerd2-proxy#979)
* reduce error boilerplate with `thiserror` (linkerd/linkerd2-proxy#980)
* app: inbound: fuzzer: generalise fuzzers and solve coverage build (linkerd/linkerd2-proxy#978)
* admin: Assume meshed connections are HTTP/2 (linkerd/linkerd2-proxy#982)
* chore: Fix deprecations on nightly (linkerd/linkerd2-proxy#983)
* fuzz: Add logging to fuzz targets (linkerd/linkerd2-proxy#985)
* fuzz: don't panic when the proxy closes the conn (linkerd/linkerd2-proxy#986)
* Commit lock files for fuzzers (linkerd/linkerd2-proxy#984)
* fuzz: allow client requests to fail  (linkerd/linkerd2-proxy#989)
* tower: update dependency to 0.4.7 (linkerd/linkerd2-proxy#990)
* outbound: Make the Endpoint::logical_addr field optional (linkerd/linkerd2-proxy#991)
* trace: explicitly disable ANSI terminal colors (linkerd/linkerd2-proxy#994)
* ingress: Require the l5d-dst-override header (linkerd/linkerd2-proxy#992)
* outbound: Do not support TCP-forwarding in ingress-mode (linkerd/linkerd2-proxy#995)
* Decouple tcp forward stack from Endpoint target (linkerd/linkerd2-proxy#996)
* Pickup linkerd-await wrapper in docker build (linkerd/linkerd2-proxy#999)
* docs: Add fuzzing report (linkerd/linkerd2-proxy#1000)
* tests: Use io::Error in mocked connector (linkerd/linkerd2-proxy#1001)
* outbound: Decouple endpoint & logical stacks (linkerd/linkerd2-proxy#1002)
olix0r added a commit to linkerd/linkerd2 that referenced this pull request May 18, 2021
This release simplifies internals so that endpoint-forwarding logic is
completely distinct from handling of load balanced services.

The ingress-mode outbound proxy has been simplified to *require* the
`l5d-dst-override` header and to fail non-HTTP communication. This
ensures that the ingress-mode proxy does not unexpectedly revert to
insecure communication.

Finally, a regression was recently introduced that caused all proxy logs
to be output with ANSI control characters. Logs are now output in
plaintext by default

---

* discover: replace `linkerd-channel` with `tokio-util` `PollSender` (linkerd/linkerd2-proxy#969)
* replace `linkerd-channel` with `tokio-stream` (linkerd/linkerd2-proxy#970)
* concurrency-limit: use `tokio-util`'s `PollSemaphore` (linkerd/linkerd2-proxy#968)
* http: Do not fail fuzz tests when all IO is not read (linkerd/linkerd2-proxy#973)
* transport: Fix orig-dst compilation on non-Linux targets  (linkerd/linkerd2-proxy#974)
* Update trust-dns to fix possible panic (linkerd/linkerd2-proxy#975)
*  addr: fix `to_http_authority` panic with IPv6 (linkerd/linkerd2-proxy#976)
* outbound: skip logical stacks when no profile is discovered (linkerd/linkerd2-proxy#963)
* split: change traffic splits to require a profile (linkerd/linkerd2-proxy#964)
* inbound: only build profile route stacks when a profile is resolved (linkerd/linkerd2-proxy#966)
* profiles: make receiver param in `route_request` non-optional (linkerd/linkerd2-proxy#967)
* outbound: move target types into stack modules (linkerd/linkerd2-proxy#971)
* outbound: only build logical stacks for profiles with logical addrs (linkerd/linkerd2-proxy#972)
* app: inbound: add fuzzer (linkerd/linkerd2-proxy#977)
* admin: Fail connections when HTTP detection fails (linkerd/linkerd2-proxy#979)
* reduce error boilerplate with `thiserror` (linkerd/linkerd2-proxy#980)
* app: inbound: fuzzer: generalise fuzzers and solve coverage build (linkerd/linkerd2-proxy#978)
* admin: Assume meshed connections are HTTP/2 (linkerd/linkerd2-proxy#982)
* chore: Fix deprecations on nightly (linkerd/linkerd2-proxy#983)
* fuzz: Add logging to fuzz targets (linkerd/linkerd2-proxy#985)
* fuzz: don't panic when the proxy closes the conn (linkerd/linkerd2-proxy#986)
* Commit lock files for fuzzers (linkerd/linkerd2-proxy#984)
* fuzz: allow client requests to fail  (linkerd/linkerd2-proxy#989)
* tower: update dependency to 0.4.7 (linkerd/linkerd2-proxy#990)
* outbound: Make the Endpoint::logical_addr field optional (linkerd/linkerd2-proxy#991)
* trace: explicitly disable ANSI terminal colors (linkerd/linkerd2-proxy#994)
* ingress: Require the l5d-dst-override header (linkerd/linkerd2-proxy#992)
* outbound: Do not support TCP-forwarding in ingress-mode (linkerd/linkerd2-proxy#995)
* Decouple tcp forward stack from Endpoint target (linkerd/linkerd2-proxy#996)
* Pickup linkerd-await wrapper in docker build (linkerd/linkerd2-proxy#999)
* docs: Add fuzzing report (linkerd/linkerd2-proxy#1000)
* tests: Use io::Error in mocked connector (linkerd/linkerd2-proxy#1001)
* outbound: Decouple endpoint & logical stacks (linkerd/linkerd2-proxy#1002)
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