Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions api/envoy/api/v2/route/route.proto
Original file line number Diff line number Diff line change
Expand Up @@ -765,6 +765,15 @@ message RouteAction {
// time gaps between gRPC request and response in gRPC streaming mode.
google.protobuf.Duration max_grpc_timeout = 23 [(gogoproto.stdduration) = true];

// If present, Envoy will adjust the timeout provided by the `grpc-timeout` header by subtracting
// the provided duration from the header. This is useful in allowing Envoy to set its global
// timeout to be less than that of the deadline imposed by the calling client, which makes it more
// likely that Envoy will handle the timeout instead of having the call canceled by the client.
// The offset will only be applied if the provided grpc_timeout is greater than the offset. This
// ensures that the offset will only ever decrease the timeout and never set it to 0 (meaning
// infinity).
google.protobuf.Duration grpc_timeout_offset = 28 [(gogoproto.stdduration) = true];

// Allows enabling and disabling upgrades on a per-route basis.
// This overrides any enabled/disabled upgrade filter chain specified in the
// HttpConnectionManager
Expand Down
1 change: 1 addition & 0 deletions docs/root/intro/version_history.rst
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Version history
* redis: added
:ref:`max_buffer_size_before_flush <envoy_api_field_config.filter.network.redis_proxy.v2.RedisProxy.ConnPoolSettings.max_buffer_size_before_flush>` to batch commands together until the encoder buffer hits a certain size, and
:ref:`buffer_flush_timeout <envoy_api_field_config.filter.network.redis_proxy.v2.RedisProxy.ConnPoolSettings.buffer_flush_timeout>` to control how quickly the buffer is flushed if it is not full.
* router: add support for configuring a :ref:`grpc timeout offset <envoy_api_field_route.RouteAction.grpc_timeout_offset>` on incoming requests.
* router: added ability to control retry back-off intervals via :ref:`retry policy <envoy_api_msg_route.RetryPolicy.RetryBackOff>`.
* upstream: added :ref:`upstream_cx_pool_overflow <config_cluster_manager_cluster_stats>` for the connection pool circuit breaker.

Expand Down
7 changes: 7 additions & 0 deletions include/envoy/router/router.h
Original file line number Diff line number Diff line change
Expand Up @@ -621,6 +621,13 @@ class RouteEntry : public ResponseEntry {
*/
virtual absl::optional<std::chrono::milliseconds> maxGrpcTimeout() const PURE;

/**
* @return absl::optional<std::chrono::milliseconds> the timeout offset to apply to the timeout
* provided by the 'grpc-timeout' header of a gRPC request. This value will be positive and should
* be subtracted from the value provided by the header.
*/
virtual absl::optional<std::chrono::milliseconds> grpcTimeoutOffset() const PURE;

/**
* Determine whether a specific request path belongs to a virtual cluster for use in stats, etc.
* @param headers supplies the request headers.
Expand Down
3 changes: 3 additions & 0 deletions source/common/http/async_client_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,9 @@ class AsyncStreamImpl : public AsyncClient::Stream,
absl::optional<std::chrono::milliseconds> maxGrpcTimeout() const override {
return absl::nullopt;
}
absl::optional<std::chrono::milliseconds> grpcTimeoutOffset() const override {
return absl::nullopt;
}
const Router::VirtualCluster* virtualCluster(const Http::HeaderMap&) const override {
return nullptr;
}
Expand Down
1 change: 1 addition & 0 deletions source/common/router/config_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,7 @@ RouteEntryImplBase::RouteEntryImplBase(const VirtualHostImpl& vhost,
timeout_(PROTOBUF_GET_MS_OR_DEFAULT(route.route(), timeout, DEFAULT_ROUTE_TIMEOUT_MS)),
idle_timeout_(PROTOBUF_GET_OPTIONAL_MS(route.route(), idle_timeout)),
max_grpc_timeout_(PROTOBUF_GET_OPTIONAL_MS(route.route(), max_grpc_timeout)),
grpc_timeout_offset_(PROTOBUF_GET_OPTIONAL_MS(route.route(), grpc_timeout_offset)),
loader_(factory_context.runtime()), runtime_(loadRuntimeData(route.match())),
scheme_redirect_(route.redirect().scheme_redirect()),
host_redirect_(route.redirect().host_redirect()),
Expand Down
7 changes: 7 additions & 0 deletions source/common/router/config_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,9 @@ class RouteEntryImplBase : public RouteEntry,
absl::optional<std::chrono::milliseconds> maxGrpcTimeout() const override {
return max_grpc_timeout_;
}
absl::optional<std::chrono::milliseconds> grpcTimeoutOffset() const override {
return grpc_timeout_offset_;
}
const VirtualHost& virtualHost() const override { return vhost_; }
bool autoHostRewrite() const override { return auto_host_rewrite_; }
const std::multimap<std::string, std::string>& opaqueConfig() const override {
Expand Down Expand Up @@ -481,6 +484,9 @@ class RouteEntryImplBase : public RouteEntry,
absl::optional<std::chrono::milliseconds> maxGrpcTimeout() const override {
return parent_->maxGrpcTimeout();
}
absl::optional<std::chrono::milliseconds> grpcTimeoutOffset() const override {
return parent_->maxGrpcTimeout();
}
const MetadataMatchCriteria* metadataMatchCriteria() const override {
return parent_->metadataMatchCriteria();
}
Expand Down Expand Up @@ -604,6 +610,7 @@ class RouteEntryImplBase : public RouteEntry,
const std::chrono::milliseconds timeout_;
const absl::optional<std::chrono::milliseconds> idle_timeout_;
const absl::optional<std::chrono::milliseconds> max_grpc_timeout_;
const absl::optional<std::chrono::milliseconds> grpc_timeout_offset_;
Runtime::Loader& loader_;
const absl::optional<RuntimeData> runtime_;
const std::string scheme_redirect_;
Expand Down
9 changes: 9 additions & 0 deletions source/common/router/router.cc
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,15 @@ FilterUtility::finalTimeout(const RouteEntry& route, Http::HeaderMap& request_he
if (grpc_request && route.maxGrpcTimeout()) {
const std::chrono::milliseconds max_grpc_timeout = route.maxGrpcTimeout().value();
std::chrono::milliseconds grpc_timeout = Grpc::Common::getGrpcTimeout(request_headers);
if (route.grpcTimeoutOffset()) {
// We only apply the offset if it won't result in grpc_timeout hitting 0 or below, as
// setting it to 0 means infinity and a negative timeout makes no sense.
const auto offset = *route.grpcTimeoutOffset();
if (offset < grpc_timeout) {
grpc_timeout -= offset;
}
}

// Cap gRPC timeout to the configured maximum considering that 0 means infinity.
if (max_grpc_timeout != std::chrono::milliseconds(0) &&
(grpc_timeout == std::chrono::milliseconds(0) || grpc_timeout > max_grpc_timeout)) {
Expand Down
30 changes: 30 additions & 0 deletions test/common/router/config_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1878,6 +1878,36 @@ TEST_F(RouteMatcherTest, ContentType) {
}
}

TEST_F(RouteMatcherTest, GrpcTimeoutOffset) {
const std::string yaml = R"EOF(
virtual_hosts:
- name: local_service
domains:
- "*"
routes:
- match:
prefix: "/foo"
route:
cluster: local_service_grpc
- match:
prefix: "/"
route:
grpc_timeout_offset: 0.01s
cluster: local_service_grpc
)EOF";

TestConfigImpl config(parseRouteConfigurationFromV2Yaml(yaml), factory_context_, true);

{
EXPECT_EQ(
absl::make_optional(std::chrono::milliseconds(10)),
config.route(genHeaders("www.lyft.com", "/", "GET"), 0)->routeEntry()->grpcTimeoutOffset());
}
EXPECT_EQ(absl::nullopt, config.route(genHeaders("www.lyft.com", "/foo", "GET"), 0)
->routeEntry()
->grpcTimeoutOffset());
}

TEST_F(RouteMatcherTest, FractionalRuntime) {
const std::string yaml = R"EOF(
virtual_hosts:
Expand Down
22 changes: 22 additions & 0 deletions test/common/router/router_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2571,6 +2571,28 @@ TEST(RouterFilterUtilityTest, FinalTimeout) {
EXPECT_EQ(std::chrono::milliseconds(0), timeout.per_try_timeout_);
EXPECT_EQ("999m", headers.get_("grpc-timeout"));
}
{
NiceMock<MockRouteEntry> route;
EXPECT_CALL(route, maxGrpcTimeout())
.WillRepeatedly(Return(absl::optional<std::chrono::milliseconds>(999)));
EXPECT_CALL(route, grpcTimeoutOffset())
.WillRepeatedly(Return(absl::optional<std::chrono::milliseconds>(10)));
Http::TestHeaderMapImpl headers{{"content-type", "application/grpc"}, {"grpc-timeout", "100m"}};
FilterUtility::TimeoutData timeout = FilterUtility::finalTimeout(route, headers, true, true);
EXPECT_EQ(std::chrono::milliseconds(90), timeout.global_timeout_);
EXPECT_EQ(std::chrono::milliseconds(0), timeout.per_try_timeout_);
}
{
NiceMock<MockRouteEntry> route;
EXPECT_CALL(route, maxGrpcTimeout())
.WillRepeatedly(Return(absl::optional<std::chrono::milliseconds>(999)));
EXPECT_CALL(route, grpcTimeoutOffset())
.WillRepeatedly(Return(absl::optional<std::chrono::milliseconds>(10)));
Http::TestHeaderMapImpl headers{{"content-type", "application/grpc"}, {"grpc-timeout", "1m"}};
FilterUtility::TimeoutData timeout = FilterUtility::finalTimeout(route, headers, true, true);
EXPECT_EQ(std::chrono::milliseconds(1), timeout.global_timeout_);
EXPECT_EQ(std::chrono::milliseconds(0), timeout.per_try_timeout_);
}
{
NiceMock<MockRouteEntry> route;
EXPECT_CALL(route, maxGrpcTimeout())
Expand Down
1 change: 1 addition & 0 deletions test/mocks/router/mocks.h
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,7 @@ class MockRouteEntry : public RouteEntry {
MOCK_CONST_METHOD0(timeout, std::chrono::milliseconds());
MOCK_CONST_METHOD0(idleTimeout, absl::optional<std::chrono::milliseconds>());
MOCK_CONST_METHOD0(maxGrpcTimeout, absl::optional<std::chrono::milliseconds>());
MOCK_CONST_METHOD0(grpcTimeoutOffset, absl::optional<std::chrono::milliseconds>());
MOCK_CONST_METHOD1(virtualCluster, const VirtualCluster*(const Http::HeaderMap& headers));
MOCK_CONST_METHOD0(virtualHostName, const std::string&());
MOCK_CONST_METHOD0(virtualHost, const VirtualHost&());
Expand Down