Skip to content

Commit dd4fbcd

Browse files
authored
feat(http/retry): model PeekTrailersBody<B> with Frame<T> (#3559)
this branch contains a sequence of commits that refactor `PeekTrailersBody<B>`. this branch is specifically focused on making this body middleware forward-compatible with the 1.0 interface(s) of `http_body::Body` and `http_body_util::BodyExt`. it does this in two main steps: (1) temporarily vendoring `http_body::Frame<T>` and providing a compatibility shim that provides a `frame()` method for a body, and (2) modeling `PeekTrailersBody<B>` and its peeking logic in terms of this `Frame<'a, T>` future. --- * feat(http/retry): add `Frame<T>` compatibility facilities this commit introduces a `compat` submodule to `linkerd-http-retry`. this helps us frontrun the task of replacing all of the finicky control flow in `PeekTrailersBody<B>` using the antiquated `data()` and `trailers()` future combinators. instead, we can perform our peeking in terms of an approximation of `http_body_util::BodyExt::frame()`. to accomplish this, this commit vendors a copy of the `Frame<T>` type. we can use this to preemptively model our peek body in terms of this type, and move to the "real" version of it when we're upgrading in pr #3504. additionally, this commit includes a type called `ForwardCompatibleBody<B>`, and a variant of the `Frame<'a, T>` combinator. these are a bit boilerplate-y, admittedly, but the pleasant part of this is that we have, in effect, migrated the trickiest body middleware in advance of #3504. once we upgrade to http-body 1.0, all of these types can be removed. https://docs.rs/http-body-util/latest/http_body_util/trait.BodyExt.html#method.frame https://docs.rs/http-body-util/0.1.2/src/http_body_util/combinators/frame.rs.html#10 Signed-off-by: katelyn martin <[email protected]> * refactor(http/retry): `PeekTrailersBody<B>` uses `BodyExt::frame()` this commit reworks `PeekTrailersBody<B>`. the most important goal here is replacing the control flow of `read_body()`, which polls a body using `BodyExt` future combinators `data()` and `frame()` for up to two frames, with varying levels of persistence depending on outcomes. to quote #3556: > the intent of this type is to only yield the asynchronous task > responsible for reading the body once. depending on what the inner > body yields, and when, this can result in combinations of: no data > frames and no trailers, no data frames with trailers, one data frame > and no trailers, one data frame with trailers, or two data frames. > moreover, depending on which of these are yielded, the body will call > .await some scenarios, and only poll functions once in others. > > migrating this to the Frame<T> and poll_frame() style of the 1.0 Body > interface, away from the 0.4 interface that provides distinct > poll_data() and poll_trailers() methods, is fundamentally tricky. this means that `PeekTrailersBody<B>` is notably difficult to port across the http-body 0.4/1.0 upgrade boundary. this body middleware must navigate a number of edge conditions, and once it _has_ obtained a `Frame<T>`, make use of conversion methods to ascertain whether it is a data or trailers frame, due to the fact that its internal enum representation is not exposed publicly. one it has done all of that, it must do the same thing once more to examine the second frame. this commit uses the compatibility facilities and backported `Frame<T>` introduced in the previous commit, and rewrites this control flow using a form of the `BodyExt::frame()` combinator. this means that this middleware is forward-compatible with http-body 1.0, which will dramatically simplify the remaining migration work to be done in #3504. see linkerd/linkerd2#8733 for more information and other links related to this ongoing migration work. Signed-off-by: katelyn martin <[email protected]> * refactor(http/retry): mock body enforces `poll_trailers()` contract this commit addresses a `TODO` note, and tightens the enforcement of a rule defined by the v0.4 signature of the `Body` trait. this commit changes the mock body type, used in tests, so that it will panic if the caller improperly polls for a trailers frame before the final data frame has been yielded. previously, a comment indicated that we were "fairly sure" this was okay. while that may have been acceptable in practice, the changes in the previous commit mean that we now properly respect these rules. thus, a panic can be placed here, to enforce that "[is] only be called once `poll_data()` returns `None`", per the documentation. Signed-off-by: katelyn martin <[email protected]> * refactor(http/retry): rename `PeekTrailersBody::Buffered` <#3559 (comment)> this is a nicer name than `Unknown` for this case. not to mention, we'll want that name shortly to address the possibility of unknown frame variants. Signed-off-by: katelyn martin <[email protected]> * refactor(http/retry): encapsulate `Inner<B>` enum variants this commit makes the inner enum variants private. #3559 (comment) Signed-off-by: katelyn martin <[email protected]> * refactor(http/retry): gracefully ignore unknown frames #3559 (comment) Signed-off-by: katelyn martin <[email protected]> --------- Signed-off-by: katelyn martin <[email protected]>
1 parent 5d4314c commit dd4fbcd

File tree

4 files changed

+475
-111
lines changed

4 files changed

+475
-111
lines changed

linkerd/http/retry/src/compat.rs

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
//! Compatibility utilities for upgrading to http-body 1.0.
2+
3+
use http_body::Body;
4+
5+
pub(crate) use self::frame::Frame;
6+
7+
mod frame;
8+
9+
#[derive(Debug)]
10+
pub(crate) struct ForwardCompatibleBody<B> {
11+
inner: B,
12+
data_finished: bool,
13+
trailers_finished: bool,
14+
}
15+
16+
// === impl ForwardCompatibleBody ===
17+
18+
impl<B: Body> ForwardCompatibleBody<B> {
19+
pub(crate) fn new(body: B) -> Self {
20+
if body.is_end_stream() {
21+
Self {
22+
inner: body,
23+
data_finished: true,
24+
trailers_finished: true,
25+
}
26+
} else {
27+
Self {
28+
inner: body,
29+
data_finished: false,
30+
trailers_finished: false,
31+
}
32+
}
33+
}
34+
35+
pub(crate) fn into_inner(self) -> B {
36+
self.inner
37+
}
38+
39+
/// Returns a future that resolves to the next frame.
40+
pub(crate) fn frame(&mut self) -> combinators::Frame<'_, B> {
41+
combinators::Frame(self)
42+
}
43+
}
44+
45+
/// Future that resolves to the next frame from a `Body`.
46+
///
47+
/// NB: This is a vendored stand-in for [`Frame<'a, T>`][frame], and and can be replaced once
48+
/// we upgrade from http-body 0.4 to 1.0. This file was vendored, and subsequently adapted to this
49+
/// project, at commit 86fdf00.
50+
///
51+
/// See linkerd/linkerd2#8733 for more information.
52+
///
53+
/// [frame]: https://docs.rs/http-body-util/0.1.2/http_body_util/combinators/struct.Frame.html
54+
mod combinators {
55+
use core::future::Future;
56+
use core::pin::Pin;
57+
use core::task;
58+
use http_body::Body;
59+
use std::ops::Not;
60+
use std::task::ready;
61+
62+
use super::ForwardCompatibleBody;
63+
64+
#[must_use = "futures don't do anything unless polled"]
65+
#[derive(Debug)]
66+
/// Future that resolves to the next frame from a [`Body`].
67+
pub struct Frame<'a, T>(pub(super) &'a mut super::ForwardCompatibleBody<T>);
68+
69+
impl<T: Body + Unpin> Future for Frame<'_, T> {
70+
type Output = Option<Result<super::Frame<T::Data>, T::Error>>;
71+
72+
fn poll(self: Pin<&mut Self>, ctx: &mut task::Context<'_>) -> task::Poll<Self::Output> {
73+
let Self(ForwardCompatibleBody {
74+
inner,
75+
data_finished,
76+
trailers_finished,
77+
}) = self.get_mut();
78+
let mut pinned = Pin::new(inner);
79+
80+
// We have already yielded the trailers, the body is done.
81+
if *trailers_finished {
82+
return task::Poll::Ready(None);
83+
}
84+
85+
// We are still yielding data frames.
86+
if data_finished.not() {
87+
match ready!(pinned.as_mut().poll_data(ctx)) {
88+
Some(Ok(data)) => {
89+
// We yielded a frame.
90+
return task::Poll::Ready(Some(Ok(super::Frame::data(data))));
91+
}
92+
Some(Err(error)) => {
93+
// If we encountered an error, we are finished.
94+
*data_finished = true;
95+
*trailers_finished = true;
96+
return task::Poll::Ready(Some(Err(error)));
97+
}
98+
None => {
99+
// We are done yielding data frames. Mark the corresponding flag, and fall
100+
// through to poll the trailers...
101+
*data_finished = true;
102+
}
103+
};
104+
}
105+
106+
// We have yielded all of the data frames but have not yielded the trailers.
107+
let trailers = ready!(pinned.poll_trailers(ctx));
108+
*trailers_finished = true;
109+
let trailers = trailers
110+
.transpose()
111+
.map(|res| res.map(super::Frame::trailers));
112+
task::Poll::Ready(trailers)
113+
}
114+
}
115+
}
Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
#![allow(unused, reason = "this code is vendored from `http-body v1.0.1")]
2+
3+
//! A frame of any kind related to an HTTP stream (body).
4+
//!
5+
//! NB: This is a vendored stand-in for [`Frame<T>`][frame], and and can be replaced once
6+
//! we upgrade from http-body 0.4 to 1.0. This file was vendored at commit 86fdf00.
7+
//!
8+
//! See linkerd/linkerd2#8733 for more information.
9+
//!
10+
//! [frame]: https://docs.rs/http-body/1.0.1/http_body/struct.Frame.html>
11+
12+
use http::HeaderMap;
13+
14+
/// A frame of any kind related to an HTTP stream (body).
15+
#[derive(Debug)]
16+
pub struct Frame<T> {
17+
kind: Kind<T>,
18+
}
19+
20+
#[derive(Debug)]
21+
enum Kind<T> {
22+
// The first two variants are "inlined" since they are undoubtedly
23+
// the most common. This saves us from having to allocate a
24+
// boxed trait object for them.
25+
Data(T),
26+
Trailers(HeaderMap),
27+
//Unknown(Box<dyn Frameish>),
28+
}
29+
30+
impl<T> Frame<T> {
31+
/// Create a DATA frame with the provided `Buf`.
32+
pub fn data(buf: T) -> Self {
33+
Self {
34+
kind: Kind::Data(buf),
35+
}
36+
}
37+
38+
/// Create a trailers frame.
39+
pub fn trailers(map: HeaderMap) -> Self {
40+
Self {
41+
kind: Kind::Trailers(map),
42+
}
43+
}
44+
45+
/// Maps this frame's data to a different type.
46+
pub fn map_data<F, D>(self, f: F) -> Frame<D>
47+
where
48+
F: FnOnce(T) -> D,
49+
{
50+
match self.kind {
51+
Kind::Data(data) => Frame {
52+
kind: Kind::Data(f(data)),
53+
},
54+
Kind::Trailers(trailers) => Frame {
55+
kind: Kind::Trailers(trailers),
56+
},
57+
}
58+
}
59+
60+
/// Returns whether this is a DATA frame.
61+
pub fn is_data(&self) -> bool {
62+
matches!(self.kind, Kind::Data(..))
63+
}
64+
65+
/// Consumes self into the buf of the DATA frame.
66+
///
67+
/// Returns an [`Err`] containing the original [`Frame`] when frame is not a DATA frame.
68+
/// `Frame::is_data` can also be used to determine if the frame is a DATA frame.
69+
pub fn into_data(self) -> Result<T, Self> {
70+
match self.kind {
71+
Kind::Data(data) => Ok(data),
72+
_ => Err(self),
73+
}
74+
}
75+
76+
/// If this is a DATA frame, returns a reference to it.
77+
///
78+
/// Returns `None` if not a DATA frame.
79+
pub fn data_ref(&self) -> Option<&T> {
80+
match self.kind {
81+
Kind::Data(ref data) => Some(data),
82+
_ => None,
83+
}
84+
}
85+
86+
/// If this is a DATA frame, returns a mutable reference to it.
87+
///
88+
/// Returns `None` if not a DATA frame.
89+
pub fn data_mut(&mut self) -> Option<&mut T> {
90+
match self.kind {
91+
Kind::Data(ref mut data) => Some(data),
92+
_ => None,
93+
}
94+
}
95+
96+
/// Returns whether this is a trailers frame.
97+
pub fn is_trailers(&self) -> bool {
98+
matches!(self.kind, Kind::Trailers(..))
99+
}
100+
101+
/// Consumes self into the buf of the trailers frame.
102+
///
103+
/// Returns an [`Err`] containing the original [`Frame`] when frame is not a trailers frame.
104+
/// `Frame::is_trailers` can also be used to determine if the frame is a trailers frame.
105+
pub fn into_trailers(self) -> Result<HeaderMap, Self> {
106+
match self.kind {
107+
Kind::Trailers(trailers) => Ok(trailers),
108+
_ => Err(self),
109+
}
110+
}
111+
112+
/// If this is a trailers frame, returns a reference to it.
113+
///
114+
/// Returns `None` if not a trailers frame.
115+
pub fn trailers_ref(&self) -> Option<&HeaderMap> {
116+
match self.kind {
117+
Kind::Trailers(ref trailers) => Some(trailers),
118+
_ => None,
119+
}
120+
}
121+
122+
/// If this is a trailers frame, returns a mutable reference to it.
123+
///
124+
/// Returns `None` if not a trailers frame.
125+
pub fn trailers_mut(&mut self) -> Option<&mut HeaderMap> {
126+
match self.kind {
127+
Kind::Trailers(ref mut trailers) => Some(trailers),
128+
_ => None,
129+
}
130+
}
131+
}

linkerd/http/retry/src/lib.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
pub mod peek_trailers;
55
pub mod replay;
66

7+
mod compat;
8+
79
pub use self::{peek_trailers::PeekTrailersBody, replay::ReplayBody};
810
pub use tower::retry::budget::Budget;
911

0 commit comments

Comments
 (0)