From 4444c0a3d5a5b7a37f874c656069efbd8b323dbb Mon Sep 17 00:00:00 2001 From: Juanli Shen Date: Fri, 1 Nov 2019 23:22:07 -0700 Subject: [PATCH 1/3] Disable client_idle_filter --- src/core/ext/filters/client_idle/client_idle_filter.cc | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/core/ext/filters/client_idle/client_idle_filter.cc b/src/core/ext/filters/client_idle/client_idle_filter.cc index b8a5721f84c..2ea3eadeec3 100644 --- a/src/core/ext/filters/client_idle/client_idle_filter.cc +++ b/src/core/ext/filters/client_idle/client_idle_filter.cc @@ -27,10 +27,9 @@ #include "src/core/lib/surface/channel_init.h" #include "src/core/lib/transport/http2_errors.h" -// The idle filter is enabled in client channel by default. -// Set GRPC_ARG_CLIENT_IDLE_TIMEOUT_MS to [1000, INT_MAX) in channel args to -// configure the idle timeout. -#define DEFAULT_IDLE_TIMEOUT_MS (30 /*minutes*/ * 60 * 1000) +// TODO(juanlishen): The idle filter is disabled in client channel by default +// due to b/143502997. Try to fix the bug and enable the filter by default. +#define DEFAULT_IDLE_TIMEOUT_MS INT_MAX // The user input idle timeout smaller than this would be capped to it. #define MIN_IDLE_TIMEOUT_MS (1 /*second*/ * 1000) From 6352ca36822f68b6c260253dd902fb491bf3fbb7 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Thu, 7 Nov 2019 01:10:18 -0800 Subject: [PATCH 2/3] Don't check for client_idle since client_idle is disabled --- test/core/channel/channel_args_test.cc | 10 +++++----- test/core/channel/minimal_stack_is_minimal_test.cc | 4 ++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/test/core/channel/channel_args_test.cc b/test/core/channel/channel_args_test.cc index ab859240d63..a494a387dbe 100644 --- a/test/core/channel/channel_args_test.cc +++ b/test/core/channel/channel_args_test.cc @@ -126,12 +126,12 @@ grpc_channel_args* mutate_channel_args(const char* target, return new_args; } -// Minimal stack should not have client_idle filter -static bool channel_has_client_idle_filter(grpc_channel* c) { +// Minimal stack should not have deadline filter +static bool channel_has_deadline_filter(grpc_channel* c) { grpc_channel_stack* stack = grpc_channel_get_channel_stack(c); for (size_t i = 0; i < stack->count; i++) { if (strcmp(grpc_channel_stack_element(stack, i)->filter->name, - "client_idle") == 0) { + "deadline") == 0) { return true; } } @@ -157,12 +157,12 @@ static void test_channel_create_with_global_mutator(void) { grpc_channel_args client_args = {GPR_ARRAY_SIZE(client_a), client_a}; grpc_channel* c = grpc_insecure_channel_create("no_op_mutator", &client_args, nullptr); - GPR_ASSERT(channel_has_client_idle_filter(c)); + GPR_ASSERT(channel_has_deadline_filter(c)); grpc_channel_destroy(c); c = grpc_insecure_channel_create("minimal_stack_mutator", &client_args, nullptr); - GPR_ASSERT(channel_has_client_idle_filter(c) == false); + GPR_ASSERT(channel_has_deadline_filter(c) == false); grpc_channel_destroy(c); gpr_free(fc); diff --git a/test/core/channel/minimal_stack_is_minimal_test.cc b/test/core/channel/minimal_stack_is_minimal_test.cc index 3ca1992195b..bee0bfb41f2 100644 --- a/test/core/channel/minimal_stack_is_minimal_test.cc +++ b/test/core/channel/minimal_stack_is_minimal_test.cc @@ -100,8 +100,8 @@ int main(int argc, char** argv) { errors += CHECK_STACK("chttp2", nullptr, GRPC_SERVER_CHANNEL, "server", "message_size", "deadline", "http-server", "message_compress", "connected", NULL); - errors += CHECK_STACK(nullptr, nullptr, GRPC_CLIENT_CHANNEL, - "client_idle, client-channel", NULL); + errors += CHECK_STACK(nullptr, nullptr, GRPC_CLIENT_CHANNEL, "client-channel", + NULL); GPR_ASSERT(errors == 0); grpc_shutdown(); From 9dfa8fb2f10d60937c5701a160a1d00c54cc81d5 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Thu, 14 Nov 2019 13:37:57 -0800 Subject: [PATCH 3/3] Temporarily disable client_idle filter check --- test/core/channel/channel_args_test.cc | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/test/core/channel/channel_args_test.cc b/test/core/channel/channel_args_test.cc index a494a387dbe..6f7ebc60ab6 100644 --- a/test/core/channel/channel_args_test.cc +++ b/test/core/channel/channel_args_test.cc @@ -126,12 +126,12 @@ grpc_channel_args* mutate_channel_args(const char* target, return new_args; } -// Minimal stack should not have deadline filter -static bool channel_has_deadline_filter(grpc_channel* c) { +// Minimal stack should not have client_idle filter +static bool channel_has_client_idle_filter(grpc_channel* c) { grpc_channel_stack* stack = grpc_channel_get_channel_stack(c); for (size_t i = 0; i < stack->count; i++) { if (strcmp(grpc_channel_stack_element(stack, i)->filter->name, - "deadline") == 0) { + "client_idle") == 0) { return true; } } @@ -157,12 +157,12 @@ static void test_channel_create_with_global_mutator(void) { grpc_channel_args client_args = {GPR_ARRAY_SIZE(client_a), client_a}; grpc_channel* c = grpc_insecure_channel_create("no_op_mutator", &client_args, nullptr); - GPR_ASSERT(channel_has_deadline_filter(c)); + GPR_ASSERT(channel_has_client_idle_filter(c)); grpc_channel_destroy(c); c = grpc_insecure_channel_create("minimal_stack_mutator", &client_args, nullptr); - GPR_ASSERT(channel_has_deadline_filter(c) == false); + GPR_ASSERT(channel_has_client_idle_filter(c) == false); grpc_channel_destroy(c); gpr_free(fc); @@ -206,7 +206,8 @@ int main(int argc, char** argv) { test_channel_create_with_args(); test_server_create_with_args(); // This has to be the last test. - test_channel_create_with_global_mutator(); + // TODO(markdroth): re-enable this test once client_idle is re-enabled + // test_channel_create_with_global_mutator(); grpc_shutdown(); return 0; }