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

Flip grpc client cache flag #19253

Merged
merged 7 commits into from
Jan 18, 2022
Merged
Show file tree
Hide file tree
Changes from 4 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
1 change: 1 addition & 0 deletions docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ Minor Behavior Changes
* bandwidth_limit: added :ref:`response trailers <envoy_v3_api_field_extensions.filters.http.bandwidth_limit.v3.BandwidthLimit.enable_response_trailers>` when request or response delay are enforced.
* bandwidth_limit: added :ref:`bandwidth limit stats <config_http_filters_bandwidth_limit>` *request_enforced* and *response_enforced*.
* dns: now respecting the returned DNS TTL for resolved hosts, rather than always relying on the hard-coded :ref:`dns_refresh_rate. <envoy_v3_api_field_config.cluster.v3.Cluster.dns_refresh_rate>` This behavior can be temporarily reverted by setting the runtime guard ``envoy.reloadable_features.use_dns_ttl`` to false.
* grpc: flip runtime guard ``envoy.reloadable_features.enable_grpc_async_client_cache`` to be default enabled.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you make sure comment what the functional change is?

Also this got lost as well due to the waiting tag, sorry. I've removed it so hopefully it will make progress again, but you'll want to main merge for current.rst

* http: envoy will now proxy 102 and 103 headers from upstream, though as with 100s only the first 1xx response headers will be sent. This behavioral change by can temporarily reverted by setting runtime guard ``envoy.reloadable_features.proxy_102_103`` to false.
* http: usage of the experimental matching API is no longer guarded behind a feature flag, as the corresponding protobuf fields have been marked as WIP.
* http: when envoy run out of ``max_requests_per_connection``, it will send an HTTP/2 "shutdown nofitication" (GOAWAY frame with max stream ID) and go to a default grace period of 5000 milliseconds (5 seconds) if ``drain_timeout`` is not specified. During this grace period, envoy will continue to accept new streams. After the grace period, a final GOAWAY is sent and envoy will start refusing new streams. However before bugfix, during the grace period, every time a new stream is received, old envoy will always send a "shutdown notification" and restart drain again which actually causes the grace period to be extended and is no longer equal to ``drain_timeout``.
Expand Down
5 changes: 1 addition & 4 deletions source/common/runtime/runtime_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ constexpr const char* runtime_features[] = {
"envoy.reloadable_features.conn_pool_delete_when_idle",
"envoy.reloadable_features.correct_scheme_and_xfp",
"envoy.reloadable_features.disable_tls_inspector_injection",
"envoy.reloadable_features.enable_grpc_async_client_cache",
"envoy.reloadable_features.fix_added_trailers",
"envoy.reloadable_features.grpc_bridge_stats_disabled",
"envoy.reloadable_features.handle_stream_reset_during_hcm_encoding",
Expand Down Expand Up @@ -112,10 +113,6 @@ constexpr const char* disabled_runtime_features[] = {
"envoy.reloadable_features.allow_multiple_dns_addresses",
// Sentinel and test flag.
"envoy.reloadable_features.test_feature_false",
// When the runtime is flipped to true, use shared cache in getOrCreateRawAsyncClient method if
// CacheOption is CacheWhenRuntimeEnabled.
// Caller that use AlwaysCache option will always cache, unaffected by this runtime.
"envoy.reloadable_features.enable_grpc_async_client_cache",
// TODO(dmitri-d) reset to true to enable unified mux by default
"envoy.reloadable_features.unified_mux",
// TODO(birenroy): flip to true in a future PR to enable by default
Expand Down
6 changes: 3 additions & 3 deletions test/common/grpc/async_client_manager_impl_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,9 @@ TEST_F(AsyncClientManagerImplTest, EnvoyGrpcOk) {
}

TEST_F(AsyncClientManagerImplTest, DisableRawAsyncClientCache) {
TestScopedRuntime scoped_runtime;
Runtime::LoaderSingleton::getExisting()->mergeValues(
{{"envoy.reloadable_features.enable_grpc_async_client_cache", "false"}});
envoy::config::core::v3::GrpcService grpc_service;
grpc_service.mutable_envoy_grpc()->set_cluster_name("foo");

Expand All @@ -146,9 +149,6 @@ TEST_F(AsyncClientManagerImplTest, DisableRawAsyncClientCache) {
}

TEST_F(AsyncClientManagerImplTest, EnableRawAsyncClientCache) {
TestScopedRuntime scoped_runtime;
Runtime::LoaderSingleton::getExisting()->mergeValues(
{{"envoy.reloadable_features.enable_grpc_async_client_cache", "true"}});
envoy::config::core::v3::GrpcService grpc_service;
grpc_service.mutable_envoy_grpc()->set_cluster_name("foo");

Expand Down