outbound: don't double-wrap replay bodies (#1036)

hawkw · web-flow · commit 122344732ec1 · 2021-06-07T14:45:11.000-07:00
In PR #1020, the retry code was changed so that bodies are only wrapped in a `ReplayBody` when the request will be retried. The intention was to remove the `replay::layer` pushed to the stack before the retry layer, so that bodies are only wrapped when retries are enabled for the route, and not for all routes in a profile. However, I somehow managed to not remove the `replay::layer` from the stack (possibly due to a messed up rebase). Oops. This meant that all bodies for endpoints with service profiles were wrapped, and when the route has retries enabled, it's wrapped twice. When wrapped twice, this means that we potentially buffer the body data *twice*.
diff --git a/linkerd/app/outbound/src/http/logical.rs b/linkerd/app/outbound/src/http/logical.rs
@@ -122,7 +122,6 @@ impl<E> Outbound<E> {
                     )
                     // Sets an optional retry policy.
                     .push(retry::layer(rt.metrics.http_route_retry.clone()))
-                    .push_on_response(retry::replay::layer())
                     // Sets an optional request timeout.
                     .push(http::MakeTimeoutLayer::default())
                     // Records per-route metrics.
diff --git a/linkerd/http-retry/src/replay.rs b/linkerd/http-retry/src/replay.rs
@@ -1,7 +1,6 @@
 use bytes::{Buf, BufMut, Bytes, BytesMut};
 use http::HeaderMap;
 use http_body::Body;
-use linkerd_stack as svc;
 use parking_lot::Mutex;
 use std::{collections::VecDeque, io::IoSlice, pin::Pin, sync::Arc, task::Context, task::Poll};
 
@@ -71,11 +70,6 @@ struct BodyState<B> {
     is_completed: bool,
 }
 
-pub fn layer<B: Body>() -> svc::MapTargetLayer<fn(http::Request<B>) -> http::Request<ReplayBody<B>>>
-{
-    svc::MapTargetLayer::new(|req: http::Request<B>| req.map(ReplayBody::new))
-}
-
 // === impl ReplayBody ===
 
 impl<B: Body> ReplayBody<B> {

Original file line number	Diff line number	Diff line change
`@@ -122,7 +122,6 @@ impl<E> Outbound<E> {`
`122`	`122`	`)`
`123`	`123`	`// Sets an optional retry policy.`
`124`	`124`	`.push(retry::layer(rt.metrics.http_route_retry.clone()))`
`125`		`- .push_on_response(retry::replay::layer())`
`126`	`125`	`// Sets an optional request timeout.`
`127`	`126`	`.push(http::MakeTimeoutLayer::default())`
`128`	`127`	`// Records per-route metrics.`