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

feat(http/retry): model ReplayBody<B> with Frame<T> #3598

Merged
merged 6 commits into from
Feb 6, 2025

Conversation

cratelyn
Copy link
Collaborator

@cratelyn cratelyn commented Feb 6, 2025

pr #3559 (dd4fbcd) refactored our trailer peeking body middleware to
model its buffering in terms of the Frame<T> type used in
http-body's 1.0 release.

this branch performs a similar change for the other piece of body
middleware that super linkerd's retry facilities: ReplayBody<B>. the
inner body B is now wrapped in the ForwardCompatibleBody<B> adapter,
and we now poll it in terms of frames.

NB: polling the underlying in terms of frames has a subtle knock-on
effect regarding when we observe the trailers, in the liminal period
between this refactor and the subsequent upgrade to hyper 1.0, whilst we
must still implement the existing 0.4 interface for Body that includes
poll_trailers().

see the comment above replay_trailers for more on this, describing why
we now initialize this to true. relatedly, this is why we no longer
delegate down to B::poll_trailers ourselves. it will have already been
called by our adapter.

ReplayBody::is_end_stream() now behaves identically when initially
polling a body compared to subsequent replays. this is fine, as
is_end_stream() is a hint that facilitates optimizations
(hyperium/http-body#143). we do still report the end properly, we just
won't be quite as prescient on the initial playthrough.

in the same manner as the existing frame() method mimics
http_body_util::BodyExt::frame(), this branch introduces
a new ForwardCompatibleBody::poll_frame() method.

this allows us to poll the compatibility layer for a Frame<T>.

see:

@cratelyn cratelyn marked this pull request as ready for review February 6, 2025 05:14
@cratelyn cratelyn requested a review from a team as a code owner February 6, 2025 05:14
linkerd/http/retry/src/replay.rs Outdated Show resolved Hide resolved
Base automatically changed from kate/http-retry.outline-the-replay-buffer to main February 6, 2025 15:50
some tests do not set up a tracing subscriber, because they do not use
the shared `Test::new()` helper function used elsewhere in this test
suite.

to provide a trace of the test's execution in the event of a failure,
initialize a tracing subscriber in some additional unit tests.

Signed-off-by: katelyn martin <kate@buoyant.io>
this commit removes the `cfg(test)` gate on the method exposing
`B::is_end_stream()`, and introduces another method also exposing the
`size_hint()` method.

we will want these in order to implement these methods for
`ReplayBody<B>`.

Signed-off-by: katelyn martin <kate@buoyant.io>
in the same manner as the existing `frame()` method mimics
`http_body_util::BodyExt::frame()`, this commit introduces
a new `ForwardCompatibleBody::poll_frame()` method.

this allows us to poll the compatibility layer for a `Frame<T>`.

Signed-off-by: katelyn martin <kate@buoyant.io>
pr #3559 (dd4fbcd) refactored our trailer peeking body middleware to
model its buffering in terms of the `Frame<T>` type used in
`http-body`'s 1.0 release.

this commit performs a similar change for the other piece of body
middleware that super linkerd's retry facilities: `ReplayBody<B>`. the
inner body `B` is now wrapped in the `ForwardCompatibleBody<B>` adapter,
and we now poll it in terms of frames.

NB: polling the underlying in terms of frames has a subtle knock-on
effect regarding when we observe the trailers, in the liminal period
between this refactor and the subsequent upgrade to hyper 1.0, whilst we
must still implement the existing 0.4 interface for `Body` that includes
`poll_trailers()`.

see the comment above `replay_trailers` for more on this, describing why
we now initialize this to `true`. relatedly, this is why we now longer
delegate down to `B::poll_trailers` ourselves. it will have already been
called by our adapter.

`ReplayBody::is_end_stream()` now behaves identically when initially
polling a body compared to subsequent replays. this is fine, as
`is_end_stream()` is a hint that facilitates optimizations
(hyperium/http-body#143). we do still report the end properly, we just
won't be quite as prescient on the initial playthrough.

see:
- linkerd/linkerd2#8733.
- #3559

Signed-off-by: katelyn martin <kate@buoyant.io>
this commit introduces some trace-level diagnostics tracking how the
replay body has determined whether or not it has reached the end of the
stream.

Signed-off-by: katelyn martin <kate@buoyant.io>
Signed-off-by: katelyn martin <kate@buoyant.io>
@cratelyn cratelyn force-pushed the kate/http-retry.new-frame-polling-same-great-taste branch from 28d2967 to 25bb2b1 Compare February 6, 2025 15:53
@cratelyn cratelyn enabled auto-merge (squash) February 6, 2025 15:54
@cratelyn cratelyn merged commit 4b53081 into main Feb 6, 2025
15 checks passed
@cratelyn cratelyn deleted the kate/http-retry.new-frame-polling-same-great-taste branch February 6, 2025 16:00
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