Skip to content

Commit

Permalink
refactor(http/retry): defer construction of PeekTrailersBody<B>
Browse files Browse the repository at this point in the history
this commit refactors the polling logic in
`PeekTrailersBody<B>::read_response`.

this commit makes some subtle changes with the migration to hyper 1.0 in
mind, to make this function more easily portable to the new signature of
`http_body::Body`.

see linkerd/linkerd2#8733 for more
information.

this commit defers the `Self` construction of the `PeekTrailersBody`
body. this means that the control flow does not need to reach through to
e.g. `body.inner` to poll the inner body being peeked. additionally, it
provides us with `let` bindings for the first data frame yielded, and
the trailers frame yielded thereafter.

this is largely cosmetic, but will make it easier to deal with the
additional matching we'll need to do when there is a single polling
function that yields us `Frame<D>` objects.

Signed-off-by: katelyn martin <kate@buoyant.io>
  • Loading branch information
cratelyn committed Jan 21, 2025
1 parent e462406 commit 3a7821a
Showing 1 changed file with 31 additions and 25 deletions.
56 changes: 31 additions & 25 deletions linkerd/http/retry/src/peek_trailers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,37 +96,43 @@ impl<B: Body> PeekTrailersBody<B> {
B::Data: Send + Unpin,
B::Error: Send,
{
let (parts, body) = rsp.into_parts();
let mut body = Self {
inner: body,
first_data: None,
trailers: None,
};
let (parts, mut body) = rsp.into_parts();

// First, poll the body for its first frame.
tracing::debug!("Buffering first data frame");
if let Some(data) = body.inner.data().await {
// The body has data; stop waiting for trailers.
body.first_data = Some(data);

// Peek to see if there's immediately a trailers frame, and grab
// it if so. Otherwise, bail.
// XXX(eliza): the documentation for the `http::Body` trait says
// that `poll_trailers` should only be called after `poll_data`
// returns `None`...but, in practice, I'm fairly sure that this just
// means that it *will not return `Ready`* until there are no data
// frames left, which is fine for us here, because we `now_or_never`
// it.
body.trailers = body.inner.trailers().now_or_never();
let first_data = body.data().await;

// Now, inspect the frame yielded. If the body yielded a data frame, we will only peek
// the trailers if they are immediately available. If the body did not yield a data frame,
// we will poll a future to yield the trailers.
let trailers = if first_data.is_some() {
// The body has data; stop waiting for trailers. Peek to see if there's immediately a
// trailers frame, and grab it if so. Otherwise, bail.
//
// XXX(eliza): the documentation for the `http::Body` trait says that `poll_trailers`
// should only be called after `poll_data` returns `None`...but, in practice, I'm
// fairly sure that this just means that it *will not return `Ready`* until there are
// no data frames left, which is fine for us here, because we `now_or_never` it.
body.trailers().now_or_never()
} else {
// Okay, `poll_data` has returned `None`, so there are no data
// frames left. Let's see if there's trailers...
body.trailers = Some(body.inner.trailers().await);
}
if body.trailers.is_some() {
// Okay, `poll_data` has returned `None`, so there are no data frames left. Let's see
// if there's trailers...
let trls = body.trailers().await;
Some(trls)
};

if trailers.is_some() {
tracing::debug!("Buffered trailers frame");
}

http::Response::from_parts(parts, body)
http::Response::from_parts(
parts,
Self {
inner: body,
first_data,
trailers,
},
)
}

/// Returns a response with an inert [`PeekTrailersBody<B>`].
Expand Down

0 comments on commit 3a7821a

Please sign in to comment.