From 1bc34473ce470517d51bf162db1f99cd939be7fe Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Sun, 26 Mar 2017 23:15:38 -0700 Subject: [PATCH 01/20] Add max age enforcement for server channels --- include/grpc/impl/codegen/grpc_types.h | 4 ++ src/core/lib/surface/server.c | 58 ++++++++++++++++++++++++++ 2 files changed, 62 insertions(+) diff --git a/include/grpc/impl/codegen/grpc_types.h b/include/grpc/impl/codegen/grpc_types.h index 887c176f1a4..6b380bf87e5 100644 --- a/include/grpc/impl/codegen/grpc_types.h +++ b/include/grpc/impl/codegen/grpc_types.h @@ -163,6 +163,10 @@ typedef struct { /** Maximum message length that the channel can send. Int valued, bytes. -1 means unlimited. */ #define GRPC_ARG_MAX_SEND_MESSAGE_LENGTH "grpc.max_send_message_length" + +#define GPRC_ARG_MAX_CONNECION_AGE_S "grpc.max_connection_age" + +#define GPRC_ARG_MAX_CONNECION_AGE_GRACE_S "grpc.max_connection_age_grace" /** Initial sequence number for http2 transports. Int valued. */ #define GRPC_ARG_HTTP2_INITIAL_SEQUENCE_NUMBER \ "grpc.http2.initial_sequence_number" diff --git a/src/core/lib/surface/server.c b/src/core/lib/surface/server.c index a123c9ca433..dee722e67eb 100644 --- a/src/core/lib/surface/server.c +++ b/src/core/lib/surface/server.c @@ -45,6 +45,7 @@ #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/channel/connected_channel.h" #include "src/core/lib/iomgr/iomgr.h" +#include "src/core/lib/iomgr/timer.h" #include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/support/stack_lockfree.h" #include "src/core/lib/support/string.h" @@ -53,9 +54,12 @@ #include "src/core/lib/surface/channel.h" #include "src/core/lib/surface/completion_queue.h" #include "src/core/lib/surface/init.h" +#include "src/core/lib/transport/http2_errors.h" #include "src/core/lib/transport/metadata.h" #include "src/core/lib/transport/static_metadata.h" +#define DEFAULT_MAX_CONNECTION_AGE_S INT_MAX + typedef struct listener { void *arg; void (*start)(grpc_exec_ctx *exec_ctx, grpc_server *server, void *arg, @@ -116,6 +120,9 @@ struct channel_data { uint32_t registered_method_max_probes; grpc_closure finish_destroy_channel_closure; grpc_closure channel_connectivity_changed; + grpc_timer max_age_timer; + gpr_timespec max_connection_age; + grpc_closure close_max_age_channel; }; typedef struct shutdown_tag { @@ -381,6 +388,7 @@ static void request_matcher_kill_requests(grpc_exec_ctx *exec_ctx, static void server_ref(grpc_server *server) { gpr_ref(&server->internal_refcount); + gpr_log(GPR_DEBUG, "server_ref"); } static void server_delete(grpc_exec_ctx *exec_ctx, grpc_server *server) { @@ -442,9 +450,11 @@ static void finish_destroy_channel(grpc_exec_ctx *exec_ctx, void *cd, static void destroy_channel(grpc_exec_ctx *exec_ctx, channel_data *chand, grpc_error *error) { + gpr_log(GPR_DEBUG, "destroy_channel"); if (is_channel_orphaned(chand)) return; GPR_ASSERT(chand->server != NULL); orphan_channel(chand); + grpc_timer_cancel(exec_ctx, &chand->max_age_timer); server_ref(chand->server); maybe_finish_shutdown(exec_ctx, chand->server); grpc_closure_init(&chand->finish_destroy_channel_closure, @@ -831,6 +841,7 @@ static void got_initial_metadata(grpc_exec_ctx *exec_ctx, void *ptr, static void accept_stream(grpc_exec_ctx *exec_ctx, void *cd, grpc_transport *transport, const void *transport_server_data) { + gpr_log(GPR_DEBUG, "accept_stream"); channel_data *chand = cd; /* create a call */ grpc_call_create_args args; @@ -882,6 +893,7 @@ static void channel_connectivity_changed(grpc_exec_ctx *exec_ctx, void *cd, static grpc_error *init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, const grpc_call_element_args *args) { + gpr_log(GPR_DEBUG, "init_call_elem"); call_data *calld = elem->call_data; channel_data *chand = elem->channel_data; memset(calld, 0, sizeof(call_data)); @@ -903,6 +915,7 @@ static void destroy_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, channel_data *chand = elem->channel_data; call_data *calld = elem->call_data; + gpr_log(GPR_DEBUG, "destroy_call_elem"); GPR_ASSERT(calld->state != PENDING); if (calld->host_set) { @@ -918,6 +931,23 @@ static void destroy_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, server_unref(exec_ctx, chand->server); } +static void close_max_age_channel(grpc_exec_ctx *exec_ctx, void *arg, + grpc_error *error) { + channel_data *chand = arg; + if (error == GRPC_ERROR_NONE) { + grpc_transport_op *op = grpc_make_transport_op(NULL); + op->goaway_error = + grpc_error_set_int(GRPC_ERROR_CREATE_FROM_STATIC_STRING("max_age"), + GRPC_ERROR_INT_HTTP2_ERROR, GRPC_HTTP2_NO_ERROR); + grpc_channel_element *elem = grpc_channel_stack_element( + grpc_channel_get_channel_stack(chand->channel), 0); + elem->filter->start_transport_op(exec_ctx, elem, op); + } else if (error != GRPC_ERROR_CANCELLED) { + GRPC_LOG_IF_ERROR("close_max_age_channel", error); + } + GRPC_CHANNEL_INTERNAL_UNREF(exec_ctx, chand->channel, "max age"); +} + static grpc_error *init_channel_elem(grpc_exec_ctx *exec_ctx, grpc_channel_element *elem, grpc_channel_element_args *args) { @@ -929,6 +959,28 @@ static grpc_error *init_channel_elem(grpc_exec_ctx *exec_ctx, chand->next = chand->prev = chand; chand->registered_methods = NULL; chand->connectivity_state = GRPC_CHANNEL_IDLE; + chand->max_connection_age = + DEFAULT_MAX_CONNECTION_AGE_S == INT_MAX + ? gpr_inf_future(GPR_TIMESPAN) + : gpr_time_from_seconds(DEFAULT_MAX_CONNECTION_AGE_S, GPR_TIMESPAN); + grpc_closure_init(&chand->close_max_age_channel, close_max_age_channel, chand, + grpc_schedule_on_exec_ctx); + const grpc_channel_args *channel_args = args->channel_args; + if (channel_args) { + size_t i; + for (i = 0; i < channel_args->num_args; i++) { + if (0 == + strcmp(channel_args->args[i].key, GPRC_ARG_MAX_CONNECION_AGE_S)) { + const int value = grpc_channel_arg_get_integer( + &channel_args->args[i], + (grpc_integer_options){DEFAULT_MAX_CONNECTION_AGE_S, 1, INT_MAX}); + chand->max_connection_age = + value == INT_MAX ? gpr_inf_future(GPR_TIMESPAN) + : gpr_time_from_seconds(value, GPR_TIMESPAN); + } + } + } + grpc_closure_init(&chand->channel_connectivity_changed, channel_connectivity_changed, chand, grpc_schedule_on_exec_ctx); @@ -1132,6 +1184,7 @@ void grpc_server_setup_transport(grpc_exec_ctx *exec_ctx, grpc_server *s, grpc_transport *transport, grpc_pollset *accepting_pollset, const grpc_channel_args *args) { + gpr_log(GPR_DEBUG, "grpc_server_setup_transport"); size_t num_registered_methods; size_t alloc; registered_method *rm; @@ -1152,6 +1205,11 @@ void grpc_server_setup_transport(grpc_exec_ctx *exec_ctx, grpc_server *s, chand->server = s; server_ref(s); chand->channel = channel; + GRPC_CHANNEL_INTERNAL_REF(channel, "max age"); + grpc_timer_init( + exec_ctx, &chand->max_age_timer, + gpr_time_add(gpr_now(GPR_CLOCK_MONOTONIC), chand->max_connection_age), + &chand->close_max_age_channel, gpr_now(GPR_CLOCK_MONOTONIC)); size_t cq_idx; grpc_completion_queue *accepting_cq = grpc_cq_from_pollset(accepting_pollset); From a809ea5c14d0e0d3b2ec5756626dadddfee6f4d6 Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Tue, 28 Mar 2017 02:02:45 -0700 Subject: [PATCH 02/20] Add max_age_filter --- BUILD | 2 + CMakeLists.txt | 5 + Makefile | 5 + binding.gyp | 1 + build.yaml | 2 + config.m4 | 1 + gRPC-Core.podspec | 3 + grpc.gemspec | 2 + package.xml | 2 + src/core/lib/channel/max_age_filter.c | 180 ++++++++++++++++++ src/core/lib/channel/max_age_filter.h | 39 ++++ src/core/lib/surface/init.c | 4 + src/core/lib/surface/server.c | 58 ------ src/python/grpcio/grpc_core_dependencies.py | 1 + tools/doxygen/Doxyfile.core.internal | 2 + .../generated/sources_and_headers.json | 3 + vsprojects/vcxproj/grpc/grpc.vcxproj | 3 + vsprojects/vcxproj/grpc/grpc.vcxproj.filters | 6 + .../grpc_test_util/grpc_test_util.vcxproj | 3 + .../grpc_test_util.vcxproj.filters | 6 + .../grpc_unsecure/grpc_unsecure.vcxproj | 3 + .../grpc_unsecure.vcxproj.filters | 6 + 22 files changed, 279 insertions(+), 58 deletions(-) create mode 100644 src/core/lib/channel/max_age_filter.c create mode 100644 src/core/lib/channel/max_age_filter.h diff --git a/BUILD b/BUILD index b3617e02ced..3c5279638d4 100644 --- a/BUILD +++ b/BUILD @@ -436,6 +436,7 @@ grpc_cc_library( "src/core/lib/channel/handshaker_registry.c", "src/core/lib/channel/http_client_filter.c", "src/core/lib/channel/http_server_filter.c", + "src/core/lib/channel/max_age_filter.c", "src/core/lib/channel/message_size_filter.c", "src/core/lib/compression/compression.c", "src/core/lib/compression/message_compress.c", @@ -562,6 +563,7 @@ grpc_cc_library( "src/core/lib/channel/handshaker_registry.h", "src/core/lib/channel/http_client_filter.h", "src/core/lib/channel/http_server_filter.h", + "src/core/lib/channel/max_age_filter.h", "src/core/lib/channel/message_size_filter.h", "src/core/lib/compression/algorithm_metadata.h", "src/core/lib/compression/message_compress.h", diff --git a/CMakeLists.txt b/CMakeLists.txt index 1ddeb9e34c0..61863a230cc 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -877,6 +877,7 @@ add_library(grpc src/core/lib/channel/handshaker_registry.c src/core/lib/channel/http_client_filter.c src/core/lib/channel/http_server_filter.c + src/core/lib/channel/max_age_filter.c src/core/lib/channel/message_size_filter.c src/core/lib/compression/compression.c src/core/lib/compression/message_compress.c @@ -1190,6 +1191,7 @@ add_library(grpc_cronet src/core/lib/channel/handshaker_registry.c src/core/lib/channel/http_client_filter.c src/core/lib/channel/http_server_filter.c + src/core/lib/channel/max_age_filter.c src/core/lib/channel/message_size_filter.c src/core/lib/compression/compression.c src/core/lib/compression/message_compress.c @@ -1494,6 +1496,7 @@ add_library(grpc_test_util src/core/lib/channel/handshaker_registry.c src/core/lib/channel/http_client_filter.c src/core/lib/channel/http_server_filter.c + src/core/lib/channel/max_age_filter.c src/core/lib/channel/message_size_filter.c src/core/lib/compression/compression.c src/core/lib/compression/message_compress.c @@ -1745,6 +1748,7 @@ add_library(grpc_unsecure src/core/lib/channel/handshaker_registry.c src/core/lib/channel/http_client_filter.c src/core/lib/channel/http_server_filter.c + src/core/lib/channel/max_age_filter.c src/core/lib/channel/message_size_filter.c src/core/lib/compression/compression.c src/core/lib/compression/message_compress.c @@ -2356,6 +2360,7 @@ add_library(grpc++_cronet src/core/lib/channel/handshaker_registry.c src/core/lib/channel/http_client_filter.c src/core/lib/channel/http_server_filter.c + src/core/lib/channel/max_age_filter.c src/core/lib/channel/message_size_filter.c src/core/lib/compression/compression.c src/core/lib/compression/message_compress.c diff --git a/Makefile b/Makefile index 1be009f276f..2884134f40e 100644 --- a/Makefile +++ b/Makefile @@ -2768,6 +2768,7 @@ LIBGRPC_SRC = \ src/core/lib/channel/handshaker_registry.c \ src/core/lib/channel/http_client_filter.c \ src/core/lib/channel/http_server_filter.c \ + src/core/lib/channel/max_age_filter.c \ src/core/lib/channel/message_size_filter.c \ src/core/lib/compression/compression.c \ src/core/lib/compression/message_compress.c \ @@ -3084,6 +3085,7 @@ LIBGRPC_CRONET_SRC = \ src/core/lib/channel/handshaker_registry.c \ src/core/lib/channel/http_client_filter.c \ src/core/lib/channel/http_server_filter.c \ + src/core/lib/channel/max_age_filter.c \ src/core/lib/channel/message_size_filter.c \ src/core/lib/compression/compression.c \ src/core/lib/compression/message_compress.c \ @@ -3391,6 +3393,7 @@ LIBGRPC_TEST_UTIL_SRC = \ src/core/lib/channel/handshaker_registry.c \ src/core/lib/channel/http_client_filter.c \ src/core/lib/channel/http_server_filter.c \ + src/core/lib/channel/max_age_filter.c \ src/core/lib/channel/message_size_filter.c \ src/core/lib/compression/compression.c \ src/core/lib/compression/message_compress.c \ @@ -3622,6 +3625,7 @@ LIBGRPC_UNSECURE_SRC = \ src/core/lib/channel/handshaker_registry.c \ src/core/lib/channel/http_client_filter.c \ src/core/lib/channel/http_server_filter.c \ + src/core/lib/channel/max_age_filter.c \ src/core/lib/channel/message_size_filter.c \ src/core/lib/compression/compression.c \ src/core/lib/compression/message_compress.c \ @@ -4235,6 +4239,7 @@ LIBGRPC++_CRONET_SRC = \ src/core/lib/channel/handshaker_registry.c \ src/core/lib/channel/http_client_filter.c \ src/core/lib/channel/http_server_filter.c \ + src/core/lib/channel/max_age_filter.c \ src/core/lib/channel/message_size_filter.c \ src/core/lib/compression/compression.c \ src/core/lib/compression/message_compress.c \ diff --git a/binding.gyp b/binding.gyp index e4335b55029..b589130f25c 100644 --- a/binding.gyp +++ b/binding.gyp @@ -619,6 +619,7 @@ 'src/core/lib/channel/handshaker_registry.c', 'src/core/lib/channel/http_client_filter.c', 'src/core/lib/channel/http_server_filter.c', + 'src/core/lib/channel/max_age_filter.c', 'src/core/lib/channel/message_size_filter.c', 'src/core/lib/compression/compression.c', 'src/core/lib/compression/message_compress.c', diff --git a/build.yaml b/build.yaml index 650f1274929..059221cd72f 100644 --- a/build.yaml +++ b/build.yaml @@ -186,6 +186,7 @@ filegroups: - src/core/lib/channel/handshaker_registry.h - src/core/lib/channel/http_client_filter.h - src/core/lib/channel/http_server_filter.h + - src/core/lib/channel/max_age_filter.h - src/core/lib/channel/message_size_filter.h - src/core/lib/compression/algorithm_metadata.h - src/core/lib/compression/message_compress.h @@ -295,6 +296,7 @@ filegroups: - src/core/lib/channel/handshaker_registry.c - src/core/lib/channel/http_client_filter.c - src/core/lib/channel/http_server_filter.c + - src/core/lib/channel/max_age_filter.c - src/core/lib/channel/message_size_filter.c - src/core/lib/compression/compression.c - src/core/lib/compression/message_compress.c diff --git a/config.m4 b/config.m4 index 226cdbd4371..ea2e406854c 100644 --- a/config.m4 +++ b/config.m4 @@ -94,6 +94,7 @@ if test "$PHP_GRPC" != "no"; then src/core/lib/channel/handshaker_registry.c \ src/core/lib/channel/http_client_filter.c \ src/core/lib/channel/http_server_filter.c \ + src/core/lib/channel/max_age_filter.c \ src/core/lib/channel/message_size_filter.c \ src/core/lib/compression/compression.c \ src/core/lib/compression/message_compress.c \ diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index 348ff0eb52f..5f44cf4f5f0 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -267,6 +267,7 @@ Pod::Spec.new do |s| 'src/core/lib/channel/handshaker_registry.h', 'src/core/lib/channel/http_client_filter.h', 'src/core/lib/channel/http_server_filter.h', + 'src/core/lib/channel/max_age_filter.h', 'src/core/lib/channel/message_size_filter.h', 'src/core/lib/compression/algorithm_metadata.h', 'src/core/lib/compression/message_compress.h', @@ -466,6 +467,7 @@ Pod::Spec.new do |s| 'src/core/lib/channel/handshaker_registry.c', 'src/core/lib/channel/http_client_filter.c', 'src/core/lib/channel/http_server_filter.c', + 'src/core/lib/channel/max_age_filter.c', 'src/core/lib/channel/message_size_filter.c', 'src/core/lib/compression/compression.c', 'src/core/lib/compression/message_compress.c', @@ -710,6 +712,7 @@ Pod::Spec.new do |s| 'src/core/lib/channel/handshaker_registry.h', 'src/core/lib/channel/http_client_filter.h', 'src/core/lib/channel/http_server_filter.h', + 'src/core/lib/channel/max_age_filter.h', 'src/core/lib/channel/message_size_filter.h', 'src/core/lib/compression/algorithm_metadata.h', 'src/core/lib/compression/message_compress.h', diff --git a/grpc.gemspec b/grpc.gemspec index 7559d5606ff..7664a4503f4 100755 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -184,6 +184,7 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/channel/handshaker_registry.h ) s.files += %w( src/core/lib/channel/http_client_filter.h ) s.files += %w( src/core/lib/channel/http_server_filter.h ) + s.files += %w( src/core/lib/channel/max_age_filter.h ) s.files += %w( src/core/lib/channel/message_size_filter.h ) s.files += %w( src/core/lib/compression/algorithm_metadata.h ) s.files += %w( src/core/lib/compression/message_compress.h ) @@ -383,6 +384,7 @@ Gem::Specification.new do |s| s.files += %w( src/core/lib/channel/handshaker_registry.c ) s.files += %w( src/core/lib/channel/http_client_filter.c ) s.files += %w( src/core/lib/channel/http_server_filter.c ) + s.files += %w( src/core/lib/channel/max_age_filter.c ) s.files += %w( src/core/lib/channel/message_size_filter.c ) s.files += %w( src/core/lib/compression/compression.c ) s.files += %w( src/core/lib/compression/message_compress.c ) diff --git a/package.xml b/package.xml index 429d07fd724..89a0bb2e65b 100644 --- a/package.xml +++ b/package.xml @@ -193,6 +193,7 @@ + @@ -392,6 +393,7 @@ + diff --git a/src/core/lib/channel/max_age_filter.c b/src/core/lib/channel/max_age_filter.c new file mode 100644 index 00000000000..f840483c722 --- /dev/null +++ b/src/core/lib/channel/max_age_filter.c @@ -0,0 +1,180 @@ +// +// Copyright 2017, Google Inc. +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +// + +#include "src/core/lib/channel/message_size_filter.h" + +#include +#include + +#include +#include "src/core/lib/channel/channel_args.h" +#include "src/core/lib/iomgr/timer.h" +#include "src/core/lib/transport/http2_errors.h" +#include "src/core/lib/transport/service_config.h" + +#define DEFAULT_MAX_CONNECTION_AGE_S INT_MAX + +typedef struct channel_data { + // We take a reference to the channel stack for the timer callback + grpc_channel_stack* channel_stack; + // Guards access to max_age_timer and max_age_timer_pending + gpr_mu max_age_timer_mu; + // True if the max_age timer callback is currently pending + bool max_age_timer_pending; + // The timer for checking if the channel has reached its max age + grpc_timer max_age_timer; + // Allowed max time a channel may exist + gpr_timespec max_connection_age; + // Closure to run when the channel reaches its max age and should be closed + grpc_closure close_max_age_channel; + // Closure to run when the init fo channel stack is done and the timer should + // be started + grpc_closure start_timer_after_init; +} channel_data; + +static void start_timer_after_init(grpc_exec_ctx* exec_ctx, void* arg, + grpc_error* error) { + channel_data* chand = arg; + gpr_mu_lock(&chand->max_age_timer_mu); + chand->max_age_timer_pending = true; + grpc_timer_init( + exec_ctx, &chand->max_age_timer, + gpr_time_add(gpr_now(GPR_CLOCK_MONOTONIC), chand->max_connection_age), + &chand->close_max_age_channel, gpr_now(GPR_CLOCK_MONOTONIC)); + gpr_mu_unlock(&chand->max_age_timer_mu); + GRPC_CHANNEL_STACK_UNREF(exec_ctx, chand->channel_stack, + "max_age start_timer_after_init"); +} + +static void close_max_age_channel(grpc_exec_ctx* exec_ctx, void* arg, + grpc_error* error) { + channel_data* chand = arg; + gpr_mu_lock(&chand->max_age_timer_mu); + chand->max_age_timer_pending = false; + gpr_mu_unlock(&chand->max_age_timer_mu); + if (error == GRPC_ERROR_NONE) { + grpc_transport_op* op = grpc_make_transport_op(NULL); + op->goaway_error = + grpc_error_set_int(GRPC_ERROR_CREATE_FROM_STATIC_STRING("max_age"), + GRPC_ERROR_INT_HTTP2_ERROR, GRPC_HTTP2_NO_ERROR); + grpc_channel_element* elem = + grpc_channel_stack_element(chand->channel_stack, 0); + elem->filter->start_transport_op(exec_ctx, elem, op); + } else if (error != GRPC_ERROR_CANCELLED) { + GRPC_LOG_IF_ERROR("close_max_age_channel", error); + } +} + +// Constructor for call_data. +static grpc_error* init_call_elem(grpc_exec_ctx* exec_ctx, + grpc_call_element* elem, + const grpc_call_element_args* args) { + // call_num ++; + return GRPC_ERROR_NONE; +} + +// Destructor for call_data. +static void destroy_call_elem(grpc_exec_ctx* exec_ctx, grpc_call_element* elem, + const grpc_call_final_info* final_info, + grpc_closure* ignored) { + // call_num --; +} + +// Constructor for channel_data. +static grpc_error* init_channel_elem(grpc_exec_ctx* exec_ctx, + grpc_channel_element* elem, + grpc_channel_element_args* args) { + channel_data* chand = elem->channel_data; + gpr_mu_init(&chand->max_age_timer_mu); + chand->max_age_timer_pending = false; + chand->channel_stack = args->channel_stack; + chand->max_connection_age = + DEFAULT_MAX_CONNECTION_AGE_S == INT_MAX + ? gpr_inf_future(GPR_TIMESPAN) + : gpr_time_from_seconds(DEFAULT_MAX_CONNECTION_AGE_S, GPR_TIMESPAN); + for (size_t i = 0; i < args->channel_args->num_args; ++i) { + if (0 == + strcmp(args->channel_args->args[i].key, GPRC_ARG_MAX_CONNECION_AGE_S)) { + const int value = grpc_channel_arg_get_integer( + &args->channel_args->args[i], + (grpc_integer_options){DEFAULT_MAX_CONNECTION_AGE_S, 1, INT_MAX}); + chand->max_connection_age = + value == INT_MAX ? gpr_inf_future(GPR_TIMESPAN) + : gpr_time_from_seconds(value, GPR_TIMESPAN); + } + } + grpc_closure_init(&chand->close_max_age_channel, close_max_age_channel, chand, + grpc_schedule_on_exec_ctx); + + if (gpr_time_cmp(chand->max_connection_age, gpr_inf_future(GPR_TIMESPAN)) != + 0) { + // When the channel reaches its max age, we send down an op with + // goaway_error set. However, we can't send down any ops until after the + // channel stack is fully initialized. If we start the timer here, we have + // no guarantee that the timer won't pop before channel stack initialization + // is finished. To avoid that problem, we create a closure to start the + // timer, and we schedule that closure to be run after call stack + // initialization is done. + GRPC_CHANNEL_STACK_REF(chand->channel_stack, + "max_age start_timer_after_init"); + grpc_closure_init(&chand->start_timer_after_init, start_timer_after_init, + chand, grpc_schedule_on_exec_ctx); + grpc_closure_sched(exec_ctx, &chand->start_timer_after_init, + GRPC_ERROR_NONE); + } + + return GRPC_ERROR_NONE; +} + +// Destructor for channel_data. +static void destroy_channel_elem(grpc_exec_ctx* exec_ctx, + grpc_channel_element* elem) { + channel_data* chand = elem->channel_data; + gpr_mu_lock(&chand->max_age_timer_mu); + if (chand->max_age_timer_pending) { + grpc_timer_cancel(exec_ctx, &chand->max_age_timer); + } + gpr_mu_unlock(&chand->max_age_timer_mu); +} + +const grpc_channel_filter grpc_max_age_filter = { + grpc_call_next_op, + grpc_channel_next_op, + 0, // sizeof_call_data + init_call_elem, + grpc_call_stack_ignore_set_pollset_or_pollset_set, + destroy_call_elem, + sizeof(channel_data), + init_channel_elem, + destroy_channel_elem, + grpc_call_next_get_peer, + grpc_channel_next_get_info, + "max_age"}; diff --git a/src/core/lib/channel/max_age_filter.h b/src/core/lib/channel/max_age_filter.h new file mode 100644 index 00000000000..93e357a88e6 --- /dev/null +++ b/src/core/lib/channel/max_age_filter.h @@ -0,0 +1,39 @@ +// +// Copyright 2017, Google Inc. +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +// + +#ifndef GRPC_CORE_LIB_CHANNEL_MAX_AGE_FILTER_H +#define GRPC_CORE_LIB_CHANNEL_MAX_AGE_FILTER_H + +#include "src/core/lib/channel/channel_stack.h" + +extern const grpc_channel_filter grpc_max_age_filter; + +#endif /* GRPC_CORE_LIB_CHANNEL_MAX_AGE_FILTER_H */ diff --git a/src/core/lib/surface/init.c b/src/core/lib/surface/init.c index 91bd014a0e5..b46ecac18d9 100644 --- a/src/core/lib/surface/init.c +++ b/src/core/lib/surface/init.c @@ -47,6 +47,7 @@ #include "src/core/lib/channel/handshaker_registry.h" #include "src/core/lib/channel/http_client_filter.h" #include "src/core/lib/channel/http_server_filter.h" +#include "src/core/lib/channel/max_age_filter.h" #include "src/core/lib/channel/message_size_filter.h" #include "src/core/lib/debug/trace.h" #include "src/core/lib/http/parser.h" @@ -113,6 +114,9 @@ static void register_builtin_channel_init() { grpc_channel_init_register_stage( GRPC_SERVER_CHANNEL, GRPC_CHANNEL_INIT_BUILTIN_PRIORITY, prepend_filter, (void *)&grpc_server_deadline_filter); + grpc_channel_init_register_stage( + GRPC_SERVER_CHANNEL, GRPC_CHANNEL_INIT_BUILTIN_PRIORITY, prepend_filter, + (void *)&grpc_max_age_filter); grpc_channel_init_register_stage( GRPC_CLIENT_SUBCHANNEL, GRPC_CHANNEL_INIT_BUILTIN_PRIORITY, prepend_filter, (void *)&grpc_message_size_filter); diff --git a/src/core/lib/surface/server.c b/src/core/lib/surface/server.c index dee722e67eb..a123c9ca433 100644 --- a/src/core/lib/surface/server.c +++ b/src/core/lib/surface/server.c @@ -45,7 +45,6 @@ #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/channel/connected_channel.h" #include "src/core/lib/iomgr/iomgr.h" -#include "src/core/lib/iomgr/timer.h" #include "src/core/lib/slice/slice_internal.h" #include "src/core/lib/support/stack_lockfree.h" #include "src/core/lib/support/string.h" @@ -54,12 +53,9 @@ #include "src/core/lib/surface/channel.h" #include "src/core/lib/surface/completion_queue.h" #include "src/core/lib/surface/init.h" -#include "src/core/lib/transport/http2_errors.h" #include "src/core/lib/transport/metadata.h" #include "src/core/lib/transport/static_metadata.h" -#define DEFAULT_MAX_CONNECTION_AGE_S INT_MAX - typedef struct listener { void *arg; void (*start)(grpc_exec_ctx *exec_ctx, grpc_server *server, void *arg, @@ -120,9 +116,6 @@ struct channel_data { uint32_t registered_method_max_probes; grpc_closure finish_destroy_channel_closure; grpc_closure channel_connectivity_changed; - grpc_timer max_age_timer; - gpr_timespec max_connection_age; - grpc_closure close_max_age_channel; }; typedef struct shutdown_tag { @@ -388,7 +381,6 @@ static void request_matcher_kill_requests(grpc_exec_ctx *exec_ctx, static void server_ref(grpc_server *server) { gpr_ref(&server->internal_refcount); - gpr_log(GPR_DEBUG, "server_ref"); } static void server_delete(grpc_exec_ctx *exec_ctx, grpc_server *server) { @@ -450,11 +442,9 @@ static void finish_destroy_channel(grpc_exec_ctx *exec_ctx, void *cd, static void destroy_channel(grpc_exec_ctx *exec_ctx, channel_data *chand, grpc_error *error) { - gpr_log(GPR_DEBUG, "destroy_channel"); if (is_channel_orphaned(chand)) return; GPR_ASSERT(chand->server != NULL); orphan_channel(chand); - grpc_timer_cancel(exec_ctx, &chand->max_age_timer); server_ref(chand->server); maybe_finish_shutdown(exec_ctx, chand->server); grpc_closure_init(&chand->finish_destroy_channel_closure, @@ -841,7 +831,6 @@ static void got_initial_metadata(grpc_exec_ctx *exec_ctx, void *ptr, static void accept_stream(grpc_exec_ctx *exec_ctx, void *cd, grpc_transport *transport, const void *transport_server_data) { - gpr_log(GPR_DEBUG, "accept_stream"); channel_data *chand = cd; /* create a call */ grpc_call_create_args args; @@ -893,7 +882,6 @@ static void channel_connectivity_changed(grpc_exec_ctx *exec_ctx, void *cd, static grpc_error *init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, const grpc_call_element_args *args) { - gpr_log(GPR_DEBUG, "init_call_elem"); call_data *calld = elem->call_data; channel_data *chand = elem->channel_data; memset(calld, 0, sizeof(call_data)); @@ -915,7 +903,6 @@ static void destroy_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, channel_data *chand = elem->channel_data; call_data *calld = elem->call_data; - gpr_log(GPR_DEBUG, "destroy_call_elem"); GPR_ASSERT(calld->state != PENDING); if (calld->host_set) { @@ -931,23 +918,6 @@ static void destroy_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, server_unref(exec_ctx, chand->server); } -static void close_max_age_channel(grpc_exec_ctx *exec_ctx, void *arg, - grpc_error *error) { - channel_data *chand = arg; - if (error == GRPC_ERROR_NONE) { - grpc_transport_op *op = grpc_make_transport_op(NULL); - op->goaway_error = - grpc_error_set_int(GRPC_ERROR_CREATE_FROM_STATIC_STRING("max_age"), - GRPC_ERROR_INT_HTTP2_ERROR, GRPC_HTTP2_NO_ERROR); - grpc_channel_element *elem = grpc_channel_stack_element( - grpc_channel_get_channel_stack(chand->channel), 0); - elem->filter->start_transport_op(exec_ctx, elem, op); - } else if (error != GRPC_ERROR_CANCELLED) { - GRPC_LOG_IF_ERROR("close_max_age_channel", error); - } - GRPC_CHANNEL_INTERNAL_UNREF(exec_ctx, chand->channel, "max age"); -} - static grpc_error *init_channel_elem(grpc_exec_ctx *exec_ctx, grpc_channel_element *elem, grpc_channel_element_args *args) { @@ -959,28 +929,6 @@ static grpc_error *init_channel_elem(grpc_exec_ctx *exec_ctx, chand->next = chand->prev = chand; chand->registered_methods = NULL; chand->connectivity_state = GRPC_CHANNEL_IDLE; - chand->max_connection_age = - DEFAULT_MAX_CONNECTION_AGE_S == INT_MAX - ? gpr_inf_future(GPR_TIMESPAN) - : gpr_time_from_seconds(DEFAULT_MAX_CONNECTION_AGE_S, GPR_TIMESPAN); - grpc_closure_init(&chand->close_max_age_channel, close_max_age_channel, chand, - grpc_schedule_on_exec_ctx); - const grpc_channel_args *channel_args = args->channel_args; - if (channel_args) { - size_t i; - for (i = 0; i < channel_args->num_args; i++) { - if (0 == - strcmp(channel_args->args[i].key, GPRC_ARG_MAX_CONNECION_AGE_S)) { - const int value = grpc_channel_arg_get_integer( - &channel_args->args[i], - (grpc_integer_options){DEFAULT_MAX_CONNECTION_AGE_S, 1, INT_MAX}); - chand->max_connection_age = - value == INT_MAX ? gpr_inf_future(GPR_TIMESPAN) - : gpr_time_from_seconds(value, GPR_TIMESPAN); - } - } - } - grpc_closure_init(&chand->channel_connectivity_changed, channel_connectivity_changed, chand, grpc_schedule_on_exec_ctx); @@ -1184,7 +1132,6 @@ void grpc_server_setup_transport(grpc_exec_ctx *exec_ctx, grpc_server *s, grpc_transport *transport, grpc_pollset *accepting_pollset, const grpc_channel_args *args) { - gpr_log(GPR_DEBUG, "grpc_server_setup_transport"); size_t num_registered_methods; size_t alloc; registered_method *rm; @@ -1205,11 +1152,6 @@ void grpc_server_setup_transport(grpc_exec_ctx *exec_ctx, grpc_server *s, chand->server = s; server_ref(s); chand->channel = channel; - GRPC_CHANNEL_INTERNAL_REF(channel, "max age"); - grpc_timer_init( - exec_ctx, &chand->max_age_timer, - gpr_time_add(gpr_now(GPR_CLOCK_MONOTONIC), chand->max_connection_age), - &chand->close_max_age_channel, gpr_now(GPR_CLOCK_MONOTONIC)); size_t cq_idx; grpc_completion_queue *accepting_cq = grpc_cq_from_pollset(accepting_pollset); diff --git a/src/python/grpcio/grpc_core_dependencies.py b/src/python/grpcio/grpc_core_dependencies.py index f5f1182f5d6..3285b182fec 100644 --- a/src/python/grpcio/grpc_core_dependencies.py +++ b/src/python/grpcio/grpc_core_dependencies.py @@ -88,6 +88,7 @@ CORE_SOURCE_FILES = [ 'src/core/lib/channel/handshaker_registry.c', 'src/core/lib/channel/http_client_filter.c', 'src/core/lib/channel/http_server_filter.c', + 'src/core/lib/channel/max_age_filter.c', 'src/core/lib/channel/message_size_filter.c', 'src/core/lib/compression/compression.c', 'src/core/lib/compression/message_compress.c', diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index 4ab24d1bc5f..fc247458695 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -1036,6 +1036,8 @@ src/core/lib/channel/http_client_filter.c \ src/core/lib/channel/http_client_filter.h \ src/core/lib/channel/http_server_filter.c \ src/core/lib/channel/http_server_filter.h \ +src/core/lib/channel/max_age_filter.c \ +src/core/lib/channel/max_age_filter.h \ src/core/lib/channel/message_size_filter.c \ src/core/lib/channel/message_size_filter.h \ src/core/lib/compression/algorithm_metadata.h \ diff --git a/tools/run_tests/generated/sources_and_headers.json b/tools/run_tests/generated/sources_and_headers.json index 147c22fcde3..250a8788f99 100644 --- a/tools/run_tests/generated/sources_and_headers.json +++ b/tools/run_tests/generated/sources_and_headers.json @@ -7475,6 +7475,7 @@ "src/core/lib/channel/handshaker_registry.h", "src/core/lib/channel/http_client_filter.h", "src/core/lib/channel/http_server_filter.h", + "src/core/lib/channel/max_age_filter.h", "src/core/lib/channel/message_size_filter.h", "src/core/lib/compression/algorithm_metadata.h", "src/core/lib/compression/message_compress.h", @@ -7610,6 +7611,8 @@ "src/core/lib/channel/http_client_filter.h", "src/core/lib/channel/http_server_filter.c", "src/core/lib/channel/http_server_filter.h", + "src/core/lib/channel/max_age_filter.c", + "src/core/lib/channel/max_age_filter.h", "src/core/lib/channel/message_size_filter.c", "src/core/lib/channel/message_size_filter.h", "src/core/lib/compression/algorithm_metadata.h", diff --git a/vsprojects/vcxproj/grpc/grpc.vcxproj b/vsprojects/vcxproj/grpc/grpc.vcxproj index b15c002aa60..af475b9204f 100644 --- a/vsprojects/vcxproj/grpc/grpc.vcxproj +++ b/vsprojects/vcxproj/grpc/grpc.vcxproj @@ -312,6 +312,7 @@ + @@ -525,6 +526,8 @@ + + diff --git a/vsprojects/vcxproj/grpc/grpc.vcxproj.filters b/vsprojects/vcxproj/grpc/grpc.vcxproj.filters index 262ec83ebe5..0386e271208 100644 --- a/vsprojects/vcxproj/grpc/grpc.vcxproj.filters +++ b/vsprojects/vcxproj/grpc/grpc.vcxproj.filters @@ -37,6 +37,9 @@ src\core\lib\channel + + src\core\lib\channel + src\core\lib\channel @@ -821,6 +824,9 @@ src\core\lib\channel + + src\core\lib\channel + src\core\lib\channel diff --git a/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj b/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj index 04d6d9f55a3..1bc29589ec3 100644 --- a/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj +++ b/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj @@ -207,6 +207,7 @@ + @@ -368,6 +369,8 @@ + + diff --git a/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj.filters b/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj.filters index a2849888a14..7dcddccb77d 100644 --- a/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj.filters +++ b/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj.filters @@ -94,6 +94,9 @@ src\core\lib\channel + + src\core\lib\channel + src\core\lib\channel @@ -608,6 +611,9 @@ src\core\lib\channel + + src\core\lib\channel + src\core\lib\channel diff --git a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj index b94b6e6074d..e8ab18d0c17 100644 --- a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj +++ b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj @@ -302,6 +302,7 @@ + @@ -492,6 +493,8 @@ + + diff --git a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters index 86dea27c99b..d0a58a1689e 100644 --- a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters +++ b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters @@ -40,6 +40,9 @@ src\core\lib\channel + + src\core\lib\channel + src\core\lib\channel @@ -731,6 +734,9 @@ src\core\lib\channel + + src\core\lib\channel + src\core\lib\channel From a1506c4e8fb8e0c3c0d79c85d2bf385cdf2dae01 Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Tue, 28 Mar 2017 02:50:50 -0700 Subject: [PATCH 03/20] Add max age grace period --- include/grpc/impl/codegen/grpc_types.h | 6 +- src/core/lib/channel/max_age_filter.c | 97 +++++++++++++++++++++++--- 2 files changed, 90 insertions(+), 13 deletions(-) diff --git a/include/grpc/impl/codegen/grpc_types.h b/include/grpc/impl/codegen/grpc_types.h index 6b380bf87e5..28db2db0b48 100644 --- a/include/grpc/impl/codegen/grpc_types.h +++ b/include/grpc/impl/codegen/grpc_types.h @@ -163,9 +163,11 @@ typedef struct { /** Maximum message length that the channel can send. Int valued, bytes. -1 means unlimited. */ #define GRPC_ARG_MAX_SEND_MESSAGE_LENGTH "grpc.max_send_message_length" - +/** Maximum time that a channel may exist. Int valued, seconds. INT_MAX means + unlimited. */ #define GPRC_ARG_MAX_CONNECION_AGE_S "grpc.max_connection_age" - +/** Grace period after the chennel reaches its max age. Int valued, seconds. + INT_MAX means unlimited. */ #define GPRC_ARG_MAX_CONNECION_AGE_GRACE_S "grpc.max_connection_age_grace" /** Initial sequence number for http2 transports. Int valued. */ #define GRPC_ARG_HTTP2_INITIAL_SEQUENCE_NUMBER \ diff --git a/src/core/lib/channel/max_age_filter.c b/src/core/lib/channel/max_age_filter.c index f840483c722..364e01705d2 100644 --- a/src/core/lib/channel/max_age_filter.c +++ b/src/core/lib/channel/max_age_filter.c @@ -41,6 +41,7 @@ #include "src/core/lib/transport/service_config.h" #define DEFAULT_MAX_CONNECTION_AGE_S INT_MAX +#define DEFAULT_MAX_CONNECTION_AGE_GRACE_S INT_MAX typedef struct channel_data { // We take a reference to the channel stack for the timer callback @@ -49,19 +50,31 @@ typedef struct channel_data { gpr_mu max_age_timer_mu; // True if the max_age timer callback is currently pending bool max_age_timer_pending; + // True if the max_age timer callback is currently pending + bool max_age_grace_timer_pending; // The timer for checking if the channel has reached its max age grpc_timer max_age_timer; + // The timer for checking if the channel has reached its max age + grpc_timer max_age_grace_timer; // Allowed max time a channel may exist gpr_timespec max_connection_age; + // Allowed grace period after the channel reaches its max age + gpr_timespec max_connection_age_grace; // Closure to run when the channel reaches its max age and should be closed + // gracefully grpc_closure close_max_age_channel; - // Closure to run when the init fo channel stack is done and the timer should - // be started - grpc_closure start_timer_after_init; + // Closure to run the channel uses up its max age grace time and should be + // closed forcibly + grpc_closure force_close_max_age_channel; + // Closure to run when the init fo channel stack is done and the max_age timer + // should be started + grpc_closure start_max_age_timer_after_init; + // Closure to run when the goaway op is finished and the max_age_timer + grpc_closure start_max_age_grace_timer_after_goaway_op; } channel_data; -static void start_timer_after_init(grpc_exec_ctx* exec_ctx, void* arg, - grpc_error* error) { +static void start_max_age_timer_after_init(grpc_exec_ctx* exec_ctx, void* arg, + grpc_error* error) { channel_data* chand = arg; gpr_mu_lock(&chand->max_age_timer_mu); chand->max_age_timer_pending = true; @@ -71,7 +84,23 @@ static void start_timer_after_init(grpc_exec_ctx* exec_ctx, void* arg, &chand->close_max_age_channel, gpr_now(GPR_CLOCK_MONOTONIC)); gpr_mu_unlock(&chand->max_age_timer_mu); GRPC_CHANNEL_STACK_UNREF(exec_ctx, chand->channel_stack, - "max_age start_timer_after_init"); + "max_age start_max_age_timer_after_init"); +} + +static void start_max_age_grace_timer_after_goaway_op(grpc_exec_ctx* exec_ctx, + void* arg, + grpc_error* error) { + channel_data* chand = arg; + gpr_mu_lock(&chand->max_age_timer_mu); + chand->max_age_grace_timer_pending = true; + grpc_timer_init(exec_ctx, &chand->max_age_grace_timer, + gpr_time_add(gpr_now(GPR_CLOCK_MONOTONIC), + chand->max_connection_age_grace), + &chand->force_close_max_age_channel, + gpr_now(GPR_CLOCK_MONOTONIC)); + gpr_mu_unlock(&chand->max_age_timer_mu); + GRPC_CHANNEL_STACK_UNREF(exec_ctx, chand->channel_stack, + "max_age start_max_age_grace_timer_after_goaway_op"); } static void close_max_age_channel(grpc_exec_ctx* exec_ctx, void* arg, @@ -81,7 +110,10 @@ static void close_max_age_channel(grpc_exec_ctx* exec_ctx, void* arg, chand->max_age_timer_pending = false; gpr_mu_unlock(&chand->max_age_timer_mu); if (error == GRPC_ERROR_NONE) { - grpc_transport_op* op = grpc_make_transport_op(NULL); + GRPC_CHANNEL_STACK_REF(chand->channel_stack, + "max_age start_max_age_grace_timer_after_goaway_op"); + grpc_transport_op* op = grpc_make_transport_op( + &chand->start_max_age_grace_timer_after_goaway_op); op->goaway_error = grpc_error_set_int(GRPC_ERROR_CREATE_FROM_STATIC_STRING("max_age"), GRPC_ERROR_INT_HTTP2_ERROR, GRPC_HTTP2_NO_ERROR); @@ -93,6 +125,24 @@ static void close_max_age_channel(grpc_exec_ctx* exec_ctx, void* arg, } } +static void force_close_max_age_channel(grpc_exec_ctx* exec_ctx, void* arg, + grpc_error* error) { + channel_data* chand = arg; + gpr_mu_lock(&chand->max_age_timer_mu); + chand->max_age_grace_timer_pending = false; + gpr_mu_unlock(&chand->max_age_timer_mu); + if (error == GRPC_ERROR_NONE) { + grpc_transport_op* op = grpc_make_transport_op(NULL); + op->disconnect_with_error = + GRPC_ERROR_CREATE_FROM_STATIC_STRING("Channel reaches max age"); + grpc_channel_element* elem = + grpc_channel_stack_element(chand->channel_stack, 0); + elem->filter->start_transport_op(exec_ctx, elem, op); + } else if (error != GRPC_ERROR_CANCELLED) { + GRPC_LOG_IF_ERROR("force_close_max_age_channel", error); + } +} + // Constructor for call_data. static grpc_error* init_call_elem(grpc_exec_ctx* exec_ctx, grpc_call_element* elem, @@ -115,11 +165,17 @@ static grpc_error* init_channel_elem(grpc_exec_ctx* exec_ctx, channel_data* chand = elem->channel_data; gpr_mu_init(&chand->max_age_timer_mu); chand->max_age_timer_pending = false; + chand->max_age_grace_timer_pending = false; chand->channel_stack = args->channel_stack; chand->max_connection_age = DEFAULT_MAX_CONNECTION_AGE_S == INT_MAX ? gpr_inf_future(GPR_TIMESPAN) : gpr_time_from_seconds(DEFAULT_MAX_CONNECTION_AGE_S, GPR_TIMESPAN); + chand->max_connection_age = + DEFAULT_MAX_CONNECTION_AGE_GRACE_S == INT_MAX + ? gpr_inf_future(GPR_TIMESPAN) + : gpr_time_from_seconds(DEFAULT_MAX_CONNECTION_AGE_GRACE_S, + GPR_TIMESPAN); for (size_t i = 0; i < args->channel_args->num_args; ++i) { if (0 == strcmp(args->channel_args->args[i].key, GPRC_ARG_MAX_CONNECION_AGE_S)) { @@ -129,10 +185,28 @@ static grpc_error* init_channel_elem(grpc_exec_ctx* exec_ctx, chand->max_connection_age = value == INT_MAX ? gpr_inf_future(GPR_TIMESPAN) : gpr_time_from_seconds(value, GPR_TIMESPAN); + } else if (0 == strcmp(args->channel_args->args[i].key, + GPRC_ARG_MAX_CONNECION_AGE_GRACE_S)) { + const int value = grpc_channel_arg_get_integer( + &args->channel_args->args[i], + (grpc_integer_options){DEFAULT_MAX_CONNECTION_AGE_GRACE_S, 1, + INT_MAX}); + chand->max_connection_age_grace = + value == INT_MAX ? gpr_inf_future(GPR_TIMESPAN) + : gpr_time_from_seconds(value, GPR_TIMESPAN); } } grpc_closure_init(&chand->close_max_age_channel, close_max_age_channel, chand, grpc_schedule_on_exec_ctx); + grpc_closure_init(&chand->force_close_max_age_channel, + force_close_max_age_channel, chand, + grpc_schedule_on_exec_ctx); + grpc_closure_init(&chand->start_max_age_timer_after_init, + start_max_age_timer_after_init, chand, + grpc_schedule_on_exec_ctx); + grpc_closure_init(&chand->start_max_age_grace_timer_after_goaway_op, + start_max_age_grace_timer_after_goaway_op, chand, + grpc_schedule_on_exec_ctx); if (gpr_time_cmp(chand->max_connection_age, gpr_inf_future(GPR_TIMESPAN)) != 0) { @@ -144,10 +218,8 @@ static grpc_error* init_channel_elem(grpc_exec_ctx* exec_ctx, // timer, and we schedule that closure to be run after call stack // initialization is done. GRPC_CHANNEL_STACK_REF(chand->channel_stack, - "max_age start_timer_after_init"); - grpc_closure_init(&chand->start_timer_after_init, start_timer_after_init, - chand, grpc_schedule_on_exec_ctx); - grpc_closure_sched(exec_ctx, &chand->start_timer_after_init, + "max_age start_max_age_timer_after_init"); + grpc_closure_sched(exec_ctx, &chand->start_max_age_timer_after_init, GRPC_ERROR_NONE); } @@ -162,6 +234,9 @@ static void destroy_channel_elem(grpc_exec_ctx* exec_ctx, if (chand->max_age_timer_pending) { grpc_timer_cancel(exec_ctx, &chand->max_age_timer); } + if (chand->max_age_grace_timer_pending) { + grpc_timer_cancel(exec_ctx, &chand->max_age_grace_timer); + } gpr_mu_unlock(&chand->max_age_timer_mu); } From 367428ad580a107de57545ba9cf5ab53223bc103 Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Tue, 28 Mar 2017 03:44:47 -0700 Subject: [PATCH 04/20] Add max_connection_idle enforcement --- include/grpc/impl/codegen/grpc_types.h | 3 + src/core/lib/channel/max_age_filter.c | 98 ++++++++++++++++++++++++-- 2 files changed, 94 insertions(+), 7 deletions(-) diff --git a/include/grpc/impl/codegen/grpc_types.h b/include/grpc/impl/codegen/grpc_types.h index 28db2db0b48..c01babece16 100644 --- a/include/grpc/impl/codegen/grpc_types.h +++ b/include/grpc/impl/codegen/grpc_types.h @@ -163,6 +163,9 @@ typedef struct { /** Maximum message length that the channel can send. Int valued, bytes. -1 means unlimited. */ #define GRPC_ARG_MAX_SEND_MESSAGE_LENGTH "grpc.max_send_message_length" +/** Maximum time that a channel may have no outstanding rpcs. Int valued, + seconds. INT_MAX means unlimited. */ +#define GPRC_ARG_MAX_CONNECION_IDLE_S "grpc.max_connection_idle" /** Maximum time that a channel may exist. Int valued, seconds. INT_MAX means unlimited. */ #define GPRC_ARG_MAX_CONNECION_AGE_S "grpc.max_connection_age" diff --git a/src/core/lib/channel/max_age_filter.c b/src/core/lib/channel/max_age_filter.c index 364e01705d2..928bb96138d 100644 --- a/src/core/lib/channel/max_age_filter.c +++ b/src/core/lib/channel/max_age_filter.c @@ -42,37 +42,76 @@ #define DEFAULT_MAX_CONNECTION_AGE_S INT_MAX #define DEFAULT_MAX_CONNECTION_AGE_GRACE_S INT_MAX +#define DEFAULT_MAX_CONNECTION_IDLE_S INT_MAX typedef struct channel_data { // We take a reference to the channel stack for the timer callback grpc_channel_stack* channel_stack; - // Guards access to max_age_timer and max_age_timer_pending + // Guards access to max_age_timer, max_age_timer_pending, max_age_grace_timer + // and max_age_grace_timer_pending gpr_mu max_age_timer_mu; // True if the max_age timer callback is currently pending bool max_age_timer_pending; - // True if the max_age timer callback is currently pending + // True if the max_age_grace timer callback is currently pending bool max_age_grace_timer_pending; // The timer for checking if the channel has reached its max age grpc_timer max_age_timer; - // The timer for checking if the channel has reached its max age + // The timer for checking if the max-aged channel has uesed up the grace + // period grpc_timer max_age_grace_timer; + // The timer for checking if the channel's idle duration reaches + // max_connection_idle + grpc_timer max_idle_timer; + // Allowed max time a channel may have no outstanding rpcs + gpr_timespec max_connection_idle; // Allowed max time a channel may exist gpr_timespec max_connection_age; // Allowed grace period after the channel reaches its max age gpr_timespec max_connection_age_grace; + // Closure to run when the channel's idle duration reaches max_connection_idle + // and should be closed gracefully + grpc_closure close_max_idle_channel; // Closure to run when the channel reaches its max age and should be closed // gracefully grpc_closure close_max_age_channel; // Closure to run the channel uses up its max age grace time and should be // closed forcibly grpc_closure force_close_max_age_channel; + // Closure to run when the init fo channel stack is done and the max_idle + // timer should be started + grpc_closure start_max_idle_timer_after_init; // Closure to run when the init fo channel stack is done and the max_age timer // should be started grpc_closure start_max_age_timer_after_init; // Closure to run when the goaway op is finished and the max_age_timer grpc_closure start_max_age_grace_timer_after_goaway_op; + // Number of active calls + gpr_atm call_count; } channel_data; +static void increase_call_count(grpc_exec_ctx* exec_ctx, channel_data* chand) { + if (gpr_atm_full_fetch_add(&chand->call_count, 1) == 0) { + grpc_timer_cancel(exec_ctx, &chand->max_idle_timer); + } +} + +static void decrease_call_count(grpc_exec_ctx* exec_ctx, channel_data* chand) { + if (gpr_atm_full_fetch_add(&chand->call_count, -1) == 1) { + grpc_timer_init( + exec_ctx, &chand->max_idle_timer, + gpr_time_add(gpr_now(GPR_CLOCK_MONOTONIC), chand->max_connection_idle), + &chand->close_max_idle_channel, gpr_now(GPR_CLOCK_MONOTONIC)); + } +} + +static void start_max_idle_timer_after_init(grpc_exec_ctx* exec_ctx, void* arg, + grpc_error* error) { + channel_data* chand = arg; + decrease_call_count(exec_ctx, chand); + GRPC_CHANNEL_STACK_UNREF(exec_ctx, chand->channel_stack, + "max_age start_max_idle_timer_after_init"); +} + static void start_max_age_timer_after_init(grpc_exec_ctx* exec_ctx, void* arg, grpc_error* error) { channel_data* chand = arg; @@ -103,6 +142,23 @@ static void start_max_age_grace_timer_after_goaway_op(grpc_exec_ctx* exec_ctx, "max_age start_max_age_grace_timer_after_goaway_op"); } +static void close_max_idle_channel(grpc_exec_ctx* exec_ctx, void* arg, + grpc_error* error) { + channel_data* chand = arg; + gpr_atm_no_barrier_fetch_add(&chand->call_count, 1); + if (error == GRPC_ERROR_NONE) { + grpc_transport_op* op = grpc_make_transport_op(NULL); + op->goaway_error = + grpc_error_set_int(GRPC_ERROR_CREATE_FROM_STATIC_STRING("max_idle"), + GRPC_ERROR_INT_HTTP2_ERROR, GRPC_HTTP2_NO_ERROR); + grpc_channel_element* elem = + grpc_channel_stack_element(chand->channel_stack, 0); + elem->filter->start_transport_op(exec_ctx, elem, op); + } else if (error != GRPC_ERROR_CANCELLED) { + GRPC_LOG_IF_ERROR("close_max_idle_channel", error); + } +} + static void close_max_age_channel(grpc_exec_ctx* exec_ctx, void* arg, grpc_error* error) { channel_data* chand = arg; @@ -147,7 +203,8 @@ static void force_close_max_age_channel(grpc_exec_ctx* exec_ctx, void* arg, static grpc_error* init_call_elem(grpc_exec_ctx* exec_ctx, grpc_call_element* elem, const grpc_call_element_args* args) { - // call_num ++; + channel_data* chand = elem->channel_data; + increase_call_count(exec_ctx, chand); return GRPC_ERROR_NONE; } @@ -155,7 +212,8 @@ static grpc_error* init_call_elem(grpc_exec_ctx* exec_ctx, static void destroy_call_elem(grpc_exec_ctx* exec_ctx, grpc_call_element* elem, const grpc_call_final_info* final_info, grpc_closure* ignored) { - // call_num --; + channel_data* chand = elem->channel_data; + decrease_call_count(exec_ctx, chand); } // Constructor for channel_data. @@ -171,11 +229,15 @@ static grpc_error* init_channel_elem(grpc_exec_ctx* exec_ctx, DEFAULT_MAX_CONNECTION_AGE_S == INT_MAX ? gpr_inf_future(GPR_TIMESPAN) : gpr_time_from_seconds(DEFAULT_MAX_CONNECTION_AGE_S, GPR_TIMESPAN); - chand->max_connection_age = + chand->max_connection_age_grace = DEFAULT_MAX_CONNECTION_AGE_GRACE_S == INT_MAX ? gpr_inf_future(GPR_TIMESPAN) : gpr_time_from_seconds(DEFAULT_MAX_CONNECTION_AGE_GRACE_S, GPR_TIMESPAN); + chand->max_connection_idle = + DEFAULT_MAX_CONNECTION_IDLE_S == INT_MAX + ? gpr_inf_future(GPR_TIMESPAN) + : gpr_time_from_seconds(DEFAULT_MAX_CONNECTION_IDLE_S, GPR_TIMESPAN); for (size_t i = 0; i < args->channel_args->num_args; ++i) { if (0 == strcmp(args->channel_args->args[i].key, GPRC_ARG_MAX_CONNECION_AGE_S)) { @@ -189,18 +251,31 @@ static grpc_error* init_channel_elem(grpc_exec_ctx* exec_ctx, GPRC_ARG_MAX_CONNECION_AGE_GRACE_S)) { const int value = grpc_channel_arg_get_integer( &args->channel_args->args[i], - (grpc_integer_options){DEFAULT_MAX_CONNECTION_AGE_GRACE_S, 1, + (grpc_integer_options){DEFAULT_MAX_CONNECTION_AGE_GRACE_S, 0, INT_MAX}); chand->max_connection_age_grace = value == INT_MAX ? gpr_inf_future(GPR_TIMESPAN) : gpr_time_from_seconds(value, GPR_TIMESPAN); + } else if (0 == strcmp(args->channel_args->args[i].key, + GPRC_ARG_MAX_CONNECION_IDLE_S)) { + const int value = grpc_channel_arg_get_integer( + &args->channel_args->args[i], + (grpc_integer_options){DEFAULT_MAX_CONNECTION_IDLE_S, 1, INT_MAX}); + chand->max_connection_age_grace = + value == INT_MAX ? gpr_inf_future(GPR_TIMESPAN) + : gpr_time_from_seconds(value, GPR_TIMESPAN); } } + grpc_closure_init(&chand->close_max_idle_channel, close_max_idle_channel, + chand, grpc_schedule_on_exec_ctx); grpc_closure_init(&chand->close_max_age_channel, close_max_age_channel, chand, grpc_schedule_on_exec_ctx); grpc_closure_init(&chand->force_close_max_age_channel, force_close_max_age_channel, chand, grpc_schedule_on_exec_ctx); + grpc_closure_init(&chand->start_max_idle_timer_after_init, + start_max_idle_timer_after_init, chand, + grpc_schedule_on_exec_ctx); grpc_closure_init(&chand->start_max_age_timer_after_init, start_max_age_timer_after_init, chand, grpc_schedule_on_exec_ctx); @@ -223,6 +298,14 @@ static grpc_error* init_channel_elem(grpc_exec_ctx* exec_ctx, GRPC_ERROR_NONE); } + gpr_atm_rel_store(&chand->call_count, 1); + if (gpr_time_cmp(chand->max_connection_idle, gpr_inf_future(GPR_TIMESPAN)) != + 0) { + GRPC_CHANNEL_STACK_REF(chand->channel_stack, + "max_age start_max_idle_timer_after_init"); + grpc_closure_sched(exec_ctx, &chand->start_max_idle_timer_after_init, + GRPC_ERROR_NONE); + } return GRPC_ERROR_NONE; } @@ -238,6 +321,7 @@ static void destroy_channel_elem(grpc_exec_ctx* exec_ctx, grpc_timer_cancel(exec_ctx, &chand->max_age_grace_timer); } gpr_mu_unlock(&chand->max_age_timer_mu); + increase_call_count(exec_ctx, chand); } const grpc_channel_filter grpc_max_age_filter = { From d88168b85bc9a6f6ccddff6ecdd8327d1b601d96 Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Tue, 28 Mar 2017 14:13:37 -0700 Subject: [PATCH 05/20] add installation of scipy and numpy in linux perf worker init --- tools/gce/linux_performance_worker_init.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/gce/linux_performance_worker_init.sh b/tools/gce/linux_performance_worker_init.sh index 3380f3de3e5..63fb0d81c54 100755 --- a/tools/gce/linux_performance_worker_init.sh +++ b/tools/gce/linux_performance_worker_init.sh @@ -166,3 +166,5 @@ echo 4096 | sudo tee /proc/sys/kernel/perf_event_mlock_kb # on benchmarks git clone -v https://github.com/brendangregg/FlameGraph ~/FlameGraph +# Install scipy and numpy for benchmarking scripts +sudo apt-get install python-scipy python-numpy From 22321fc7e58cd097d930a4c90fac656b98787c9d Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Tue, 28 Mar 2017 19:10:09 -0700 Subject: [PATCH 06/20] Add max_connection_age end2end test --- CMakeLists.txt | 2 + Makefile | 2 + include/grpc/impl/codegen/grpc_types.h | 6 +- src/core/lib/channel/max_age_filter.c | 10 +- test/core/end2end/end2end_nosec_tests.c | 8 + test/core/end2end/end2end_tests.c | 8 + test/core/end2end/gen_build_yaml.py | 1 + test/core/end2end/generate_tests.bzl | 1 + test/core/end2end/tests/max_connection_age.c | 359 ++++++++ .../generated/sources_and_headers.json | 2 + tools/run_tests/generated/tests.json | 858 ++++++++++++++++-- .../end2end_nosec_tests.vcxproj | 2 + .../end2end_nosec_tests.vcxproj.filters | 3 + .../tests/end2end_tests/end2end_tests.vcxproj | 2 + .../end2end_tests.vcxproj.filters | 3 + 15 files changed, 1191 insertions(+), 76 deletions(-) create mode 100644 test/core/end2end/tests/max_connection_age.c diff --git a/CMakeLists.txt b/CMakeLists.txt index 61863a230cc..25b9b8550c0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -3892,6 +3892,7 @@ add_library(end2end_tests test/core/end2end/tests/large_metadata.c test/core/end2end/tests/load_reporting_hook.c test/core/end2end/tests/max_concurrent_streams.c + test/core/end2end/tests/max_connection_age.c test/core/end2end/tests/max_message_length.c test/core/end2end/tests/negative_deadline.c test/core/end2end/tests/network_status_change.c @@ -3982,6 +3983,7 @@ add_library(end2end_nosec_tests test/core/end2end/tests/large_metadata.c test/core/end2end/tests/load_reporting_hook.c test/core/end2end/tests/max_concurrent_streams.c + test/core/end2end/tests/max_connection_age.c test/core/end2end/tests/max_message_length.c test/core/end2end/tests/negative_deadline.c test/core/end2end/tests/network_status_change.c diff --git a/Makefile b/Makefile index 2884134f40e..8aac6cbbbdf 100644 --- a/Makefile +++ b/Makefile @@ -7838,6 +7838,7 @@ LIBEND2END_TESTS_SRC = \ test/core/end2end/tests/large_metadata.c \ test/core/end2end/tests/load_reporting_hook.c \ test/core/end2end/tests/max_concurrent_streams.c \ + test/core/end2end/tests/max_connection_age.c \ test/core/end2end/tests/max_message_length.c \ test/core/end2end/tests/negative_deadline.c \ test/core/end2end/tests/network_status_change.c \ @@ -7927,6 +7928,7 @@ LIBEND2END_NOSEC_TESTS_SRC = \ test/core/end2end/tests/large_metadata.c \ test/core/end2end/tests/load_reporting_hook.c \ test/core/end2end/tests/max_concurrent_streams.c \ + test/core/end2end/tests/max_connection_age.c \ test/core/end2end/tests/max_message_length.c \ test/core/end2end/tests/negative_deadline.c \ test/core/end2end/tests/network_status_change.c \ diff --git a/include/grpc/impl/codegen/grpc_types.h b/include/grpc/impl/codegen/grpc_types.h index c01babece16..6839e36dcae 100644 --- a/include/grpc/impl/codegen/grpc_types.h +++ b/include/grpc/impl/codegen/grpc_types.h @@ -165,13 +165,13 @@ typedef struct { #define GRPC_ARG_MAX_SEND_MESSAGE_LENGTH "grpc.max_send_message_length" /** Maximum time that a channel may have no outstanding rpcs. Int valued, seconds. INT_MAX means unlimited. */ -#define GPRC_ARG_MAX_CONNECION_IDLE_S "grpc.max_connection_idle" +#define GRPC_ARG_MAX_CONNECTION_IDLE_S "grpc.max_connection_idle" /** Maximum time that a channel may exist. Int valued, seconds. INT_MAX means unlimited. */ -#define GPRC_ARG_MAX_CONNECION_AGE_S "grpc.max_connection_age" +#define GRPC_ARG_MAX_CONNECTION_AGE_S "grpc.max_connection_age" /** Grace period after the chennel reaches its max age. Int valued, seconds. INT_MAX means unlimited. */ -#define GPRC_ARG_MAX_CONNECION_AGE_GRACE_S "grpc.max_connection_age_grace" +#define GRPC_ARG_MAX_CONNECTION_AGE_GRACE_S "grpc.max_connection_age_grace" /** Initial sequence number for http2 transports. Int valued. */ #define GRPC_ARG_HTTP2_INITIAL_SEQUENCE_NUMBER \ "grpc.http2.initial_sequence_number" diff --git a/src/core/lib/channel/max_age_filter.c b/src/core/lib/channel/max_age_filter.c index 928bb96138d..3388b779c8a 100644 --- a/src/core/lib/channel/max_age_filter.c +++ b/src/core/lib/channel/max_age_filter.c @@ -166,6 +166,7 @@ static void close_max_age_channel(grpc_exec_ctx* exec_ctx, void* arg, chand->max_age_timer_pending = false; gpr_mu_unlock(&chand->max_age_timer_mu); if (error == GRPC_ERROR_NONE) { + gpr_log(GPR_DEBUG, "close_max_age_channel"); GRPC_CHANNEL_STACK_REF(chand->channel_stack, "max_age start_max_age_grace_timer_after_goaway_op"); grpc_transport_op* op = grpc_make_transport_op( @@ -188,6 +189,7 @@ static void force_close_max_age_channel(grpc_exec_ctx* exec_ctx, void* arg, chand->max_age_grace_timer_pending = false; gpr_mu_unlock(&chand->max_age_timer_mu); if (error == GRPC_ERROR_NONE) { + gpr_log(GPR_DEBUG, "force_close_max_age_channel"); grpc_transport_op* op = grpc_make_transport_op(NULL); op->disconnect_with_error = GRPC_ERROR_CREATE_FROM_STATIC_STRING("Channel reaches max age"); @@ -239,8 +241,8 @@ static grpc_error* init_channel_elem(grpc_exec_ctx* exec_ctx, ? gpr_inf_future(GPR_TIMESPAN) : gpr_time_from_seconds(DEFAULT_MAX_CONNECTION_IDLE_S, GPR_TIMESPAN); for (size_t i = 0; i < args->channel_args->num_args; ++i) { - if (0 == - strcmp(args->channel_args->args[i].key, GPRC_ARG_MAX_CONNECION_AGE_S)) { + if (0 == strcmp(args->channel_args->args[i].key, + GRPC_ARG_MAX_CONNECTION_AGE_S)) { const int value = grpc_channel_arg_get_integer( &args->channel_args->args[i], (grpc_integer_options){DEFAULT_MAX_CONNECTION_AGE_S, 1, INT_MAX}); @@ -248,7 +250,7 @@ static grpc_error* init_channel_elem(grpc_exec_ctx* exec_ctx, value == INT_MAX ? gpr_inf_future(GPR_TIMESPAN) : gpr_time_from_seconds(value, GPR_TIMESPAN); } else if (0 == strcmp(args->channel_args->args[i].key, - GPRC_ARG_MAX_CONNECION_AGE_GRACE_S)) { + GRPC_ARG_MAX_CONNECTION_AGE_GRACE_S)) { const int value = grpc_channel_arg_get_integer( &args->channel_args->args[i], (grpc_integer_options){DEFAULT_MAX_CONNECTION_AGE_GRACE_S, 0, @@ -257,7 +259,7 @@ static grpc_error* init_channel_elem(grpc_exec_ctx* exec_ctx, value == INT_MAX ? gpr_inf_future(GPR_TIMESPAN) : gpr_time_from_seconds(value, GPR_TIMESPAN); } else if (0 == strcmp(args->channel_args->args[i].key, - GPRC_ARG_MAX_CONNECION_IDLE_S)) { + GRPC_ARG_MAX_CONNECTION_IDLE_S)) { const int value = grpc_channel_arg_get_integer( &args->channel_args->args[i], (grpc_integer_options){DEFAULT_MAX_CONNECTION_IDLE_S, 1, INT_MAX}); diff --git a/test/core/end2end/end2end_nosec_tests.c b/test/core/end2end/end2end_nosec_tests.c index b351bdee277..70206a4cfa8 100644 --- a/test/core/end2end/end2end_nosec_tests.c +++ b/test/core/end2end/end2end_nosec_tests.c @@ -97,6 +97,8 @@ extern void load_reporting_hook(grpc_end2end_test_config config); extern void load_reporting_hook_pre_init(void); extern void max_concurrent_streams(grpc_end2end_test_config config); extern void max_concurrent_streams_pre_init(void); +extern void max_connection_age(grpc_end2end_test_config config); +extern void max_connection_age_pre_init(void); extern void max_message_length(grpc_end2end_test_config config); extern void max_message_length_pre_init(void); extern void negative_deadline(grpc_end2end_test_config config); @@ -174,6 +176,7 @@ void grpc_end2end_tests_pre_init(void) { large_metadata_pre_init(); load_reporting_hook_pre_init(); max_concurrent_streams_pre_init(); + max_connection_age_pre_init(); max_message_length_pre_init(); negative_deadline_pre_init(); network_status_change_pre_init(); @@ -232,6 +235,7 @@ void grpc_end2end_tests(int argc, char **argv, large_metadata(config); load_reporting_hook(config); max_concurrent_streams(config); + max_connection_age(config); max_message_length(config); negative_deadline(config); network_status_change(config); @@ -363,6 +367,10 @@ void grpc_end2end_tests(int argc, char **argv, max_concurrent_streams(config); continue; } + if (0 == strcmp("max_connection_age", argv[i])) { + max_connection_age(config); + continue; + } if (0 == strcmp("max_message_length", argv[i])) { max_message_length(config); continue; diff --git a/test/core/end2end/end2end_tests.c b/test/core/end2end/end2end_tests.c index 199c09ec96a..57e9eabe886 100644 --- a/test/core/end2end/end2end_tests.c +++ b/test/core/end2end/end2end_tests.c @@ -99,6 +99,8 @@ extern void load_reporting_hook(grpc_end2end_test_config config); extern void load_reporting_hook_pre_init(void); extern void max_concurrent_streams(grpc_end2end_test_config config); extern void max_concurrent_streams_pre_init(void); +extern void max_connection_age(grpc_end2end_test_config config); +extern void max_connection_age_pre_init(void); extern void max_message_length(grpc_end2end_test_config config); extern void max_message_length_pre_init(void); extern void negative_deadline(grpc_end2end_test_config config); @@ -177,6 +179,7 @@ void grpc_end2end_tests_pre_init(void) { large_metadata_pre_init(); load_reporting_hook_pre_init(); max_concurrent_streams_pre_init(); + max_connection_age_pre_init(); max_message_length_pre_init(); negative_deadline_pre_init(); network_status_change_pre_init(); @@ -236,6 +239,7 @@ void grpc_end2end_tests(int argc, char **argv, large_metadata(config); load_reporting_hook(config); max_concurrent_streams(config); + max_connection_age(config); max_message_length(config); negative_deadline(config); network_status_change(config); @@ -371,6 +375,10 @@ void grpc_end2end_tests(int argc, char **argv, max_concurrent_streams(config); continue; } + if (0 == strcmp("max_connection_age", argv[i])) { + max_connection_age(config); + continue; + } if (0 == strcmp("max_message_length", argv[i])) { max_message_length(config); continue; diff --git a/test/core/end2end/gen_build_yaml.py b/test/core/end2end/gen_build_yaml.py index 0c749537e60..d40f852e491 100755 --- a/test/core/end2end/gen_build_yaml.py +++ b/test/core/end2end/gen_build_yaml.py @@ -122,6 +122,7 @@ END2END_TESTS = { 'keepalive_timeout': default_test_options._replace(proxyable=False), 'large_metadata': default_test_options, 'max_concurrent_streams': default_test_options._replace(proxyable=False), + 'max_connection_age': default_test_options, 'max_message_length': default_test_options, 'negative_deadline': default_test_options, 'network_status_change': default_test_options, diff --git a/test/core/end2end/generate_tests.bzl b/test/core/end2end/generate_tests.bzl index 431c6995ba0..530a889dd93 100755 --- a/test/core/end2end/generate_tests.bzl +++ b/test/core/end2end/generate_tests.bzl @@ -109,6 +109,7 @@ END2END_TESTS = { 'keepalive_timeout': test_options(proxyable=False), 'large_metadata': test_options(), 'max_concurrent_streams': test_options(proxyable=False), + 'max_connection_age': test_options(), 'max_message_length': test_options(), 'negative_deadline': test_options(), 'network_status_change': test_options(), diff --git a/test/core/end2end/tests/max_connection_age.c b/test/core/end2end/tests/max_connection_age.c new file mode 100644 index 00000000000..9b31752963a --- /dev/null +++ b/test/core/end2end/tests/max_connection_age.c @@ -0,0 +1,359 @@ +/* + * + * Copyright 2017, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#include "test/core/end2end/end2end_tests.h" + +#include +#include + +#include +#include +#include +#include +#include +#include + +#include "test/core/end2end/cq_verifier.h" + +#define MAX_CONNECTION_AGE 1 +#define MAX_CONNECTION_AGE_GRACE 2 + +static void *tag(intptr_t t) { return (void *)t; } + +static void drain_cq(grpc_completion_queue *cq) { + grpc_event ev; + do { + ev = grpc_completion_queue_next(cq, grpc_timeout_seconds_to_deadline(5), + NULL); + } while (ev.type != GRPC_QUEUE_SHUTDOWN); +} + +static void shutdown_server(grpc_end2end_test_fixture *f) { + if (!f->server) return; + grpc_server_destroy(f->server); + f->server = NULL; +} + +static void shutdown_client(grpc_end2end_test_fixture *f) { + if (!f->client) return; + grpc_channel_destroy(f->client); + f->client = NULL; +} + +static void end_test(grpc_end2end_test_fixture *f) { + shutdown_server(f); + shutdown_client(f); + + grpc_completion_queue_shutdown(f->cq); + drain_cq(f->cq); + grpc_completion_queue_destroy(f->cq); +} + +static void test_max_age_forcibly_close(grpc_end2end_test_config config) { + grpc_end2end_test_fixture f = config.create_fixture(NULL, NULL); + cq_verifier *cqv = cq_verifier_create(f.cq); + grpc_arg server_a[] = {{.type = GRPC_ARG_INTEGER, + .key = GRPC_ARG_MAX_CONNECTION_AGE_S, + .value.integer = MAX_CONNECTION_AGE}, + {.type = GRPC_ARG_INTEGER, + .key = GRPC_ARG_MAX_CONNECTION_AGE_GRACE_S, + .value.integer = MAX_CONNECTION_AGE_GRACE}}; + grpc_channel_args server_args = {.num_args = GPR_ARRAY_SIZE(server_a), + .args = server_a}; + + config.init_client(&f, NULL); + config.init_server(&f, &server_args); + + grpc_call *c; + grpc_call *s; + gpr_timespec deadline = grpc_timeout_seconds_to_deadline(10); + grpc_op ops[6]; + grpc_op *op; + grpc_metadata_array initial_metadata_recv; + grpc_metadata_array trailing_metadata_recv; + grpc_metadata_array request_metadata_recv; + grpc_call_details call_details; + grpc_status_code status; + grpc_call_error error; + grpc_slice details; + int was_cancelled = 2; + + c = grpc_channel_create_call( + f.client, NULL, GRPC_PROPAGATE_DEFAULTS, f.cq, + grpc_slice_from_static_string("/foo"), + get_host_override_slice("foo.test.google.fr:1234", config), deadline, + NULL); + GPR_ASSERT(c); + + grpc_metadata_array_init(&initial_metadata_recv); + grpc_metadata_array_init(&trailing_metadata_recv); + grpc_metadata_array_init(&request_metadata_recv); + grpc_call_details_init(&call_details); + + memset(ops, 0, sizeof(ops)); + op = ops; + op->op = GRPC_OP_SEND_INITIAL_METADATA; + op->data.send_initial_metadata.count = 0; + op->data.send_initial_metadata.metadata = NULL; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_SEND_CLOSE_FROM_CLIENT; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_RECV_INITIAL_METADATA; + op->data.recv_initial_metadata.recv_initial_metadata = &initial_metadata_recv; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_RECV_STATUS_ON_CLIENT; + op->data.recv_status_on_client.trailing_metadata = &trailing_metadata_recv; + op->data.recv_status_on_client.status = &status; + op->data.recv_status_on_client.status_details = &details; + op->flags = 0; + op->reserved = NULL; + op++; + error = grpc_call_start_batch(c, ops, (size_t)(op - ops), tag(1), NULL); + GPR_ASSERT(GRPC_CALL_OK == error); + + error = + grpc_server_request_call(f.server, &s, &call_details, + &request_metadata_recv, f.cq, f.cq, tag(101)); + GPR_ASSERT(GRPC_CALL_OK == error); + CQ_EXPECT_COMPLETION(cqv, tag(101), 1); + cq_verify(cqv); + + /* Wait for the channel to reach its max age */ + cq_verify_empty_timeout(cqv, MAX_CONNECTION_AGE + 1); + + /* After the channel reaches its max age, we still do nothing here. And wait + for it to use up its max age grace period. */ + CQ_EXPECT_COMPLETION(cqv, tag(1), 1); + cq_verify(cqv); + + memset(ops, 0, sizeof(ops)); + op = ops; + op->op = GRPC_OP_SEND_INITIAL_METADATA; + op->data.send_initial_metadata.count = 0; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_SEND_STATUS_FROM_SERVER; + op->data.send_status_from_server.trailing_metadata_count = 0; + op->data.send_status_from_server.status = GRPC_STATUS_UNIMPLEMENTED; + grpc_slice status_details = grpc_slice_from_static_string("xyz"); + op->data.send_status_from_server.status_details = &status_details; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_RECV_CLOSE_ON_SERVER; + op->data.recv_close_on_server.cancelled = &was_cancelled; + op->flags = 0; + op->reserved = NULL; + op++; + error = grpc_call_start_batch(s, ops, (size_t)(op - ops), tag(102), NULL); + GPR_ASSERT(GRPC_CALL_OK == error); + CQ_EXPECT_COMPLETION(cqv, tag(102), 1); + cq_verify(cqv); + + grpc_server_shutdown_and_notify(f.server, f.cq, tag(0xdead)); + CQ_EXPECT_COMPLETION(cqv, tag(0xdead), 1); + cq_verify(cqv); + + grpc_call_destroy(s); + + /* The connection should be closed immediately after the max age grace period, + the in-progress RPC should fail. */ + GPR_ASSERT(status == GRPC_STATUS_UNAVAILABLE); + char *details_str = grpc_slice_to_c_string(details); + gpr_log(GPR_DEBUG, "status: %d, details: %s", status, details_str); + gpr_free(details_str); + GPR_ASSERT(0 == grpc_slice_str_cmp(details, "Endpoint read failed")); + GPR_ASSERT(0 == grpc_slice_str_cmp(call_details.method, "/foo")); + validate_host_override_string("foo.test.google.fr:1234", call_details.host, + config); + GPR_ASSERT(was_cancelled == 1); + + grpc_slice_unref(details); + grpc_metadata_array_destroy(&initial_metadata_recv); + grpc_metadata_array_destroy(&trailing_metadata_recv); + grpc_metadata_array_destroy(&request_metadata_recv); + grpc_call_details_destroy(&call_details); + grpc_call_destroy(c); + cq_verifier_destroy(cqv); + end_test(&f); + config.tear_down_data(&f); +} + +static void test_max_age_gracefully_close(grpc_end2end_test_config config) { + grpc_end2end_test_fixture f = config.create_fixture(NULL, NULL); + cq_verifier *cqv = cq_verifier_create(f.cq); + grpc_arg server_a[] = {{.type = GRPC_ARG_INTEGER, + .key = GRPC_ARG_MAX_CONNECTION_AGE_S, + .value.integer = MAX_CONNECTION_AGE}, + {.type = GRPC_ARG_INTEGER, + .key = GRPC_ARG_MAX_CONNECTION_AGE_GRACE_S, + .value.integer = INT_MAX}}; + grpc_channel_args server_args = {.num_args = GPR_ARRAY_SIZE(server_a), + .args = server_a}; + + config.init_client(&f, NULL); + config.init_server(&f, &server_args); + + grpc_call *c; + grpc_call *s; + gpr_timespec deadline = grpc_timeout_seconds_to_deadline(10); + grpc_op ops[6]; + grpc_op *op; + grpc_metadata_array initial_metadata_recv; + grpc_metadata_array trailing_metadata_recv; + grpc_metadata_array request_metadata_recv; + grpc_call_details call_details; + grpc_status_code status; + grpc_call_error error; + grpc_slice details; + int was_cancelled = 2; + + c = grpc_channel_create_call( + f.client, NULL, GRPC_PROPAGATE_DEFAULTS, f.cq, + grpc_slice_from_static_string("/foo"), + get_host_override_slice("foo.test.google.fr:1234", config), deadline, + NULL); + GPR_ASSERT(c); + + grpc_metadata_array_init(&initial_metadata_recv); + grpc_metadata_array_init(&trailing_metadata_recv); + grpc_metadata_array_init(&request_metadata_recv); + grpc_call_details_init(&call_details); + + memset(ops, 0, sizeof(ops)); + op = ops; + op->op = GRPC_OP_SEND_INITIAL_METADATA; + op->data.send_initial_metadata.count = 0; + op->data.send_initial_metadata.metadata = NULL; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_SEND_CLOSE_FROM_CLIENT; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_RECV_INITIAL_METADATA; + op->data.recv_initial_metadata.recv_initial_metadata = &initial_metadata_recv; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_RECV_STATUS_ON_CLIENT; + op->data.recv_status_on_client.trailing_metadata = &trailing_metadata_recv; + op->data.recv_status_on_client.status = &status; + op->data.recv_status_on_client.status_details = &details; + op->flags = 0; + op->reserved = NULL; + op++; + error = grpc_call_start_batch(c, ops, (size_t)(op - ops), tag(1), NULL); + GPR_ASSERT(GRPC_CALL_OK == error); + + error = + grpc_server_request_call(f.server, &s, &call_details, + &request_metadata_recv, f.cq, f.cq, tag(101)); + GPR_ASSERT(GRPC_CALL_OK == error); + CQ_EXPECT_COMPLETION(cqv, tag(101), 1); + cq_verify(cqv); + + /* Wait for the channel to reach its max age */ + cq_verify_empty_timeout(cqv, MAX_CONNECTION_AGE + 1); + + /* The connection is shutting down gracefully. In-progress rpc should not be + closed, hence the completion queue should see nothing here. */ + cq_verify_empty_timeout(cqv, 2); + + memset(ops, 0, sizeof(ops)); + op = ops; + op->op = GRPC_OP_SEND_INITIAL_METADATA; + op->data.send_initial_metadata.count = 0; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_SEND_STATUS_FROM_SERVER; + op->data.send_status_from_server.trailing_metadata_count = 0; + op->data.send_status_from_server.status = GRPC_STATUS_UNIMPLEMENTED; + grpc_slice status_details = grpc_slice_from_static_string("xyz"); + op->data.send_status_from_server.status_details = &status_details; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_RECV_CLOSE_ON_SERVER; + op->data.recv_close_on_server.cancelled = &was_cancelled; + op->flags = 0; + op->reserved = NULL; + op++; + error = grpc_call_start_batch(s, ops, (size_t)(op - ops), tag(102), NULL); + GPR_ASSERT(GRPC_CALL_OK == error); + + CQ_EXPECT_COMPLETION(cqv, tag(102), 1); + CQ_EXPECT_COMPLETION(cqv, tag(1), 1); + cq_verify(cqv); + + grpc_server_shutdown_and_notify(f.server, f.cq, tag(0xdead)); + CQ_EXPECT_COMPLETION(cqv, tag(0xdead), 1); + cq_verify(cqv); + + grpc_call_destroy(s); + + /* The connection is closed gracefully with goaway, the rpc should still be + completed. */ + GPR_ASSERT(status == GRPC_STATUS_UNIMPLEMENTED); + GPR_ASSERT(0 == grpc_slice_str_cmp(details, "xyz")); + GPR_ASSERT(0 == grpc_slice_str_cmp(call_details.method, "/foo")); + validate_host_override_string("foo.test.google.fr:1234", call_details.host, + config); + GPR_ASSERT(was_cancelled == 1); + + grpc_slice_unref(details); + grpc_metadata_array_destroy(&initial_metadata_recv); + grpc_metadata_array_destroy(&trailing_metadata_recv); + grpc_metadata_array_destroy(&request_metadata_recv); + grpc_call_details_destroy(&call_details); + grpc_call_destroy(c); + cq_verifier_destroy(cqv); + end_test(&f); + config.tear_down_data(&f); +} + +void max_connection_age(grpc_end2end_test_config config) { + test_max_age_forcibly_close(config); + test_max_age_gracefully_close(config); +} + +void max_connection_age_pre_init(void) {} diff --git a/tools/run_tests/generated/sources_and_headers.json b/tools/run_tests/generated/sources_and_headers.json index 250a8788f99..2ec415d5bd1 100644 --- a/tools/run_tests/generated/sources_and_headers.json +++ b/tools/run_tests/generated/sources_and_headers.json @@ -7108,6 +7108,7 @@ "test/core/end2end/tests/large_metadata.c", "test/core/end2end/tests/load_reporting_hook.c", "test/core/end2end/tests/max_concurrent_streams.c", + "test/core/end2end/tests/max_connection_age.c", "test/core/end2end/tests/max_message_length.c", "test/core/end2end/tests/negative_deadline.c", "test/core/end2end/tests/network_status_change.c", @@ -7180,6 +7181,7 @@ "test/core/end2end/tests/large_metadata.c", "test/core/end2end/tests/load_reporting_hook.c", "test/core/end2end/tests/max_concurrent_streams.c", + "test/core/end2end/tests/max_connection_age.c", "test/core/end2end/tests/max_message_length.c", "test/core/end2end/tests/negative_deadline.c", "test/core/end2end/tests/network_status_change.c", diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index 6202346fc2f..36d1b801d22 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -6302,6 +6302,29 @@ "posix" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_census_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -7454,6 +7477,29 @@ "posix" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_compress_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -8579,6 +8625,28 @@ "posix" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_fakesec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -9637,6 +9705,29 @@ "posix" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_fd_test", + "platforms": [ + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -10743,6 +10834,29 @@ "posix" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_full_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -11785,6 +11899,25 @@ "linux" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "linux" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_full+pipe_test", + "platforms": [ + "linux" + ] + }, { "args": [ "max_message_length" @@ -12822,6 +12955,29 @@ "posix" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_full+trace_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -13976,6 +14132,30 @@ "posix" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_http_proxy_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -15151,6 +15331,29 @@ "posix" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_load_reporting_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -16330,7 +16533,7 @@ }, { "args": [ - "max_message_length" + "max_connection_age" ], "ci_platforms": [ "windows", @@ -16354,7 +16557,7 @@ }, { "args": [ - "negative_deadline" + "max_message_length" ], "ci_platforms": [ "windows", @@ -16378,7 +16581,7 @@ }, { "args": [ - "network_status_change" + "negative_deadline" ], "ci_platforms": [ "windows", @@ -16402,7 +16605,7 @@ }, { "args": [ - "no_logging" + "network_status_change" ], "ci_platforms": [ "windows", @@ -16426,7 +16629,7 @@ }, { "args": [ - "no_op" + "no_logging" ], "ci_platforms": [ "windows", @@ -16450,7 +16653,7 @@ }, { "args": [ - "payload" + "no_op" ], "ci_platforms": [ "windows", @@ -16474,7 +16677,7 @@ }, { "args": [ - "ping" + "payload" ], "ci_platforms": [ "windows", @@ -16498,7 +16701,7 @@ }, { "args": [ - "ping_pong_streaming" + "ping" ], "ci_platforms": [ "windows", @@ -16522,7 +16725,7 @@ }, { "args": [ - "registered_call" + "ping_pong_streaming" ], "ci_platforms": [ "windows", @@ -16546,31 +16749,7 @@ }, { "args": [ - "request_with_flags" - ], - "ci_platforms": [ - "windows", - "linux", - "posix" - ], - "cpu_cost": 0.1, - "exclude_configs": [], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_oauth2_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, - { - "args": [ - "request_with_payload" + "registered_call" ], "ci_platforms": [ "windows", @@ -16594,14 +16773,14 @@ }, { "args": [ - "resource_quota_server" + "request_with_flags" ], "ci_platforms": [ "windows", "linux", "posix" ], - "cpu_cost": 1.0, + "cpu_cost": 0.1, "exclude_configs": [], "exclude_iomgrs": [ "uv" @@ -16618,7 +16797,7 @@ }, { "args": [ - "server_finishes_request" + "request_with_payload" ], "ci_platforms": [ "windows", @@ -16642,7 +16821,7 @@ }, { "args": [ - "shutdown_finishes_calls" + "resource_quota_server" ], "ci_platforms": [ "windows", @@ -16666,7 +16845,7 @@ }, { "args": [ - "shutdown_finishes_tags" + "server_finishes_request" ], "ci_platforms": [ "windows", @@ -16690,7 +16869,7 @@ }, { "args": [ - "simple_cacheable_request" + "shutdown_finishes_calls" ], "ci_platforms": [ "windows", @@ -16714,7 +16893,7 @@ }, { "args": [ - "simple_delayed_request" + "shutdown_finishes_tags" ], "ci_platforms": [ "windows", @@ -16738,7 +16917,7 @@ }, { "args": [ - "simple_metadata" + "simple_cacheable_request" ], "ci_platforms": [ "windows", @@ -16762,7 +16941,7 @@ }, { "args": [ - "simple_request" + "simple_delayed_request" ], "ci_platforms": [ "windows", @@ -16786,7 +16965,55 @@ }, { "args": [ - "streaming_error_response" + "simple_metadata" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_oauth2_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, + { + "args": [ + "simple_request" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_oauth2_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, + { + "args": [ + "streaming_error_response" ], "ci_platforms": [ "windows", @@ -17408,6 +17635,30 @@ "posix" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_proxy_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -18464,6 +18715,30 @@ "posix" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_sockpair_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -19520,6 +19795,30 @@ "posix" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_sockpair+trace_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -20600,6 +20899,32 @@ "posix" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [ + "msan" + ], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_sockpair_1byte_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -21743,6 +22068,29 @@ "posix" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_ssl_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -22895,6 +23243,29 @@ "posix" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_ssl_cert_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -23952,6 +24323,30 @@ "posix" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_ssl_proxy_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -25030,6 +25425,29 @@ "posix" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_uds_test", + "platforms": [ + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -26161,30 +26579,7 @@ }, { "args": [ - "max_message_length" - ], - "ci_platforms": [ - "windows", - "linux", - "mac", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "language": "c", - "name": "h2_census_nosec_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, - { - "args": [ - "negative_deadline" + "max_connection_age" ], "ci_platforms": [ "windows", @@ -26207,7 +26602,53 @@ }, { "args": [ - "network_status_change" + "max_message_length" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_census_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, + { + "args": [ + "negative_deadline" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_census_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, + { + "args": [ + "network_status_change" ], "ci_platforms": [ "windows", @@ -27288,6 +27729,29 @@ "posix" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_compress_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -28346,6 +28810,29 @@ "posix" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_fd_nosec_test", + "platforms": [ + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -29429,6 +29916,29 @@ "posix" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_full_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -30452,6 +30962,25 @@ "linux" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "linux" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_full+pipe_nosec_test", + "platforms": [ + "linux" + ] + }, { "args": [ "max_message_length" @@ -31466,6 +31995,29 @@ "posix" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_full+trace_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -32596,6 +33148,30 @@ "posix" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_http_proxy_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -33748,6 +34324,29 @@ "posix" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_load_reporting_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -34781,6 +35380,30 @@ "posix" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_proxy_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -35813,6 +36436,30 @@ "posix" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_sockpair_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -36845,6 +37492,30 @@ "posix" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_sockpair+trace_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -37899,6 +38570,32 @@ "posix" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [ + "msan" + ], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_sockpair_1byte_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -38994,6 +39691,29 @@ "posix" ] }, + { + "args": [ + "max_connection_age" + ], + "ci_platforms": [ + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_uds_nosec_test", + "platforms": [ + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" diff --git a/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj b/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj index 08b3acd03cd..ec9c5c8dd57 100644 --- a/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj +++ b/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj @@ -207,6 +207,8 @@ + + diff --git a/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj.filters b/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj.filters index 3a8670c1fae..f3c86e13cf3 100644 --- a/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj.filters +++ b/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj.filters @@ -85,6 +85,9 @@ test\core\end2end\tests + + test\core\end2end\tests + test\core\end2end\tests diff --git a/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj b/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj index 96418c3ca54..0afb714a977 100644 --- a/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj +++ b/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj @@ -209,6 +209,8 @@ + + diff --git a/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj.filters b/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj.filters index cf40abef436..8c81e597c24 100644 --- a/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj.filters +++ b/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj.filters @@ -88,6 +88,9 @@ test\core\end2end\tests + + test\core\end2end\tests + test\core\end2end\tests From 7dc3629a7df53248c703e9a015604101b431e546 Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Wed, 29 Mar 2017 00:09:41 -0700 Subject: [PATCH 07/20] Fix use-after-free issue --- src/core/lib/channel/max_age_filter.c | 58 ++++++++++++++++---- test/core/end2end/tests/max_connection_age.c | 3 - 2 files changed, 46 insertions(+), 15 deletions(-) diff --git a/src/core/lib/channel/max_age_filter.c b/src/core/lib/channel/max_age_filter.c index 3388b779c8a..4eba057f7c1 100644 --- a/src/core/lib/channel/max_age_filter.c +++ b/src/core/lib/channel/max_age_filter.c @@ -85,6 +85,10 @@ typedef struct channel_data { grpc_closure start_max_age_timer_after_init; // Closure to run when the goaway op is finished and the max_age_timer grpc_closure start_max_age_grace_timer_after_goaway_op; + // Closure to run when the channel connectivity state changes + grpc_closure channel_connectivity_changed; + // Records the current connectivity state + grpc_connectivity_state connectivity_state; // Number of active calls gpr_atm call_count; } channel_data; @@ -97,6 +101,7 @@ static void increase_call_count(grpc_exec_ctx* exec_ctx, channel_data* chand) { static void decrease_call_count(grpc_exec_ctx* exec_ctx, channel_data* chand) { if (gpr_atm_full_fetch_add(&chand->call_count, -1) == 1) { + GRPC_CHANNEL_STACK_REF(chand->channel_stack, "max_age max_idle_timer"); grpc_timer_init( exec_ctx, &chand->max_idle_timer, gpr_time_add(gpr_now(GPR_CLOCK_MONOTONIC), chand->max_connection_idle), @@ -117,11 +122,17 @@ static void start_max_age_timer_after_init(grpc_exec_ctx* exec_ctx, void* arg, channel_data* chand = arg; gpr_mu_lock(&chand->max_age_timer_mu); chand->max_age_timer_pending = true; + GRPC_CHANNEL_STACK_REF(chand->channel_stack, "max_age max_age_timer"); grpc_timer_init( exec_ctx, &chand->max_age_timer, gpr_time_add(gpr_now(GPR_CLOCK_MONOTONIC), chand->max_connection_age), &chand->close_max_age_channel, gpr_now(GPR_CLOCK_MONOTONIC)); gpr_mu_unlock(&chand->max_age_timer_mu); + grpc_transport_op* op = grpc_make_transport_op(NULL); + op->on_connectivity_state_change = &chand->channel_connectivity_changed, + op->connectivity_state = &chand->connectivity_state; + grpc_channel_next_op(exec_ctx, + grpc_channel_stack_element(chand->channel_stack, 0), op); GRPC_CHANNEL_STACK_UNREF(exec_ctx, chand->channel_stack, "max_age start_max_age_timer_after_init"); } @@ -132,6 +143,7 @@ static void start_max_age_grace_timer_after_goaway_op(grpc_exec_ctx* exec_ctx, channel_data* chand = arg; gpr_mu_lock(&chand->max_age_timer_mu); chand->max_age_grace_timer_pending = true; + GRPC_CHANNEL_STACK_REF(chand->channel_stack, "max_age max_age_grace_timer"); grpc_timer_init(exec_ctx, &chand->max_age_grace_timer, gpr_time_add(gpr_now(GPR_CLOCK_MONOTONIC), chand->max_connection_age_grace), @@ -157,6 +169,8 @@ static void close_max_idle_channel(grpc_exec_ctx* exec_ctx, void* arg, } else if (error != GRPC_ERROR_CANCELLED) { GRPC_LOG_IF_ERROR("close_max_idle_channel", error); } + GRPC_CHANNEL_STACK_UNREF(exec_ctx, chand->channel_stack, + "max_age max_idle_timer"); } static void close_max_age_channel(grpc_exec_ctx* exec_ctx, void* arg, @@ -180,6 +194,8 @@ static void close_max_age_channel(grpc_exec_ctx* exec_ctx, void* arg, } else if (error != GRPC_ERROR_CANCELLED) { GRPC_LOG_IF_ERROR("close_max_age_channel", error); } + GRPC_CHANNEL_STACK_UNREF(exec_ctx, chand->channel_stack, + "max_age max_age_timer"); } static void force_close_max_age_channel(grpc_exec_ctx* exec_ctx, void* arg, @@ -199,6 +215,32 @@ static void force_close_max_age_channel(grpc_exec_ctx* exec_ctx, void* arg, } else if (error != GRPC_ERROR_CANCELLED) { GRPC_LOG_IF_ERROR("force_close_max_age_channel", error); } + GRPC_CHANNEL_STACK_UNREF(exec_ctx, chand->channel_stack, + "max_age max_age_grace_timer"); +} + +static void channel_connectivity_changed(grpc_exec_ctx* exec_ctx, void* arg, + grpc_error* error) { + channel_data* chand = arg; + if (chand->connectivity_state != GRPC_CHANNEL_SHUTDOWN) { + grpc_transport_op* op = grpc_make_transport_op(NULL); + op->on_connectivity_state_change = &chand->channel_connectivity_changed, + op->connectivity_state = &chand->connectivity_state; + grpc_channel_next_op( + exec_ctx, grpc_channel_stack_element(chand->channel_stack, 0), op); + } else { + gpr_mu_lock(&chand->max_age_timer_mu); + if (chand->max_age_timer_pending) { + grpc_timer_cancel(exec_ctx, &chand->max_age_timer); + chand->max_age_timer_pending = false; + } + if (chand->max_age_grace_timer_pending) { + grpc_timer_cancel(exec_ctx, &chand->max_age_grace_timer); + chand->max_age_grace_timer_pending = false; + } + gpr_mu_unlock(&chand->max_age_timer_mu); + increase_call_count(exec_ctx, chand); + } } // Constructor for call_data. @@ -284,6 +326,9 @@ static grpc_error* init_channel_elem(grpc_exec_ctx* exec_ctx, grpc_closure_init(&chand->start_max_age_grace_timer_after_goaway_op, start_max_age_grace_timer_after_goaway_op, chand, grpc_schedule_on_exec_ctx); + grpc_closure_init(&chand->channel_connectivity_changed, + channel_connectivity_changed, chand, + grpc_schedule_on_exec_ctx); if (gpr_time_cmp(chand->max_connection_age, gpr_inf_future(GPR_TIMESPAN)) != 0) { @@ -313,18 +358,7 @@ static grpc_error* init_channel_elem(grpc_exec_ctx* exec_ctx, // Destructor for channel_data. static void destroy_channel_elem(grpc_exec_ctx* exec_ctx, - grpc_channel_element* elem) { - channel_data* chand = elem->channel_data; - gpr_mu_lock(&chand->max_age_timer_mu); - if (chand->max_age_timer_pending) { - grpc_timer_cancel(exec_ctx, &chand->max_age_timer); - } - if (chand->max_age_grace_timer_pending) { - grpc_timer_cancel(exec_ctx, &chand->max_age_grace_timer); - } - gpr_mu_unlock(&chand->max_age_timer_mu); - increase_call_count(exec_ctx, chand); -} + grpc_channel_element* elem) {} const grpc_channel_filter grpc_max_age_filter = { grpc_call_next_op, diff --git a/test/core/end2end/tests/max_connection_age.c b/test/core/end2end/tests/max_connection_age.c index 9b31752963a..ee66d125833 100644 --- a/test/core/end2end/tests/max_connection_age.c +++ b/test/core/end2end/tests/max_connection_age.c @@ -196,9 +196,6 @@ static void test_max_age_forcibly_close(grpc_end2end_test_config config) { /* The connection should be closed immediately after the max age grace period, the in-progress RPC should fail. */ GPR_ASSERT(status == GRPC_STATUS_UNAVAILABLE); - char *details_str = grpc_slice_to_c_string(details); - gpr_log(GPR_DEBUG, "status: %d, details: %s", status, details_str); - gpr_free(details_str); GPR_ASSERT(0 == grpc_slice_str_cmp(details, "Endpoint read failed")); GPR_ASSERT(0 == grpc_slice_str_cmp(call_details.method, "/foo")); validate_host_override_string("foo.test.google.fr:1234", call_details.host, From b2caafc911af90de5f4f896f2f4daf567ab8da70 Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Wed, 29 Mar 2017 01:54:08 -0700 Subject: [PATCH 08/20] Add max_connection_idle test --- CMakeLists.txt | 2 + Makefile | 2 + src/core/lib/channel/max_age_filter.c | 2 +- test/core/end2end/end2end_nosec_tests.c | 8 + test/core/end2end/end2end_tests.c | 8 + test/core/end2end/gen_build_yaml.py | 1 + test/core/end2end/generate_tests.bzl | 1 + test/core/end2end/tests/max_connection_age.c | 23 +- test/core/end2end/tests/max_connection_idle.c | 119 +++ .../generated/sources_and_headers.json | 2 + tools/run_tests/generated/tests.json | 858 ++++++++++++++++-- .../end2end_nosec_tests.vcxproj | 2 + .../end2end_nosec_tests.vcxproj.filters | 3 + .../tests/end2end_tests/end2end_tests.vcxproj | 2 + .../end2end_tests.vcxproj.filters | 3 + 15 files changed, 958 insertions(+), 78 deletions(-) create mode 100644 test/core/end2end/tests/max_connection_idle.c diff --git a/CMakeLists.txt b/CMakeLists.txt index 25b9b8550c0..b3ea56b9c27 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -3893,6 +3893,7 @@ add_library(end2end_tests test/core/end2end/tests/load_reporting_hook.c test/core/end2end/tests/max_concurrent_streams.c test/core/end2end/tests/max_connection_age.c + test/core/end2end/tests/max_connection_idle.c test/core/end2end/tests/max_message_length.c test/core/end2end/tests/negative_deadline.c test/core/end2end/tests/network_status_change.c @@ -3984,6 +3985,7 @@ add_library(end2end_nosec_tests test/core/end2end/tests/load_reporting_hook.c test/core/end2end/tests/max_concurrent_streams.c test/core/end2end/tests/max_connection_age.c + test/core/end2end/tests/max_connection_idle.c test/core/end2end/tests/max_message_length.c test/core/end2end/tests/negative_deadline.c test/core/end2end/tests/network_status_change.c diff --git a/Makefile b/Makefile index 8aac6cbbbdf..1e1b989e4ea 100644 --- a/Makefile +++ b/Makefile @@ -7839,6 +7839,7 @@ LIBEND2END_TESTS_SRC = \ test/core/end2end/tests/load_reporting_hook.c \ test/core/end2end/tests/max_concurrent_streams.c \ test/core/end2end/tests/max_connection_age.c \ + test/core/end2end/tests/max_connection_idle.c \ test/core/end2end/tests/max_message_length.c \ test/core/end2end/tests/negative_deadline.c \ test/core/end2end/tests/network_status_change.c \ @@ -7929,6 +7930,7 @@ LIBEND2END_NOSEC_TESTS_SRC = \ test/core/end2end/tests/load_reporting_hook.c \ test/core/end2end/tests/max_concurrent_streams.c \ test/core/end2end/tests/max_connection_age.c \ + test/core/end2end/tests/max_connection_idle.c \ test/core/end2end/tests/max_message_length.c \ test/core/end2end/tests/negative_deadline.c \ test/core/end2end/tests/network_status_change.c \ diff --git a/src/core/lib/channel/max_age_filter.c b/src/core/lib/channel/max_age_filter.c index 4eba057f7c1..e6b70c44119 100644 --- a/src/core/lib/channel/max_age_filter.c +++ b/src/core/lib/channel/max_age_filter.c @@ -305,7 +305,7 @@ static grpc_error* init_channel_elem(grpc_exec_ctx* exec_ctx, const int value = grpc_channel_arg_get_integer( &args->channel_args->args[i], (grpc_integer_options){DEFAULT_MAX_CONNECTION_IDLE_S, 1, INT_MAX}); - chand->max_connection_age_grace = + chand->max_connection_idle = value == INT_MAX ? gpr_inf_future(GPR_TIMESPAN) : gpr_time_from_seconds(value, GPR_TIMESPAN); } diff --git a/test/core/end2end/end2end_nosec_tests.c b/test/core/end2end/end2end_nosec_tests.c index 70206a4cfa8..64bdceb2111 100644 --- a/test/core/end2end/end2end_nosec_tests.c +++ b/test/core/end2end/end2end_nosec_tests.c @@ -99,6 +99,8 @@ extern void max_concurrent_streams(grpc_end2end_test_config config); extern void max_concurrent_streams_pre_init(void); extern void max_connection_age(grpc_end2end_test_config config); extern void max_connection_age_pre_init(void); +extern void max_connection_idle(grpc_end2end_test_config config); +extern void max_connection_idle_pre_init(void); extern void max_message_length(grpc_end2end_test_config config); extern void max_message_length_pre_init(void); extern void negative_deadline(grpc_end2end_test_config config); @@ -177,6 +179,7 @@ void grpc_end2end_tests_pre_init(void) { load_reporting_hook_pre_init(); max_concurrent_streams_pre_init(); max_connection_age_pre_init(); + max_connection_idle_pre_init(); max_message_length_pre_init(); negative_deadline_pre_init(); network_status_change_pre_init(); @@ -236,6 +239,7 @@ void grpc_end2end_tests(int argc, char **argv, load_reporting_hook(config); max_concurrent_streams(config); max_connection_age(config); + max_connection_idle(config); max_message_length(config); negative_deadline(config); network_status_change(config); @@ -371,6 +375,10 @@ void grpc_end2end_tests(int argc, char **argv, max_connection_age(config); continue; } + if (0 == strcmp("max_connection_idle", argv[i])) { + max_connection_idle(config); + continue; + } if (0 == strcmp("max_message_length", argv[i])) { max_message_length(config); continue; diff --git a/test/core/end2end/end2end_tests.c b/test/core/end2end/end2end_tests.c index 57e9eabe886..37c1be41337 100644 --- a/test/core/end2end/end2end_tests.c +++ b/test/core/end2end/end2end_tests.c @@ -101,6 +101,8 @@ extern void max_concurrent_streams(grpc_end2end_test_config config); extern void max_concurrent_streams_pre_init(void); extern void max_connection_age(grpc_end2end_test_config config); extern void max_connection_age_pre_init(void); +extern void max_connection_idle(grpc_end2end_test_config config); +extern void max_connection_idle_pre_init(void); extern void max_message_length(grpc_end2end_test_config config); extern void max_message_length_pre_init(void); extern void negative_deadline(grpc_end2end_test_config config); @@ -180,6 +182,7 @@ void grpc_end2end_tests_pre_init(void) { load_reporting_hook_pre_init(); max_concurrent_streams_pre_init(); max_connection_age_pre_init(); + max_connection_idle_pre_init(); max_message_length_pre_init(); negative_deadline_pre_init(); network_status_change_pre_init(); @@ -240,6 +243,7 @@ void grpc_end2end_tests(int argc, char **argv, load_reporting_hook(config); max_concurrent_streams(config); max_connection_age(config); + max_connection_idle(config); max_message_length(config); negative_deadline(config); network_status_change(config); @@ -379,6 +383,10 @@ void grpc_end2end_tests(int argc, char **argv, max_connection_age(config); continue; } + if (0 == strcmp("max_connection_idle", argv[i])) { + max_connection_idle(config); + continue; + } if (0 == strcmp("max_message_length", argv[i])) { max_message_length(config); continue; diff --git a/test/core/end2end/gen_build_yaml.py b/test/core/end2end/gen_build_yaml.py index d40f852e491..518316e3e08 100755 --- a/test/core/end2end/gen_build_yaml.py +++ b/test/core/end2end/gen_build_yaml.py @@ -123,6 +123,7 @@ END2END_TESTS = { 'large_metadata': default_test_options, 'max_concurrent_streams': default_test_options._replace(proxyable=False), 'max_connection_age': default_test_options, + 'max_connection_idle': default_test_options, 'max_message_length': default_test_options, 'negative_deadline': default_test_options, 'network_status_change': default_test_options, diff --git a/test/core/end2end/generate_tests.bzl b/test/core/end2end/generate_tests.bzl index 530a889dd93..eb5e319c013 100755 --- a/test/core/end2end/generate_tests.bzl +++ b/test/core/end2end/generate_tests.bzl @@ -110,6 +110,7 @@ END2END_TESTS = { 'large_metadata': test_options(), 'max_concurrent_streams': test_options(proxyable=False), 'max_connection_age': test_options(), + 'max_connection_idle': test_options(), 'max_message_length': test_options(), 'negative_deadline': test_options(), 'network_status_change': test_options(), diff --git a/test/core/end2end/tests/max_connection_age.c b/test/core/end2end/tests/max_connection_age.c index ee66d125833..8cd6ff0f87b 100644 --- a/test/core/end2end/tests/max_connection_age.c +++ b/test/core/end2end/tests/max_connection_age.c @@ -45,8 +45,9 @@ #include "test/core/end2end/cq_verifier.h" -#define MAX_CONNECTION_AGE 1 -#define MAX_CONNECTION_AGE_GRACE 2 +#define MAX_CONNECTION_AGE_S 1 +#define MAX_CONNECTION_AGE_GRACE_S 2 +#define MAX_CONNECTION_IDLE_S 99 static void *tag(intptr_t t) { return (void *)t; } @@ -84,10 +85,13 @@ static void test_max_age_forcibly_close(grpc_end2end_test_config config) { cq_verifier *cqv = cq_verifier_create(f.cq); grpc_arg server_a[] = {{.type = GRPC_ARG_INTEGER, .key = GRPC_ARG_MAX_CONNECTION_AGE_S, - .value.integer = MAX_CONNECTION_AGE}, + .value.integer = MAX_CONNECTION_AGE_S}, {.type = GRPC_ARG_INTEGER, .key = GRPC_ARG_MAX_CONNECTION_AGE_GRACE_S, - .value.integer = MAX_CONNECTION_AGE_GRACE}}; + .value.integer = MAX_CONNECTION_AGE_GRACE_S}, + {.type = GRPC_ARG_INTEGER, + .key = GRPC_ARG_MAX_CONNECTION_IDLE_S, + .value.integer = MAX_CONNECTION_IDLE_S}}; grpc_channel_args server_args = {.num_args = GPR_ARRAY_SIZE(server_a), .args = server_a}; @@ -155,7 +159,7 @@ static void test_max_age_forcibly_close(grpc_end2end_test_config config) { cq_verify(cqv); /* Wait for the channel to reach its max age */ - cq_verify_empty_timeout(cqv, MAX_CONNECTION_AGE + 1); + cq_verify_empty_timeout(cqv, MAX_CONNECTION_AGE_S + 1); /* After the channel reaches its max age, we still do nothing here. And wait for it to use up its max age grace period. */ @@ -218,10 +222,13 @@ static void test_max_age_gracefully_close(grpc_end2end_test_config config) { cq_verifier *cqv = cq_verifier_create(f.cq); grpc_arg server_a[] = {{.type = GRPC_ARG_INTEGER, .key = GRPC_ARG_MAX_CONNECTION_AGE_S, - .value.integer = MAX_CONNECTION_AGE}, + .value.integer = MAX_CONNECTION_AGE_S}, {.type = GRPC_ARG_INTEGER, .key = GRPC_ARG_MAX_CONNECTION_AGE_GRACE_S, - .value.integer = INT_MAX}}; + .value.integer = INT_MAX}, + {.type = GRPC_ARG_INTEGER, + .key = GRPC_ARG_MAX_CONNECTION_IDLE_S, + .value.integer = MAX_CONNECTION_IDLE_S}}; grpc_channel_args server_args = {.num_args = GPR_ARRAY_SIZE(server_a), .args = server_a}; @@ -289,7 +296,7 @@ static void test_max_age_gracefully_close(grpc_end2end_test_config config) { cq_verify(cqv); /* Wait for the channel to reach its max age */ - cq_verify_empty_timeout(cqv, MAX_CONNECTION_AGE + 1); + cq_verify_empty_timeout(cqv, MAX_CONNECTION_AGE_S + 1); /* The connection is shutting down gracefully. In-progress rpc should not be closed, hence the completion queue should see nothing here. */ diff --git a/test/core/end2end/tests/max_connection_idle.c b/test/core/end2end/tests/max_connection_idle.c new file mode 100644 index 00000000000..2a3b98491bd --- /dev/null +++ b/test/core/end2end/tests/max_connection_idle.c @@ -0,0 +1,119 @@ +/* + * + * Copyright 2017, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#include "test/core/end2end/end2end_tests.h" + +#include +#include + +#include +#include +#include +#include +#include +#include + +#include "test/core/end2end/cq_verifier.h" + +#define MAX_CONNECTION_IDLE_S 1 +#define MAX_CONNECTION_AGE_S 99 + +static void *tag(intptr_t t) { return (void *)t; } + +static void test_max_connection_idle(grpc_end2end_test_config config) { + grpc_end2end_test_fixture f = config.create_fixture(NULL, NULL); + grpc_connectivity_state state = GRPC_CHANNEL_IDLE; + cq_verifier *cqv = cq_verifier_create(f.cq); + + grpc_arg client_a[] = {{.type = GRPC_ARG_INTEGER, + .key = "grpc.testing.fixed_reconnect_backoff_ms", + .value.integer = 1000}}; + grpc_arg server_a[] = {{.type = GRPC_ARG_INTEGER, + .key = GRPC_ARG_MAX_CONNECTION_IDLE_S, + .value.integer = MAX_CONNECTION_IDLE_S}, + {.type = GRPC_ARG_INTEGER, + .key = GRPC_ARG_MAX_CONNECTION_AGE_S, + .value.integer = MAX_CONNECTION_AGE_S}}; + grpc_channel_args client_args = {.num_args = GPR_ARRAY_SIZE(client_a), + .args = client_a}; + grpc_channel_args server_args = {.num_args = GPR_ARRAY_SIZE(server_a), + .args = server_a}; + + config.init_client(&f, &client_args); + config.init_server(&f, &server_args); + + /* check that we're still in idle, and start connecting */ + GPR_ASSERT(grpc_channel_check_connectivity_state(f.client, 1) == + GRPC_CHANNEL_IDLE); + /* we'll go through some set of transitions (some might be missed), until + READY is reached */ + while (state != GRPC_CHANNEL_READY) { + grpc_channel_watch_connectivity_state( + f.client, state, grpc_timeout_seconds_to_deadline(3), f.cq, tag(99)); + CQ_EXPECT_COMPLETION(cqv, tag(99), 1); + cq_verify(cqv); + state = grpc_channel_check_connectivity_state(f.client, 0); + GPR_ASSERT(state == GRPC_CHANNEL_READY || + state == GRPC_CHANNEL_CONNECTING || + state == GRPC_CHANNEL_TRANSIENT_FAILURE); + } + + /* wait for the channel to reach its maximum idle time */ + grpc_channel_watch_connectivity_state( + f.client, GRPC_CHANNEL_READY, + grpc_timeout_seconds_to_deadline(MAX_CONNECTION_IDLE_S + 1), f.cq, + tag(99)); + CQ_EXPECT_COMPLETION(cqv, tag(99), 1); + cq_verify(cqv); + state = grpc_channel_check_connectivity_state(f.client, 0); + GPR_ASSERT(state == GRPC_CHANNEL_TRANSIENT_FAILURE || + state == GRPC_CHANNEL_CONNECTING || state == GRPC_CHANNEL_IDLE); + + grpc_server_shutdown_and_notify(f.server, f.cq, tag(0xdead)); + CQ_EXPECT_COMPLETION(cqv, tag(0xdead), 1); + cq_verify(cqv); + + grpc_server_destroy(f.server); + grpc_channel_destroy(f.client); + grpc_completion_queue_shutdown(f.cq); + grpc_completion_queue_destroy(f.cq); + config.tear_down_data(&f); + + cq_verifier_destroy(cqv); +} + +void max_connection_idle(grpc_end2end_test_config config) { + test_max_connection_idle(config); +} + +void max_connection_idle_pre_init(void) {} diff --git a/tools/run_tests/generated/sources_and_headers.json b/tools/run_tests/generated/sources_and_headers.json index 2ec415d5bd1..71aee1b05d4 100644 --- a/tools/run_tests/generated/sources_and_headers.json +++ b/tools/run_tests/generated/sources_and_headers.json @@ -7109,6 +7109,7 @@ "test/core/end2end/tests/load_reporting_hook.c", "test/core/end2end/tests/max_concurrent_streams.c", "test/core/end2end/tests/max_connection_age.c", + "test/core/end2end/tests/max_connection_idle.c", "test/core/end2end/tests/max_message_length.c", "test/core/end2end/tests/negative_deadline.c", "test/core/end2end/tests/network_status_change.c", @@ -7182,6 +7183,7 @@ "test/core/end2end/tests/load_reporting_hook.c", "test/core/end2end/tests/max_concurrent_streams.c", "test/core/end2end/tests/max_connection_age.c", + "test/core/end2end/tests/max_connection_idle.c", "test/core/end2end/tests/max_message_length.c", "test/core/end2end/tests/negative_deadline.c", "test/core/end2end/tests/network_status_change.c", diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index 36d1b801d22..53ec8e8d007 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -6325,6 +6325,29 @@ "posix" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_census_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -7500,6 +7523,29 @@ "posix" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_compress_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -8647,6 +8693,28 @@ "posix" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_fakesec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -9728,6 +9796,29 @@ "posix" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_fd_test", + "platforms": [ + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -10857,6 +10948,29 @@ "posix" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_full_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -11918,6 +12032,25 @@ "linux" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "linux" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_full+pipe_test", + "platforms": [ + "linux" + ] + }, { "args": [ "max_message_length" @@ -12978,6 +13111,29 @@ "posix" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_full+trace_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -14156,6 +14312,30 @@ "posix" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_http_proxy_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -15354,6 +15534,29 @@ "posix" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_load_reporting_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -16557,7 +16760,7 @@ }, { "args": [ - "max_message_length" + "max_connection_idle" ], "ci_platforms": [ "windows", @@ -16581,7 +16784,7 @@ }, { "args": [ - "negative_deadline" + "max_message_length" ], "ci_platforms": [ "windows", @@ -16605,7 +16808,7 @@ }, { "args": [ - "network_status_change" + "negative_deadline" ], "ci_platforms": [ "windows", @@ -16629,7 +16832,7 @@ }, { "args": [ - "no_logging" + "network_status_change" ], "ci_platforms": [ "windows", @@ -16653,7 +16856,7 @@ }, { "args": [ - "no_op" + "no_logging" ], "ci_platforms": [ "windows", @@ -16677,7 +16880,7 @@ }, { "args": [ - "payload" + "no_op" ], "ci_platforms": [ "windows", @@ -16701,7 +16904,7 @@ }, { "args": [ - "ping" + "payload" ], "ci_platforms": [ "windows", @@ -16725,7 +16928,7 @@ }, { "args": [ - "ping_pong_streaming" + "ping" ], "ci_platforms": [ "windows", @@ -16749,7 +16952,7 @@ }, { "args": [ - "registered_call" + "ping_pong_streaming" ], "ci_platforms": [ "windows", @@ -16773,31 +16976,7 @@ }, { "args": [ - "request_with_flags" - ], - "ci_platforms": [ - "windows", - "linux", - "posix" - ], - "cpu_cost": 0.1, - "exclude_configs": [], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_oauth2_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, - { - "args": [ - "request_with_payload" + "registered_call" ], "ci_platforms": [ "windows", @@ -16821,14 +17000,14 @@ }, { "args": [ - "resource_quota_server" + "request_with_flags" ], "ci_platforms": [ "windows", "linux", "posix" ], - "cpu_cost": 1.0, + "cpu_cost": 0.1, "exclude_configs": [], "exclude_iomgrs": [ "uv" @@ -16845,7 +17024,7 @@ }, { "args": [ - "server_finishes_request" + "request_with_payload" ], "ci_platforms": [ "windows", @@ -16869,7 +17048,7 @@ }, { "args": [ - "shutdown_finishes_calls" + "resource_quota_server" ], "ci_platforms": [ "windows", @@ -16893,7 +17072,7 @@ }, { "args": [ - "shutdown_finishes_tags" + "server_finishes_request" ], "ci_platforms": [ "windows", @@ -16917,7 +17096,7 @@ }, { "args": [ - "simple_cacheable_request" + "shutdown_finishes_calls" ], "ci_platforms": [ "windows", @@ -16941,7 +17120,7 @@ }, { "args": [ - "simple_delayed_request" + "shutdown_finishes_tags" ], "ci_platforms": [ "windows", @@ -16965,7 +17144,7 @@ }, { "args": [ - "simple_metadata" + "simple_cacheable_request" ], "ci_platforms": [ "windows", @@ -16989,7 +17168,7 @@ }, { "args": [ - "simple_request" + "simple_delayed_request" ], "ci_platforms": [ "windows", @@ -17013,7 +17192,55 @@ }, { "args": [ - "streaming_error_response" + "simple_metadata" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_oauth2_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, + { + "args": [ + "simple_request" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_oauth2_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, + { + "args": [ + "streaming_error_response" ], "ci_platforms": [ "windows", @@ -17659,6 +17886,30 @@ "posix" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_proxy_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -18739,6 +18990,30 @@ "posix" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_sockpair_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -19819,6 +20094,30 @@ "posix" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_sockpair+trace_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -20925,6 +21224,32 @@ "posix" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [ + "msan" + ], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_sockpair_1byte_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -22091,6 +22416,29 @@ "posix" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_ssl_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -23266,6 +23614,29 @@ "posix" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_ssl_cert_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -24347,6 +24718,30 @@ "posix" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_ssl_proxy_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -25448,6 +25843,29 @@ "posix" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_uds_test", + "platforms": [ + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -26602,30 +27020,7 @@ }, { "args": [ - "max_message_length" - ], - "ci_platforms": [ - "windows", - "linux", - "mac", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [], - "flaky": false, - "language": "c", - "name": "h2_census_nosec_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, - { - "args": [ - "negative_deadline" + "max_connection_idle" ], "ci_platforms": [ "windows", @@ -26648,7 +27043,53 @@ }, { "args": [ - "network_status_change" + "max_message_length" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_census_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, + { + "args": [ + "negative_deadline" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_census_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, + { + "args": [ + "network_status_change" ], "ci_platforms": [ "windows", @@ -27752,6 +28193,29 @@ "posix" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_compress_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -28833,6 +29297,29 @@ "posix" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_fd_nosec_test", + "platforms": [ + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -29939,6 +30426,29 @@ "posix" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_full_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -30981,6 +31491,25 @@ "linux" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "linux" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_full+pipe_nosec_test", + "platforms": [ + "linux" + ] + }, { "args": [ "max_message_length" @@ -32018,6 +32547,29 @@ "posix" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_full+trace_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -33172,6 +33724,30 @@ "posix" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_http_proxy_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -34347,6 +34923,29 @@ "posix" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [], + "flaky": false, + "language": "c", + "name": "h2_load_reporting_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -35404,6 +36003,30 @@ "posix" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_proxy_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -36460,6 +37083,30 @@ "posix" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_sockpair_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -37516,6 +38163,30 @@ "posix" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_sockpair+trace_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -38596,6 +39267,32 @@ "posix" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [ + "msan" + ], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_sockpair_1byte_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" @@ -39714,6 +40411,29 @@ "posix" ] }, + { + "args": [ + "max_connection_idle" + ], + "ci_platforms": [ + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "exclude_iomgrs": [ + "uv" + ], + "flaky": false, + "language": "c", + "name": "h2_uds_nosec_test", + "platforms": [ + "linux", + "mac", + "posix" + ] + }, { "args": [ "max_message_length" diff --git a/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj b/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj index ec9c5c8dd57..5a2d6acd563 100644 --- a/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj +++ b/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj @@ -209,6 +209,8 @@ + + diff --git a/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj.filters b/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj.filters index f3c86e13cf3..3a870b945e6 100644 --- a/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj.filters +++ b/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj.filters @@ -88,6 +88,9 @@ test\core\end2end\tests + + test\core\end2end\tests + test\core\end2end\tests diff --git a/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj b/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj index 0afb714a977..4b001a751d8 100644 --- a/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj +++ b/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj @@ -211,6 +211,8 @@ + + diff --git a/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj.filters b/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj.filters index 8c81e597c24..2eace64a893 100644 --- a/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj.filters +++ b/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj.filters @@ -91,6 +91,9 @@ test\core\end2end\tests + + test\core\end2end\tests + test\core\end2end\tests From b9d9512d8a50a82c0203a041be717ac6b26b6851 Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Fri, 17 Mar 2017 10:50:10 -0700 Subject: [PATCH 09/20] Doc perf tests --- tools/README.md | 3 ++ tools/run_tests/performance/README.md | 70 +++++++++++++++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 tools/run_tests/performance/README.md diff --git a/tools/README.md b/tools/README.md index d051846c33e..62e91246d0a 100644 --- a/tools/README.md +++ b/tools/README.md @@ -16,3 +16,6 @@ internal_ci: Support for running tests on an internal CI platform. jenkins: Support for running tests on Jenkins. run_tests: Scripts to run gRPC tests in parallel. + +run_tests/performance: See the [README](./run_tests/performance/README.md) for +more notes on the performance tests. diff --git a/tools/run_tests/performance/README.md b/tools/run_tests/performance/README.md new file mode 100644 index 00000000000..af69f39d3c1 --- /dev/null +++ b/tools/run_tests/performance/README.md @@ -0,0 +1,70 @@ +# Overview of performance test suite, with steps for manual runs: + +For design of the tests, see +http://www.grpc.io/docs/guides/benchmarking.html. + +## Pre-reqs for running these manually: +In general the benchmark workers and driver build scripts expect +[linux_performance_worker_init.sh](../../gce/linux_performance_worker_init.sh) to have been ran already. + +### To run benchmarks locally: +* From the grpc repo root, start the +[run_performance_tests.py](../run_performance_tests.py) runner script. + +### On remote machines, to start the driver and workers manually: +The [run_performance_test.py](../run_performance_tests.py) top-level runner script can also +be used with remote machines, but for e.g., profiling the server, +it might be useful to run workers manually. + +1. You'll need a "driver" and separate "worker" machines. +For example, you might use one GCE "driver" machine and 3 other +GCE "worker" machines that are in the same zone. + +2. Connect to each worker machine and start up a benchmark worker with a "driver_port". + * For example, to start the grpc-go benchmark worker: + [grpc-go worker main.go](https://github.com/grpc/grpc-go/blob/master/benchmark/worker/main.go) --driver_port + +#### Build the driver: +* Connect to the driver machine (if using a remote driver) and from the grpc repo root: +``` +$ tools/run_tests/performance/build_performance.sh +``` + +#### Run the driver: +1. Get the 'scenario_json' relevant for the scenario to run. Note that "scenario + json" configs are generated from [scenario_config.py](./scenario_config.py). + The [driver](../../../test/cpp/qps/qps_json_driver.cc) takes a list of these configs as a json string of the form: `{scenario: }` + in its `--scenarios_json` command argument. + One quick way to get a valid json string to pass to the driver is by running + the [run_performance_tests.py](./run_performance_tests.py) locally and copying the logged scenario json command arg. + +2. From the grpc repo root: + +* Set `QPS_WORKERS` environment variable to a commaa separated list of worker +machines. Note that the driver will start the "benchmark server" on the first +entry in the list, and the rest will be told to run as clients against the +benchmark server. + +Example running and profiling of go benchmark server: +``` +$ export QPS_WORKERS=:<10000>,,10000,:10000 +$ bins/opt/qps_json_driver +--scenario_json='' +``` + +### Example profiling commands + +While running the benchmark, a profiler can be attached to the server. + +Example to count syscalls in grpc-go server during a benchmark: +* Connect to server machine and run: +``` +$ netstat -tulpn | grep # to get pid of worker +$ perf stat -p -e syscalls:sys_enter_write # stop after test complete +``` + +Example memory profile of grpc-go server, with `go tools pprof`: +* After a run is done on the server, see its alloc profile with: +``` +$ go tool pprof --text --alloc_space http://localhost:/debug/heap +``` From 3835d1378809a4658d9d5192982e1437a406623c Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Wed, 29 Mar 2017 11:38:58 -0700 Subject: [PATCH 10/20] Fix the test condition for max_connection_idle test --- test/core/end2end/gen_build_yaml.py | 3 +- test/core/end2end/generate_tests.bzl | 2 +- tools/run_tests/generated/tests.json | 318 ++++----------------------- 3 files changed, 42 insertions(+), 281 deletions(-) diff --git a/test/core/end2end/gen_build_yaml.py b/test/core/end2end/gen_build_yaml.py index 518316e3e08..3c5068ff3e3 100755 --- a/test/core/end2end/gen_build_yaml.py +++ b/test/core/end2end/gen_build_yaml.py @@ -123,7 +123,8 @@ END2END_TESTS = { 'large_metadata': default_test_options, 'max_concurrent_streams': default_test_options._replace(proxyable=False), 'max_connection_age': default_test_options, - 'max_connection_idle': default_test_options, + 'max_connection_idle': connectivity_test_options._replace( + proxyable=False, exclude_iomgrs=['uv']), 'max_message_length': default_test_options, 'negative_deadline': default_test_options, 'network_status_change': default_test_options, diff --git a/test/core/end2end/generate_tests.bzl b/test/core/end2end/generate_tests.bzl index eb5e319c013..1041219f039 100755 --- a/test/core/end2end/generate_tests.bzl +++ b/test/core/end2end/generate_tests.bzl @@ -110,7 +110,7 @@ END2END_TESTS = { 'large_metadata': test_options(), 'max_concurrent_streams': test_options(proxyable=False), 'max_connection_age': test_options(), - 'max_connection_idle': test_options(), + 'max_connection_idle': test_options(needs_fullstack=True, proxyable=False), 'max_message_length': test_options(), 'negative_deadline': test_options(), 'network_status_change': test_options(), diff --git a/tools/run_tests/generated/tests.json b/tools/run_tests/generated/tests.json index 53ec8e8d007..0279010d041 100644 --- a/tools/run_tests/generated/tests.json +++ b/tools/run_tests/generated/tests.json @@ -6337,7 +6337,9 @@ ], "cpu_cost": 1.0, "exclude_configs": [], - "exclude_iomgrs": [], + "exclude_iomgrs": [ + "uv" + ], "flaky": false, "language": "c", "name": "h2_census_test", @@ -7535,7 +7537,9 @@ ], "cpu_cost": 1.0, "exclude_configs": [], - "exclude_iomgrs": [], + "exclude_iomgrs": [ + "uv" + ], "flaky": false, "language": "c", "name": "h2_compress_test", @@ -8704,7 +8708,9 @@ ], "cpu_cost": 1.0, "exclude_configs": [], - "exclude_iomgrs": [], + "exclude_iomgrs": [ + "uv" + ], "flaky": false, "language": "c", "name": "h2_fakesec_test", @@ -9796,29 +9802,6 @@ "posix" ] }, - { - "args": [ - "max_connection_idle" - ], - "ci_platforms": [ - "linux", - "mac", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_fd_test", - "platforms": [ - "linux", - "mac", - "posix" - ] - }, { "args": [ "max_message_length" @@ -10960,7 +10943,9 @@ ], "cpu_cost": 1.0, "exclude_configs": [], - "exclude_iomgrs": [], + "exclude_iomgrs": [ + "uv" + ], "flaky": false, "language": "c", "name": "h2_full_test", @@ -13123,7 +13108,9 @@ ], "cpu_cost": 1.0, "exclude_configs": [], - "exclude_iomgrs": [], + "exclude_iomgrs": [ + "uv" + ], "flaky": false, "language": "c", "name": "h2_full+trace_test", @@ -15546,7 +15533,9 @@ ], "cpu_cost": 1.0, "exclude_configs": [], - "exclude_iomgrs": [], + "exclude_iomgrs": [ + "uv" + ], "flaky": false, "language": "c", "name": "h2_load_reporting_test", @@ -17886,30 +17875,6 @@ "posix" ] }, - { - "args": [ - "max_connection_idle" - ], - "ci_platforms": [ - "windows", - "linux", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_proxy_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "max_message_length" @@ -18990,30 +18955,6 @@ "posix" ] }, - { - "args": [ - "max_connection_idle" - ], - "ci_platforms": [ - "windows", - "linux", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_sockpair_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "max_message_length" @@ -20094,30 +20035,6 @@ "posix" ] }, - { - "args": [ - "max_connection_idle" - ], - "ci_platforms": [ - "windows", - "linux", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_sockpair+trace_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "max_message_length" @@ -21224,32 +21141,6 @@ "posix" ] }, - { - "args": [ - "max_connection_idle" - ], - "ci_platforms": [ - "windows", - "linux", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_sockpair_1byte_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "max_message_length" @@ -22428,7 +22319,9 @@ ], "cpu_cost": 1.0, "exclude_configs": [], - "exclude_iomgrs": [], + "exclude_iomgrs": [ + "uv" + ], "flaky": false, "language": "c", "name": "h2_ssl_test", @@ -23626,7 +23519,9 @@ ], "cpu_cost": 1.0, "exclude_configs": [], - "exclude_iomgrs": [], + "exclude_iomgrs": [ + "uv" + ], "flaky": false, "language": "c", "name": "h2_ssl_cert_test", @@ -24718,30 +24613,6 @@ "posix" ] }, - { - "args": [ - "max_connection_idle" - ], - "ci_platforms": [ - "windows", - "linux", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_ssl_proxy_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "max_message_length" @@ -27030,7 +26901,9 @@ ], "cpu_cost": 1.0, "exclude_configs": [], - "exclude_iomgrs": [], + "exclude_iomgrs": [ + "uv" + ], "flaky": false, "language": "c", "name": "h2_census_nosec_test", @@ -28205,7 +28078,9 @@ ], "cpu_cost": 1.0, "exclude_configs": [], - "exclude_iomgrs": [], + "exclude_iomgrs": [ + "uv" + ], "flaky": false, "language": "c", "name": "h2_compress_nosec_test", @@ -29297,29 +29172,6 @@ "posix" ] }, - { - "args": [ - "max_connection_idle" - ], - "ci_platforms": [ - "linux", - "mac", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_fd_nosec_test", - "platforms": [ - "linux", - "mac", - "posix" - ] - }, { "args": [ "max_message_length" @@ -30438,7 +30290,9 @@ ], "cpu_cost": 1.0, "exclude_configs": [], - "exclude_iomgrs": [], + "exclude_iomgrs": [ + "uv" + ], "flaky": false, "language": "c", "name": "h2_full_nosec_test", @@ -32559,7 +32413,9 @@ ], "cpu_cost": 1.0, "exclude_configs": [], - "exclude_iomgrs": [], + "exclude_iomgrs": [ + "uv" + ], "flaky": false, "language": "c", "name": "h2_full+trace_nosec_test", @@ -34935,7 +34791,9 @@ ], "cpu_cost": 1.0, "exclude_configs": [], - "exclude_iomgrs": [], + "exclude_iomgrs": [ + "uv" + ], "flaky": false, "language": "c", "name": "h2_load_reporting_nosec_test", @@ -36003,30 +35861,6 @@ "posix" ] }, - { - "args": [ - "max_connection_idle" - ], - "ci_platforms": [ - "windows", - "linux", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_proxy_nosec_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "max_message_length" @@ -37083,30 +36917,6 @@ "posix" ] }, - { - "args": [ - "max_connection_idle" - ], - "ci_platforms": [ - "windows", - "linux", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_sockpair_nosec_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "max_message_length" @@ -38163,30 +37973,6 @@ "posix" ] }, - { - "args": [ - "max_connection_idle" - ], - "ci_platforms": [ - "windows", - "linux", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_sockpair+trace_nosec_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "max_message_length" @@ -39267,32 +39053,6 @@ "posix" ] }, - { - "args": [ - "max_connection_idle" - ], - "ci_platforms": [ - "windows", - "linux", - "posix" - ], - "cpu_cost": 1.0, - "exclude_configs": [ - "msan" - ], - "exclude_iomgrs": [ - "uv" - ], - "flaky": false, - "language": "c", - "name": "h2_sockpair_1byte_nosec_test", - "platforms": [ - "windows", - "linux", - "mac", - "posix" - ] - }, { "args": [ "max_message_length" From ca6dab39d65ea189f77a44adc4afcbbdf21f71f3 Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Wed, 29 Mar 2017 12:12:33 -0700 Subject: [PATCH 11/20] Change the unit to ms, clean up --- include/grpc/impl/codegen/grpc_types.h | 16 +- src/core/lib/channel/max_age_filter.c | 193 +++++++++--------- test/core/end2end/tests/max_connection_age.c | 34 ++- test/core/end2end/tests/max_connection_idle.c | 16 +- 4 files changed, 133 insertions(+), 126 deletions(-) diff --git a/include/grpc/impl/codegen/grpc_types.h b/include/grpc/impl/codegen/grpc_types.h index 6839e36dcae..421204841fb 100644 --- a/include/grpc/impl/codegen/grpc_types.h +++ b/include/grpc/impl/codegen/grpc_types.h @@ -164,14 +164,14 @@ typedef struct { -1 means unlimited. */ #define GRPC_ARG_MAX_SEND_MESSAGE_LENGTH "grpc.max_send_message_length" /** Maximum time that a channel may have no outstanding rpcs. Int valued, - seconds. INT_MAX means unlimited. */ -#define GRPC_ARG_MAX_CONNECTION_IDLE_S "grpc.max_connection_idle" -/** Maximum time that a channel may exist. Int valued, seconds. INT_MAX means - unlimited. */ -#define GRPC_ARG_MAX_CONNECTION_AGE_S "grpc.max_connection_age" -/** Grace period after the chennel reaches its max age. Int valued, seconds. - INT_MAX means unlimited. */ -#define GRPC_ARG_MAX_CONNECTION_AGE_GRACE_S "grpc.max_connection_age_grace" + milliseconds. INT_MAX means unlimited. */ +#define GRPC_ARG_MAX_CONNECTION_IDLE_MS "grpc.max_connection_idle_ms" +/** Maximum time that a channel may exist. Int valued, milliseconds. INT_MAX + means unlimited. */ +#define GRPC_ARG_MAX_CONNECTION_AGE_MS "grpc.max_connection_age_ms" +/** Grace period after the chennel reaches its max age. Int valued, + milliseconds. INT_MAX means unlimited. */ +#define GRPC_ARG_MAX_CONNECTION_AGE_GRACE_MS "grpc.max_connection_age_grace_ms" /** Initial sequence number for http2 transports. Int valued. */ #define GRPC_ARG_HTTP2_INITIAL_SEQUENCE_NUMBER \ "grpc.http2.initial_sequence_number" diff --git a/src/core/lib/channel/max_age_filter.c b/src/core/lib/channel/max_age_filter.c index e6b70c44119..c25481486c3 100644 --- a/src/core/lib/channel/max_age_filter.c +++ b/src/core/lib/channel/max_age_filter.c @@ -1,104 +1,109 @@ -// -// Copyright 2017, Google Inc. -// All rights reserved. -// -// Redistribution and use in source and binary forms, with or without -// modification, are permitted provided that the following conditions are -// met: -// -// * Redistributions of source code must retain the above copyright -// notice, this list of conditions and the following disclaimer. -// * Redistributions in binary form must reproduce the above -// copyright notice, this list of conditions and the following disclaimer -// in the documentation and/or other materials provided with the -// distribution. -// * Neither the name of Google Inc. nor the names of its -// contributors may be used to endorse or promote products derived from -// this software without specific prior written permission. -// -// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS -// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT -// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR -// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT -// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, -// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT -// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, -// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY -// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT -// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE -// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. -// +/* + * + * Copyright 2017, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ #include "src/core/lib/channel/message_size_filter.h" #include #include -#include #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/iomgr/timer.h" #include "src/core/lib/transport/http2_errors.h" #include "src/core/lib/transport/service_config.h" -#define DEFAULT_MAX_CONNECTION_AGE_S INT_MAX -#define DEFAULT_MAX_CONNECTION_AGE_GRACE_S INT_MAX -#define DEFAULT_MAX_CONNECTION_IDLE_S INT_MAX +#define DEFAULT_MAX_CONNECTION_AGE_MS INT_MAX +#define DEFAULT_MAX_CONNECTION_AGE_GRACE_MS INT_MAX +#define DEFAULT_MAX_CONNECTION_IDLE_MS INT_MAX typedef struct channel_data { - // We take a reference to the channel stack for the timer callback + /* We take a reference to the channel stack for the timer callback */ grpc_channel_stack* channel_stack; - // Guards access to max_age_timer, max_age_timer_pending, max_age_grace_timer - // and max_age_grace_timer_pending + /* Guards access to max_age_timer, max_age_timer_pending, max_age_grace_timer + and max_age_grace_timer_pending */ gpr_mu max_age_timer_mu; - // True if the max_age timer callback is currently pending + /* True if the max_age timer callback is currently pending */ bool max_age_timer_pending; - // True if the max_age_grace timer callback is currently pending + /* True if the max_age_grace timer callback is currently pending */ bool max_age_grace_timer_pending; - // The timer for checking if the channel has reached its max age + /* The timer for checking if the channel has reached its max age */ grpc_timer max_age_timer; - // The timer for checking if the max-aged channel has uesed up the grace - // period + /* The timer for checking if the max-aged channel has uesed up the grace + period */ grpc_timer max_age_grace_timer; - // The timer for checking if the channel's idle duration reaches - // max_connection_idle + /* The timer for checking if the channel's idle duration reaches + max_connection_idle */ grpc_timer max_idle_timer; - // Allowed max time a channel may have no outstanding rpcs + /* Allowed max time a channel may have no outstanding rpcs */ gpr_timespec max_connection_idle; - // Allowed max time a channel may exist + /* Allowed max time a channel may exist */ gpr_timespec max_connection_age; - // Allowed grace period after the channel reaches its max age + /* Allowed grace period after the channel reaches its max age */ gpr_timespec max_connection_age_grace; - // Closure to run when the channel's idle duration reaches max_connection_idle - // and should be closed gracefully + /* Closure to run when the channel's idle duration reaches max_connection_idle + and should be closed gracefully */ grpc_closure close_max_idle_channel; - // Closure to run when the channel reaches its max age and should be closed - // gracefully + /* Closure to run when the channel reaches its max age and should be closed + gracefully */ grpc_closure close_max_age_channel; - // Closure to run the channel uses up its max age grace time and should be - // closed forcibly + /* Closure to run the channel uses up its max age grace time and should be + closed forcibly */ grpc_closure force_close_max_age_channel; - // Closure to run when the init fo channel stack is done and the max_idle - // timer should be started + /* Closure to run when the init fo channel stack is done and the max_idle + timer should be started */ grpc_closure start_max_idle_timer_after_init; - // Closure to run when the init fo channel stack is done and the max_age timer - // should be started + /* Closure to run when the init fo channel stack is done and the max_age timer + should be started */ grpc_closure start_max_age_timer_after_init; - // Closure to run when the goaway op is finished and the max_age_timer + /* Closure to run when the goaway op is finished and the max_age_timer */ grpc_closure start_max_age_grace_timer_after_goaway_op; - // Closure to run when the channel connectivity state changes + /* Closure to run when the channel connectivity state changes */ grpc_closure channel_connectivity_changed; - // Records the current connectivity state + /* Records the current connectivity state */ grpc_connectivity_state connectivity_state; - // Number of active calls + /* Number of active calls */ gpr_atm call_count; } channel_data; +/* Increase the nubmer of active calls. Before the increasement, if there are no + calls, the max_idle_timer should be cancelled. */ static void increase_call_count(grpc_exec_ctx* exec_ctx, channel_data* chand) { if (gpr_atm_full_fetch_add(&chand->call_count, 1) == 0) { grpc_timer_cancel(exec_ctx, &chand->max_idle_timer); } } +/* Decrease the nubmer of active calls. After the decrement, if there are no + calls, the max_idle_timer should be started. */ static void decrease_call_count(grpc_exec_ctx* exec_ctx, channel_data* chand) { if (gpr_atm_full_fetch_add(&chand->call_count, -1) == 1) { GRPC_CHANNEL_STACK_REF(chand->channel_stack, "max_age max_idle_timer"); @@ -112,6 +117,9 @@ static void decrease_call_count(grpc_exec_ctx* exec_ctx, channel_data* chand) { static void start_max_idle_timer_after_init(grpc_exec_ctx* exec_ctx, void* arg, grpc_error* error) { channel_data* chand = arg; + /* Decrease call_count. If there are no active calls at this time, + max_idle_timer will start here. If the number of active calls is not 0, + max_idle_timer will start after all the active calls end. */ decrease_call_count(exec_ctx, chand); GRPC_CHANNEL_STACK_UNREF(exec_ctx, chand->channel_stack, "max_age start_max_idle_timer_after_init"); @@ -180,7 +188,6 @@ static void close_max_age_channel(grpc_exec_ctx* exec_ctx, void* arg, chand->max_age_timer_pending = false; gpr_mu_unlock(&chand->max_age_timer_mu); if (error == GRPC_ERROR_NONE) { - gpr_log(GPR_DEBUG, "close_max_age_channel"); GRPC_CHANNEL_STACK_REF(chand->channel_stack, "max_age start_max_age_grace_timer_after_goaway_op"); grpc_transport_op* op = grpc_make_transport_op( @@ -205,7 +212,6 @@ static void force_close_max_age_channel(grpc_exec_ctx* exec_ctx, void* arg, chand->max_age_grace_timer_pending = false; gpr_mu_unlock(&chand->max_age_timer_mu); if (error == GRPC_ERROR_NONE) { - gpr_log(GPR_DEBUG, "force_close_max_age_channel"); grpc_transport_op* op = grpc_make_transport_op(NULL); op->disconnect_with_error = GRPC_ERROR_CREATE_FROM_STATIC_STRING("Channel reaches max age"); @@ -239,11 +245,14 @@ static void channel_connectivity_changed(grpc_exec_ctx* exec_ctx, void* arg, chand->max_age_grace_timer_pending = false; } gpr_mu_unlock(&chand->max_age_timer_mu); + /* If there are no active calls, this increasement will cancel + max_idle_timer, and prevent max_idle_timer from being started in the + future. */ increase_call_count(exec_ctx, chand); } } -// Constructor for call_data. +/* Constructor for call_data. */ static grpc_error* init_call_elem(grpc_exec_ctx* exec_ctx, grpc_call_element* elem, const grpc_call_element_args* args) { @@ -252,7 +261,7 @@ static grpc_error* init_call_elem(grpc_exec_ctx* exec_ctx, return GRPC_ERROR_NONE; } -// Destructor for call_data. +/* Destructor for call_data. */ static void destroy_call_elem(grpc_exec_ctx* exec_ctx, grpc_call_element* elem, const grpc_call_final_info* final_info, grpc_closure* ignored) { @@ -260,7 +269,7 @@ static void destroy_call_elem(grpc_exec_ctx* exec_ctx, grpc_call_element* elem, decrease_call_count(exec_ctx, chand); } -// Constructor for channel_data. +/* Constructor for channel_data. */ static grpc_error* init_channel_elem(grpc_exec_ctx* exec_ctx, grpc_channel_element* elem, grpc_channel_element_args* args) { @@ -270,44 +279,44 @@ static grpc_error* init_channel_elem(grpc_exec_ctx* exec_ctx, chand->max_age_grace_timer_pending = false; chand->channel_stack = args->channel_stack; chand->max_connection_age = - DEFAULT_MAX_CONNECTION_AGE_S == INT_MAX + DEFAULT_MAX_CONNECTION_AGE_MS == INT_MAX ? gpr_inf_future(GPR_TIMESPAN) - : gpr_time_from_seconds(DEFAULT_MAX_CONNECTION_AGE_S, GPR_TIMESPAN); + : gpr_time_from_millis(DEFAULT_MAX_CONNECTION_AGE_MS, GPR_TIMESPAN); chand->max_connection_age_grace = - DEFAULT_MAX_CONNECTION_AGE_GRACE_S == INT_MAX + DEFAULT_MAX_CONNECTION_AGE_GRACE_MS == INT_MAX ? gpr_inf_future(GPR_TIMESPAN) - : gpr_time_from_seconds(DEFAULT_MAX_CONNECTION_AGE_GRACE_S, - GPR_TIMESPAN); + : gpr_time_from_millis(DEFAULT_MAX_CONNECTION_AGE_GRACE_MS, + GPR_TIMESPAN); chand->max_connection_idle = - DEFAULT_MAX_CONNECTION_IDLE_S == INT_MAX + DEFAULT_MAX_CONNECTION_IDLE_MS == INT_MAX ? gpr_inf_future(GPR_TIMESPAN) - : gpr_time_from_seconds(DEFAULT_MAX_CONNECTION_IDLE_S, GPR_TIMESPAN); + : gpr_time_from_millis(DEFAULT_MAX_CONNECTION_IDLE_MS, GPR_TIMESPAN); for (size_t i = 0; i < args->channel_args->num_args; ++i) { if (0 == strcmp(args->channel_args->args[i].key, - GRPC_ARG_MAX_CONNECTION_AGE_S)) { + GRPC_ARG_MAX_CONNECTION_AGE_MS)) { const int value = grpc_channel_arg_get_integer( &args->channel_args->args[i], - (grpc_integer_options){DEFAULT_MAX_CONNECTION_AGE_S, 1, INT_MAX}); + (grpc_integer_options){DEFAULT_MAX_CONNECTION_AGE_MS, 1, INT_MAX}); chand->max_connection_age = value == INT_MAX ? gpr_inf_future(GPR_TIMESPAN) - : gpr_time_from_seconds(value, GPR_TIMESPAN); + : gpr_time_from_millis(value, GPR_TIMESPAN); } else if (0 == strcmp(args->channel_args->args[i].key, - GRPC_ARG_MAX_CONNECTION_AGE_GRACE_S)) { + GRPC_ARG_MAX_CONNECTION_AGE_GRACE_MS)) { const int value = grpc_channel_arg_get_integer( &args->channel_args->args[i], - (grpc_integer_options){DEFAULT_MAX_CONNECTION_AGE_GRACE_S, 0, + (grpc_integer_options){DEFAULT_MAX_CONNECTION_AGE_GRACE_MS, 0, INT_MAX}); chand->max_connection_age_grace = value == INT_MAX ? gpr_inf_future(GPR_TIMESPAN) - : gpr_time_from_seconds(value, GPR_TIMESPAN); + : gpr_time_from_millis(value, GPR_TIMESPAN); } else if (0 == strcmp(args->channel_args->args[i].key, - GRPC_ARG_MAX_CONNECTION_IDLE_S)) { + GRPC_ARG_MAX_CONNECTION_IDLE_MS)) { const int value = grpc_channel_arg_get_integer( &args->channel_args->args[i], - (grpc_integer_options){DEFAULT_MAX_CONNECTION_IDLE_S, 1, INT_MAX}); + (grpc_integer_options){DEFAULT_MAX_CONNECTION_IDLE_MS, 1, INT_MAX}); chand->max_connection_idle = value == INT_MAX ? gpr_inf_future(GPR_TIMESPAN) - : gpr_time_from_seconds(value, GPR_TIMESPAN); + : gpr_time_from_millis(value, GPR_TIMESPAN); } } grpc_closure_init(&chand->close_max_idle_channel, close_max_idle_channel, @@ -332,19 +341,21 @@ static grpc_error* init_channel_elem(grpc_exec_ctx* exec_ctx, if (gpr_time_cmp(chand->max_connection_age, gpr_inf_future(GPR_TIMESPAN)) != 0) { - // When the channel reaches its max age, we send down an op with - // goaway_error set. However, we can't send down any ops until after the - // channel stack is fully initialized. If we start the timer here, we have - // no guarantee that the timer won't pop before channel stack initialization - // is finished. To avoid that problem, we create a closure to start the - // timer, and we schedule that closure to be run after call stack - // initialization is done. + /* When the channel reaches its max age, we send down an op with + goaway_error set. However, we can't send down any ops until after the + channel stack is fully initialized. If we start the timer here, we have + no guarantee that the timer won't pop before channel stack initialization + is finished. To avoid that problem, we create a closure to start the + timer, and we schedule that closure to be run after call stack + initialization is done. */ GRPC_CHANNEL_STACK_REF(chand->channel_stack, "max_age start_max_age_timer_after_init"); grpc_closure_sched(exec_ctx, &chand->start_max_age_timer_after_init, GRPC_ERROR_NONE); } + /* Initialize the number of calls as 1, so that the max_idle_timer will not + start until start_max_idle_timer_after_init is invoked. */ gpr_atm_rel_store(&chand->call_count, 1); if (gpr_time_cmp(chand->max_connection_idle, gpr_inf_future(GPR_TIMESPAN)) != 0) { @@ -356,14 +367,14 @@ static grpc_error* init_channel_elem(grpc_exec_ctx* exec_ctx, return GRPC_ERROR_NONE; } -// Destructor for channel_data. +/* Destructor for channel_data. */ static void destroy_channel_elem(grpc_exec_ctx* exec_ctx, grpc_channel_element* elem) {} const grpc_channel_filter grpc_max_age_filter = { grpc_call_next_op, grpc_channel_next_op, - 0, // sizeof_call_data + 0, /* sizeof_call_data */ init_call_elem, grpc_call_stack_ignore_set_pollset_or_pollset_set, destroy_call_elem, diff --git a/test/core/end2end/tests/max_connection_age.c b/test/core/end2end/tests/max_connection_age.c index 8cd6ff0f87b..9b24c63385c 100644 --- a/test/core/end2end/tests/max_connection_age.c +++ b/test/core/end2end/tests/max_connection_age.c @@ -36,18 +36,16 @@ #include #include -#include #include #include -#include #include #include #include "test/core/end2end/cq_verifier.h" -#define MAX_CONNECTION_AGE_S 1 -#define MAX_CONNECTION_AGE_GRACE_S 2 -#define MAX_CONNECTION_IDLE_S 99 +#define MAX_CONNECTION_AGE_MS 500 +#define MAX_CONNECTION_AGE_GRACE_MS 1000 +#define MAX_CONNECTION_IDLE_MS 9999 static void *tag(intptr_t t) { return (void *)t; } @@ -84,14 +82,14 @@ static void test_max_age_forcibly_close(grpc_end2end_test_config config) { grpc_end2end_test_fixture f = config.create_fixture(NULL, NULL); cq_verifier *cqv = cq_verifier_create(f.cq); grpc_arg server_a[] = {{.type = GRPC_ARG_INTEGER, - .key = GRPC_ARG_MAX_CONNECTION_AGE_S, - .value.integer = MAX_CONNECTION_AGE_S}, + .key = GRPC_ARG_MAX_CONNECTION_AGE_MS, + .value.integer = MAX_CONNECTION_AGE_MS}, {.type = GRPC_ARG_INTEGER, - .key = GRPC_ARG_MAX_CONNECTION_AGE_GRACE_S, - .value.integer = MAX_CONNECTION_AGE_GRACE_S}, + .key = GRPC_ARG_MAX_CONNECTION_AGE_GRACE_MS, + .value.integer = MAX_CONNECTION_AGE_GRACE_MS}, {.type = GRPC_ARG_INTEGER, - .key = GRPC_ARG_MAX_CONNECTION_IDLE_S, - .value.integer = MAX_CONNECTION_IDLE_S}}; + .key = GRPC_ARG_MAX_CONNECTION_IDLE_MS, + .value.integer = MAX_CONNECTION_IDLE_MS}}; grpc_channel_args server_args = {.num_args = GPR_ARRAY_SIZE(server_a), .args = server_a}; @@ -159,7 +157,7 @@ static void test_max_age_forcibly_close(grpc_end2end_test_config config) { cq_verify(cqv); /* Wait for the channel to reach its max age */ - cq_verify_empty_timeout(cqv, MAX_CONNECTION_AGE_S + 1); + cq_verify_empty_timeout(cqv, 1); /* After the channel reaches its max age, we still do nothing here. And wait for it to use up its max age grace period. */ @@ -221,14 +219,14 @@ static void test_max_age_gracefully_close(grpc_end2end_test_config config) { grpc_end2end_test_fixture f = config.create_fixture(NULL, NULL); cq_verifier *cqv = cq_verifier_create(f.cq); grpc_arg server_a[] = {{.type = GRPC_ARG_INTEGER, - .key = GRPC_ARG_MAX_CONNECTION_AGE_S, - .value.integer = MAX_CONNECTION_AGE_S}, + .key = GRPC_ARG_MAX_CONNECTION_AGE_MS, + .value.integer = MAX_CONNECTION_AGE_MS}, {.type = GRPC_ARG_INTEGER, - .key = GRPC_ARG_MAX_CONNECTION_AGE_GRACE_S, + .key = GRPC_ARG_MAX_CONNECTION_AGE_GRACE_MS, .value.integer = INT_MAX}, {.type = GRPC_ARG_INTEGER, - .key = GRPC_ARG_MAX_CONNECTION_IDLE_S, - .value.integer = MAX_CONNECTION_IDLE_S}}; + .key = GRPC_ARG_MAX_CONNECTION_IDLE_MS, + .value.integer = MAX_CONNECTION_IDLE_MS}}; grpc_channel_args server_args = {.num_args = GPR_ARRAY_SIZE(server_a), .args = server_a}; @@ -296,7 +294,7 @@ static void test_max_age_gracefully_close(grpc_end2end_test_config config) { cq_verify(cqv); /* Wait for the channel to reach its max age */ - cq_verify_empty_timeout(cqv, MAX_CONNECTION_AGE_S + 1); + cq_verify_empty_timeout(cqv, 1); /* The connection is shutting down gracefully. In-progress rpc should not be closed, hence the completion queue should see nothing here. */ diff --git a/test/core/end2end/tests/max_connection_idle.c b/test/core/end2end/tests/max_connection_idle.c index 2a3b98491bd..9dc1ee47664 100644 --- a/test/core/end2end/tests/max_connection_idle.c +++ b/test/core/end2end/tests/max_connection_idle.c @@ -36,17 +36,15 @@ #include #include -#include #include #include -#include #include #include #include "test/core/end2end/cq_verifier.h" -#define MAX_CONNECTION_IDLE_S 1 -#define MAX_CONNECTION_AGE_S 99 +#define MAX_CONNECTION_IDLE_MS 500 +#define MAX_CONNECTION_AGE_MS 9999 static void *tag(intptr_t t) { return (void *)t; } @@ -59,11 +57,11 @@ static void test_max_connection_idle(grpc_end2end_test_config config) { .key = "grpc.testing.fixed_reconnect_backoff_ms", .value.integer = 1000}}; grpc_arg server_a[] = {{.type = GRPC_ARG_INTEGER, - .key = GRPC_ARG_MAX_CONNECTION_IDLE_S, - .value.integer = MAX_CONNECTION_IDLE_S}, + .key = GRPC_ARG_MAX_CONNECTION_IDLE_MS, + .value.integer = MAX_CONNECTION_IDLE_MS}, {.type = GRPC_ARG_INTEGER, - .key = GRPC_ARG_MAX_CONNECTION_AGE_S, - .value.integer = MAX_CONNECTION_AGE_S}}; + .key = GRPC_ARG_MAX_CONNECTION_AGE_MS, + .value.integer = MAX_CONNECTION_AGE_MS}}; grpc_channel_args client_args = {.num_args = GPR_ARRAY_SIZE(client_a), .args = client_a}; grpc_channel_args server_args = {.num_args = GPR_ARRAY_SIZE(server_a), @@ -91,7 +89,7 @@ static void test_max_connection_idle(grpc_end2end_test_config config) { /* wait for the channel to reach its maximum idle time */ grpc_channel_watch_connectivity_state( f.client, GRPC_CHANNEL_READY, - grpc_timeout_seconds_to_deadline(MAX_CONNECTION_IDLE_S + 1), f.cq, + grpc_timeout_milliseconds_to_deadline(MAX_CONNECTION_IDLE_MS + 500), f.cq, tag(99)); CQ_EXPECT_COMPLETION(cqv, tag(99), 1); cq_verify(cqv); From 679981069d4085235bddf08022995c0b34dad7aa Mon Sep 17 00:00:00 2001 From: Alexander Polcyn Date: Wed, 29 Mar 2017 13:21:08 -0700 Subject: [PATCH 12/20] add notes to build and run workers in different languages --- tools/run_tests/performance/README.md | 42 +++++++++++++++++++++++++-- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/tools/run_tests/performance/README.md b/tools/run_tests/performance/README.md index af69f39d3c1..5fd64f6ee91 100644 --- a/tools/run_tests/performance/README.md +++ b/tools/run_tests/performance/README.md @@ -24,6 +24,43 @@ GCE "worker" machines that are in the same zone. * For example, to start the grpc-go benchmark worker: [grpc-go worker main.go](https://github.com/grpc/grpc-go/blob/master/benchmark/worker/main.go) --driver_port +#### Comands to start workers in different languages: + * Note that these commands are what the top-level + [run_performance_test.py](../run_performance_tests.py) script uses to + build and run different workers through the + [build_performance.sh](./build_performance.sh) script and "run worker" + scripts (such as the [run_worker_java.sh](./run_worker_java.sh)). + +##### Running benchmark workers for C-core wrapped languages (C++, Python, C#, Node, Ruby): + * These are more simple since they all live in the main grpc repo. + +``` +$ cd +$ tools/run_tests/performance/build_performance.sh +$ tools/run_tests/performance/run_worker_.sh +``` + + * Note that there is one "run_worker" script per language, e.g., + [run_worker_csharp.sh](./run_worker_csharp.sh) for c#. + +##### Running benchmark workers for gRPC-Java: + * You'll need the [grpc-java](https://github.com/grpc/grpc-java) repo. + +``` +$ cd +$ ./gradlew -PskipCodegen=true :grpc-benchmarks:installDist +$ benchmarks/build/install/grpc-benchmarks/bin/benchmark_worker --driver_port +``` + +##### Running benchmark workers for gRPC-Go: + * You'll need the [grpc-go repo](https://github.com/grpc/grpc-go) + +``` +$ cd /benchmark/worker && go install +$ # if profiling, it might be helpful to turn off inlining by building with "-gcflags=-l" +$ $GOPATH/bin/worker --driver_port +``` + #### Build the driver: * Connect to the driver machine (if using a remote driver) and from the grpc repo root: ``` @@ -40,7 +77,7 @@ $ tools/run_tests/performance/build_performance.sh 2. From the grpc repo root: -* Set `QPS_WORKERS` environment variable to a commaa separated list of worker +* Set `QPS_WORKERS` environment variable to a comma separated list of worker machines. Note that the driver will start the "benchmark server" on the first entry in the list, and the rest will be told to run as clients against the benchmark server. @@ -48,8 +85,7 @@ benchmark server. Example running and profiling of go benchmark server: ``` $ export QPS_WORKERS=:<10000>,,10000,:10000 -$ bins/opt/qps_json_driver ---scenario_json='' +$ bins/opt/qps_json_driver --scenario_json='' ``` ### Example profiling commands From ea2b439bc45f76bc53159df2a3786e076b3b2be4 Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Thu, 30 Mar 2017 13:57:01 -0700 Subject: [PATCH 13/20] Verify shutdown time in max_connection_age test --- test/core/end2end/tests/max_connection_age.c | 48 ++++++++++++++------ 1 file changed, 35 insertions(+), 13 deletions(-) diff --git a/test/core/end2end/tests/max_connection_age.c b/test/core/end2end/tests/max_connection_age.c index 9b24c63385c..9cb0dc8dcc8 100644 --- a/test/core/end2end/tests/max_connection_age.c +++ b/test/core/end2end/tests/max_connection_age.c @@ -47,6 +47,18 @@ #define MAX_CONNECTION_AGE_GRACE_MS 1000 #define MAX_CONNECTION_IDLE_MS 9999 +#define CALL_DEADLINE_S 10 +/* The amount of time we wait for the connection to time out, but after it the + connection should not use up its grace period. It should be a number between + MAX_CONNECTION_AGE_MS and MAX_CONNECTION_AGE_MS + + MAX_CONNECTION_AGE_GRACE_MS */ +#define CQ_MAX_CONNECTION_AGE_WAIT_TIME_S 1 +/* The amount of time we wait after the connection reaches its max age, it + should be shorter than CALL_DEADLINE_S - CQ_MAX_CONNECTION_AGE_WAIT_TIME_S */ +#define CQ_MAX_CONNECTION_AGE_GRACE_WAIT_TIME_S 2 +/* The grace period for the test to observe the channel shutdown process */ +#define IMMEDIATE_SHUTDOWN_GRACE_TIME_MS 300 + static void *tag(intptr_t t) { return (void *)t; } static void drain_cq(grpc_completion_queue *cq) { @@ -98,7 +110,7 @@ static void test_max_age_forcibly_close(grpc_end2end_test_config config) { grpc_call *c; grpc_call *s; - gpr_timespec deadline = grpc_timeout_seconds_to_deadline(10); + gpr_timespec deadline = grpc_timeout_seconds_to_deadline(CALL_DEADLINE_S); grpc_op ops[6]; grpc_op *op; grpc_metadata_array initial_metadata_recv; @@ -153,17 +165,27 @@ static void test_max_age_forcibly_close(grpc_end2end_test_config config) { grpc_server_request_call(f.server, &s, &call_details, &request_metadata_recv, f.cq, f.cq, tag(101)); GPR_ASSERT(GRPC_CALL_OK == error); - CQ_EXPECT_COMPLETION(cqv, tag(101), 1); + CQ_EXPECT_COMPLETION(cqv, tag(101), true); cq_verify(cqv); + gpr_timespec channel_start_time = gpr_now(GPR_CLOCK_MONOTONIC); + gpr_timespec expect_shutdown_time = gpr_time_add( + channel_start_time, + gpr_time_from_millis(MAX_CONNECTION_AGE_MS + MAX_CONNECTION_AGE_GRACE_MS + + IMMEDIATE_SHUTDOWN_GRACE_TIME_MS, + GPR_TIMESPAN)); + /* Wait for the channel to reach its max age */ - cq_verify_empty_timeout(cqv, 1); + cq_verify_empty_timeout(cqv, CQ_MAX_CONNECTION_AGE_WAIT_TIME_S); /* After the channel reaches its max age, we still do nothing here. And wait for it to use up its max age grace period. */ - CQ_EXPECT_COMPLETION(cqv, tag(1), 1); + CQ_EXPECT_COMPLETION(cqv, tag(1), true); cq_verify(cqv); + gpr_timespec channel_shutdown_time = gpr_now(GPR_CLOCK_MONOTONIC); + GPR_ASSERT(gpr_time_cmp(channel_shutdown_time, expect_shutdown_time) < 0); + memset(ops, 0, sizeof(ops)); op = ops; op->op = GRPC_OP_SEND_INITIAL_METADATA; @@ -186,11 +208,11 @@ static void test_max_age_forcibly_close(grpc_end2end_test_config config) { op++; error = grpc_call_start_batch(s, ops, (size_t)(op - ops), tag(102), NULL); GPR_ASSERT(GRPC_CALL_OK == error); - CQ_EXPECT_COMPLETION(cqv, tag(102), 1); + CQ_EXPECT_COMPLETION(cqv, tag(102), true); cq_verify(cqv); grpc_server_shutdown_and_notify(f.server, f.cq, tag(0xdead)); - CQ_EXPECT_COMPLETION(cqv, tag(0xdead), 1); + CQ_EXPECT_COMPLETION(cqv, tag(0xdead), true); cq_verify(cqv); grpc_call_destroy(s); @@ -235,7 +257,7 @@ static void test_max_age_gracefully_close(grpc_end2end_test_config config) { grpc_call *c; grpc_call *s; - gpr_timespec deadline = grpc_timeout_seconds_to_deadline(10); + gpr_timespec deadline = grpc_timeout_seconds_to_deadline(CALL_DEADLINE_S); grpc_op ops[6]; grpc_op *op; grpc_metadata_array initial_metadata_recv; @@ -290,15 +312,15 @@ static void test_max_age_gracefully_close(grpc_end2end_test_config config) { grpc_server_request_call(f.server, &s, &call_details, &request_metadata_recv, f.cq, f.cq, tag(101)); GPR_ASSERT(GRPC_CALL_OK == error); - CQ_EXPECT_COMPLETION(cqv, tag(101), 1); + CQ_EXPECT_COMPLETION(cqv, tag(101), true); cq_verify(cqv); /* Wait for the channel to reach its max age */ - cq_verify_empty_timeout(cqv, 1); + cq_verify_empty_timeout(cqv, CQ_MAX_CONNECTION_AGE_WAIT_TIME_S); /* The connection is shutting down gracefully. In-progress rpc should not be closed, hence the completion queue should see nothing here. */ - cq_verify_empty_timeout(cqv, 2); + cq_verify_empty_timeout(cqv, CQ_MAX_CONNECTION_AGE_GRACE_WAIT_TIME_S); memset(ops, 0, sizeof(ops)); op = ops; @@ -323,12 +345,12 @@ static void test_max_age_gracefully_close(grpc_end2end_test_config config) { error = grpc_call_start_batch(s, ops, (size_t)(op - ops), tag(102), NULL); GPR_ASSERT(GRPC_CALL_OK == error); - CQ_EXPECT_COMPLETION(cqv, tag(102), 1); - CQ_EXPECT_COMPLETION(cqv, tag(1), 1); + CQ_EXPECT_COMPLETION(cqv, tag(102), true); + CQ_EXPECT_COMPLETION(cqv, tag(1), true); cq_verify(cqv); grpc_server_shutdown_and_notify(f.server, f.cq, tag(0xdead)); - CQ_EXPECT_COMPLETION(cqv, tag(0xdead), 1); + CQ_EXPECT_COMPLETION(cqv, tag(0xdead), true); cq_verify(cqv); grpc_call_destroy(s); From 9d3905c2f9ae1bf529f347fdf9c29da08fccf729 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Thu, 30 Mar 2017 16:35:31 -0700 Subject: [PATCH 14/20] Add selected fullstack benchmarks to pr suite --- tools/jenkins/run_performance.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/jenkins/run_performance.sh b/tools/jenkins/run_performance.sh index dee033a951a..544e31dcbdb 100755 --- a/tools/jenkins/run_performance.sh +++ b/tools/jenkins/run_performance.sh @@ -32,7 +32,7 @@ set -ex # List of benchmarks that provide good signal for analyzing performance changes in pull requests -BENCHMARKS_TO_RUN="bm_closure bm_cq bm_call_create bm_error bm_chttp2_hpack bm_chttp2_transport bm_pollset bm_metadata" +BENCHMARKS_TO_RUN="bm_fullstack_unary_ping_pong bm_fullstack_streaming_ping_pong bm_fullstack_streaming_pump bm_closure bm_cq bm_call_create bm_error bm_chttp2_hpack bm_chttp2_transport bm_pollset bm_metadata" # Enter the gRPC repo root cd $(dirname $0)/../.. From 83643bd5cbd7e816cd211902804b718e3d0740ab Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 31 Mar 2017 06:02:57 -0700 Subject: [PATCH 15/20] Fix build on mac --- test/cpp/microbenchmarks/bm_chttp2_hpack.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cpp/microbenchmarks/bm_chttp2_hpack.cc b/test/cpp/microbenchmarks/bm_chttp2_hpack.cc index 8fbfd0fa132..55d2d2f58d0 100644 --- a/test/cpp/microbenchmarks/bm_chttp2_hpack.cc +++ b/test/cpp/microbenchmarks/bm_chttp2_hpack.cc @@ -97,7 +97,7 @@ static void BM_HpackEncoderEncodeHeader(benchmark::State &state) { logged_representative_output = true; for (size_t i = 0; i < outbuf.count; i++) { char *s = grpc_dump_slice(outbuf.slices[i], GPR_DUMP_HEX); - gpr_log(GPR_DEBUG, "%" PRId64 ": %s", i, s); + gpr_log(GPR_DEBUG, "%" PRIdPTR ": %s", i, s); gpr_free(s); } } From 6b0afac4750c19f13852cd22f9f25427fd18a3a1 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 31 Mar 2017 06:41:04 -0700 Subject: [PATCH 16/20] Sensitivity --- tools/profiling/microbenchmarks/bm_diff.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/profiling/microbenchmarks/bm_diff.py b/tools/profiling/microbenchmarks/bm_diff.py index 339e9595185..d6119332a77 100755 --- a/tools/profiling/microbenchmarks/bm_diff.py +++ b/tools/profiling/microbenchmarks/bm_diff.py @@ -65,8 +65,8 @@ nanos = { 'pct_diff': 5, } counter = { - 'abs_diff': 1, - 'pct_diff': 1, + 'abs_diff': 0.5, + 'pct_diff': 0.5, } _INTERESTING = { From dc44c26856d7cc011cc28bad19327a7278e159b5 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 31 Mar 2017 06:41:23 -0700 Subject: [PATCH 17/20] Revert "Spam cleanup" This reverts commit 1463d0e74df002e8d48515ad03d132f5f4cf7ad2. --- tools/profiling/microbenchmarks/bm_diff.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tools/profiling/microbenchmarks/bm_diff.py b/tools/profiling/microbenchmarks/bm_diff.py index d6119332a77..6f39738f594 100755 --- a/tools/profiling/microbenchmarks/bm_diff.py +++ b/tools/profiling/microbenchmarks/bm_diff.py @@ -185,6 +185,9 @@ class Benchmark: old_mdn = median(old) delta = new_mdn - old_mdn ratio = changed_ratio(new_mdn, old_mdn) + print 'new=%r old=%r new_mdn=%f old_mdn=%f delta=%f ratio=%f p=%f' % ( + new, old, new_mdn, old_mdn, delta, ratio, p + ) if p < args.p_threshold and abs(delta) > _INTERESTING[f]['abs_diff'] and abs(ratio) > _INTERESTING[f]['pct_diff']: self.final[f] = delta return self.final.keys() @@ -209,16 +212,19 @@ for bm in comparables: js_old_opt = json.loads(f.read()) for row in bm_json.expand_json(js_new_ctr, js_new_opt): + print row name = row['cpp_name'] if name.endswith('_mean') or name.endswith('_stddev'): continue benchmarks[name].add_sample(row, True) for row in bm_json.expand_json(js_old_ctr, js_old_opt): + print row name = row['cpp_name'] if name.endswith('_mean') or name.endswith('_stddev'): continue benchmarks[name].add_sample(row, False) really_interesting = set() for name, bm in benchmarks.items(): + print name really_interesting.update(bm.process()) fields = [f for f in args.track if f in args.track] From 9298a92f78a8f4c024a3c3eddd287794521aaf85 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 31 Mar 2017 06:51:48 -0700 Subject: [PATCH 18/20] Fixes --- tools/profiling/microbenchmarks/bm_diff.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/profiling/microbenchmarks/bm_diff.py b/tools/profiling/microbenchmarks/bm_diff.py index 6f39738f594..4255e29ff2f 100755 --- a/tools/profiling/microbenchmarks/bm_diff.py +++ b/tools/profiling/microbenchmarks/bm_diff.py @@ -176,7 +176,7 @@ class Benchmark: self.samples[new][f].append(float(data[f])) def process(self): - for f in args.track: + for f in sorted(args.track): new = self.samples[True][f] old = self.samples[False][f] if not new or not old: continue @@ -185,10 +185,10 @@ class Benchmark: old_mdn = median(old) delta = new_mdn - old_mdn ratio = changed_ratio(new_mdn, old_mdn) - print 'new=%r old=%r new_mdn=%f old_mdn=%f delta=%f ratio=%f p=%f' % ( - new, old, new_mdn, old_mdn, delta, ratio, p + print '%s: new=%r old=%r new_mdn=%f old_mdn=%f delta=%f(%f:%f) ratio=%f(%f:%f) p=%f' % ( + f, new, old, new_mdn, old_mdn, delta, abs(delta), _INTERESTING[f]['abs_diff'], ratio, abs(ratio), _INTERESTING[f]['pct_diff']/100.0, p ) - if p < args.p_threshold and abs(delta) > _INTERESTING[f]['abs_diff'] and abs(ratio) > _INTERESTING[f]['pct_diff']: + if p < args.p_threshold and abs(delta) > _INTERESTING[f]['abs_diff'] and abs(ratio) > _INTERESTING[f]['pct_diff']/100.0: self.final[f] = delta return self.final.keys() From 81d3ba8d273b928d35ff8e13b909419d1af7a710 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 31 Mar 2017 09:44:23 -0700 Subject: [PATCH 19/20] Tighten constraints for bm_diff to trigger --- tools/profiling/microbenchmarks/bm_diff.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/profiling/microbenchmarks/bm_diff.py b/tools/profiling/microbenchmarks/bm_diff.py index 4255e29ff2f..1dff3335eed 100755 --- a/tools/profiling/microbenchmarks/bm_diff.py +++ b/tools/profiling/microbenchmarks/bm_diff.py @@ -66,7 +66,7 @@ nanos = { } counter = { 'abs_diff': 0.5, - 'pct_diff': 0.5, + 'pct_diff': 5, } _INTERESTING = { @@ -102,7 +102,7 @@ argp.add_argument('-t', '--track', argp.add_argument('-b', '--benchmarks', nargs='+', choices=_AVAILABLE_BENCHMARK_TESTS, default=['bm_cq']) argp.add_argument('-d', '--diff_base', type=str) argp.add_argument('-r', '--repetitions', type=int, default=4) -argp.add_argument('-p', '--p_threshold', type=float, default=0.05) +argp.add_argument('-p', '--p_threshold', type=float, default=0.03) args = argp.parse_args() assert args.diff_base From c1d438154b636ebaf96ec4b8c78e7f22cdf35475 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 31 Mar 2017 12:34:48 -0700 Subject: [PATCH 20/20] Use timeout inflation to account for differences between sanitizers --- test/core/end2end/tests/max_connection_age.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/test/core/end2end/tests/max_connection_age.c b/test/core/end2end/tests/max_connection_age.c index 9cb0dc8dcc8..1de54e08252 100644 --- a/test/core/end2end/tests/max_connection_age.c +++ b/test/core/end2end/tests/max_connection_age.c @@ -168,12 +168,9 @@ static void test_max_age_forcibly_close(grpc_end2end_test_config config) { CQ_EXPECT_COMPLETION(cqv, tag(101), true); cq_verify(cqv); - gpr_timespec channel_start_time = gpr_now(GPR_CLOCK_MONOTONIC); - gpr_timespec expect_shutdown_time = gpr_time_add( - channel_start_time, - gpr_time_from_millis(MAX_CONNECTION_AGE_MS + MAX_CONNECTION_AGE_GRACE_MS + - IMMEDIATE_SHUTDOWN_GRACE_TIME_MS, - GPR_TIMESPAN)); + gpr_timespec expect_shutdown_time = grpc_timeout_milliseconds_to_deadline( + MAX_CONNECTION_AGE_MS + MAX_CONNECTION_AGE_GRACE_MS + + IMMEDIATE_SHUTDOWN_GRACE_TIME_MS); /* Wait for the channel to reach its max age */ cq_verify_empty_timeout(cqv, CQ_MAX_CONNECTION_AGE_WAIT_TIME_S);