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

retry: allow retrying requests without Content-Length headers #1341

Merged
merged 9 commits into from
Nov 1, 2021

Commits on Oct 29, 2021

  1. retry: allow retrying requests without Content-Length headers

    Currently, the proxy will retry requests with bodies if and only if they
    have a `Content-Length` header with a value <= 64 KB. If the request has
    a body but no `Content-Length` header, we currently assume that its body
    will exceed the maximum buffered size, and skip trying to retry it.
    However, this means gRPC requests will never be retried, because it
    turns out gRPC requests don't include a `Content-Length` header (see
    linkerd/linkerd2#7141). Whoops!
    
    This PR fixes this by changing the retry logic so that `Content-Length`
    is treated as a _hint_, rather than a requirement. Now, we will
    preemptively skip retrying requests with bodies when the there is a
    `Content-Length` header with a value that exceeds the limit, but if no
    `Content-Length` header is present, we will still attempt to buffer the
    body for a potential retry.
    
    We are still protected against unbounded buffering because the buffering
    body will stop buffering and discard any previously buffered data if the
    buffered length ever exceeds the maximum. Requiring a `Content-Length`
    simply allowed us to skip _trying_.
    
    This should probably fix linkerd/linkerd2#7141, although it might be
    worth testing with a non-Rust gRPC implementation to make sure it works?
    hawkw committed Oct 29, 2021
    Configuration menu
    Copy the full SHA
    5f790aa View commit details
    Browse the repository at this point in the history
  2. small refactor

    Signed-off-by: Eliza Weisman <eliza@buoyant.io>
    hawkw committed Oct 29, 2021
    Configuration menu
    Copy the full SHA
    e16c4ed View commit details
    Browse the repository at this point in the history
  3. update tests

    Signed-off-by: Eliza Weisman <eliza@buoyant.io>
    hawkw committed Oct 29, 2021
    Configuration menu
    Copy the full SHA
    3860af4 View commit details
    Browse the repository at this point in the history
  4. fix retry tests with streaming bodies never sending chunks

    `send_data` has to be awaited to actually...send the data, lol.
    
    Signed-off-by: Eliza Weisman <eliza@buoyant.io>
    hawkw committed Oct 29, 2021
    Configuration menu
    Copy the full SHA
    d2463eb View commit details
    Browse the repository at this point in the history
  5. actually expect body data to be read

    using `let _` is how we ended up accidentally not awaiting the
    `send_data` futures...we should probably also panic if the proxy dropped
    the body too early, anyway.
    
    Signed-off-by: Eliza Weisman <eliza@buoyant.io>
    hawkw committed Oct 29, 2021
    Configuration menu
    Copy the full SHA
    9a5fb91 View commit details
    Browse the repository at this point in the history
  6. fix test expecting a 533

    Signed-off-by: Eliza Weisman <eliza@buoyant.io>
    hawkw committed Oct 29, 2021
    Configuration menu
    Copy the full SHA
    39deb30 View commit details
    Browse the repository at this point in the history
  7. Configuration menu
    Copy the full SHA
    b6e916e View commit details
    Browse the repository at this point in the history
  8. do not retry when the replay buffer was capped

    this changes the retry policy to check if the replay buffer was capped
    on a previous iteration of the request. if it was, we will not retry the
    request, allowing the original response to be propagated.
    
    Signed-off-by: Eliza Weisman <eliza@buoyant.io>
    hawkw committed Oct 29, 2021
    Configuration menu
    Copy the full SHA
    2c8cd32 View commit details
    Browse the repository at this point in the history

Commits on Nov 1, 2021

  1. Use Body::size_hint to determine whether a request is retryable (#1345)

    Currently, we look at each request's `content-length` header to
    determine whether a request is too large to buffer for retries. This is
    unnecessary, as hyper will already read the `content-length` and use it
    populate the `Body::size_hint` value.
    
    This change modifies the `ReplayBody` constructor (now
    `ReplayBody::try_new`) to check the body's size hint and fails to
    construct a `ReplayBody` if the lower bounds of the body size exceeds
    the maximum buffer size.
    
    In doing so, we update all `Body` implementations to ensure that
    `size_hint` is properly implemenented. We also update the
    `ReplayBody::size_hint` implementation to be more robust, returning the
    original size hint if a clone of `ReplayBody` does not currently hold
    the inner body.
    olix0r authored Nov 1, 2021
    Configuration menu
    Copy the full SHA
    dae3917 View commit details
    Browse the repository at this point in the history