diff --git a/include/grpc/impl/codegen/grpc_types.h b/include/grpc/impl/codegen/grpc_types.h index 4471ccf7456..626668b1c20 100644 --- a/include/grpc/impl/codegen/grpc_types.h +++ b/include/grpc/impl/codegen/grpc_types.h @@ -179,6 +179,15 @@ typedef struct { Larger values give lower CPU usage for large messages, but more head of line blocking for small messages. */ #define GRPC_ARG_HTTP2_MAX_FRAME_SIZE "grpc.http2.max_frame_size" +/** Minimum time (in milliseconds) between successive ping frames being sent */ +#define GRPC_ARG_HTTP2_MIN_TIME_BETWEEN_PINGS_MS \ + "grpc.http2.min_time_between_pings_ms" +/** How many pings can we send before needing to send a data frame or header + frame? + (0 indicates that an infinite number of pings can be sent without sending + a data frame or header frame) */ +#define GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA \ + "grpc.http2.max_pings_without_data" /** Default authority to pass if none specified on call construction. A string. * */ #define GRPC_ARG_DEFAULT_AUTHORITY "grpc.default_authority" diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c index c07b51d35f4..558d588b40e 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c @@ -133,6 +133,9 @@ static void send_ping_locked(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, grpc_closure *on_initiate, grpc_closure *on_complete); +#define DEFAULT_MIN_TIME_BETWEEN_PINGS_MS 100 +#define DEFAULT_MAX_PINGS_BETWEEN_DATA 1 + /******************************************************************************* * CONSTRUCTION/DESTRUCTION/REFCOUNTING */ @@ -302,6 +305,12 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, push_setting(exec_ctx, t, GRPC_CHTTP2_SETTINGS_MAX_HEADER_LIST_SIZE, DEFAULT_MAX_HEADER_LIST_SIZE); + t->ping_policy = (grpc_chttp2_repeated_ping_policy){ + .max_pings_without_data = DEFAULT_MAX_PINGS_BETWEEN_DATA, + .min_time_between_pings = + gpr_time_from_millis(DEFAULT_MIN_TIME_BETWEEN_PINGS_MS, GPR_TIMESPAN), + }; + if (channel_args) { for (i = 0; i < channel_args->num_args; i++) { if (0 == strcmp(channel_args->args[i].key, @@ -327,6 +336,19 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, grpc_chttp2_hpack_compressor_set_max_usable_size(&t->hpack_compressor, (uint32_t)value); } + } else if (0 == strcmp(channel_args->args[i].key, + GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA)) { + t->ping_policy.max_pings_without_data = grpc_channel_arg_get_integer( + &channel_args->args[i], + (grpc_integer_options){DEFAULT_MAX_PINGS_BETWEEN_DATA, 0, INT_MAX}); + } else if (0 == strcmp(channel_args->args[i].key, + GRPC_ARG_HTTP2_MIN_TIME_BETWEEN_PINGS_MS)) { + t->ping_policy.min_time_between_pings = gpr_time_from_millis( + grpc_channel_arg_get_integer( + &channel_args->args[i], + (grpc_integer_options){DEFAULT_MIN_TIME_BETWEEN_PINGS_MS, 0, + INT_MAX}), + GPR_TIMESPAN); } else { static const struct { const char *channel_arg_name; @@ -1913,8 +1935,8 @@ static void read_action_locked(grpc_exec_ctx *exec_ctx, void *tp, gpr_timespec now = gpr_now(GPR_CLOCK_MONOTONIC); gpr_timespec dt_timespec = gpr_time_sub(now, t->last_pid_update); double dt = (double)dt_timespec.tv_sec + dt_timespec.tv_nsec * 1e-9; - if (dt > 3) { - grpc_pid_controller_reset(&t->pid_controller); + if (dt > 0.1) { + dt = 0.1; } double log2_bdp_guess = grpc_pid_controller_update(&t->pid_controller, bdp_error, dt); diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h index 8b7c040358d..3088aa7417b 100644 --- a/src/core/ext/transport/chttp2/transport/internal.h +++ b/src/core/ext/transport/chttp2/transport/internal.h @@ -93,6 +93,16 @@ typedef struct { uint64_t inflight_id; } grpc_chttp2_ping_queue; +typedef struct { + gpr_timespec min_time_between_pings; + int max_pings_without_data; +} grpc_chttp2_repeated_ping_policy; + +typedef struct { + gpr_timespec last_ping_sent_time; + int pings_before_data_required; +} grpc_chttp2_repeated_ping_state; + /* deframer state for the overall http2 stream of bytes */ typedef enum { /* prefix: one entry per http2 connection prefix byte */ @@ -281,6 +291,8 @@ struct grpc_chttp2_transport { /** ping queues for various ping insertion points */ grpc_chttp2_ping_queue ping_queues[GRPC_CHTTP2_PING_TYPE_COUNT]; + grpc_chttp2_repeated_ping_policy ping_policy; + grpc_chttp2_repeated_ping_state ping_state; uint64_t ping_ctr; /* unique id for pings */ /** parser for headers */ diff --git a/src/core/ext/transport/chttp2/transport/writing.c b/src/core/ext/transport/chttp2/transport/writing.c index 43205a82f67..57648a704de 100644 --- a/src/core/ext/transport/chttp2/transport/writing.c +++ b/src/core/ext/transport/chttp2/transport/writing.c @@ -75,6 +75,17 @@ static void maybe_initiate_ping(grpc_exec_ctx *exec_ctx, /* ping already in-flight: wait */ return; } + if (t->ping_state.pings_before_data_required > 0 && + t->ping_policy.max_pings_without_data != 0) { + /* need to send something of substance before sending a ping again */ + return; + } + gpr_timespec now = gpr_now(GPR_CLOCK_MONOTONIC); + if (gpr_time_cmp(gpr_time_sub(now, t->ping_state.last_ping_sent_time), + t->ping_policy.min_time_between_pings) < 0) { + /* not enough elapsed time between successive pings */ + return; + } /* coalesce equivalent pings into this one */ switch (ping_type) { case GRPC_CHTTP2_PING_BEFORE_TRANSPORT_WINDOW_UPDATE: @@ -92,6 +103,9 @@ static void maybe_initiate_ping(grpc_exec_ctx *exec_ctx, &pq->lists[GRPC_CHTTP2_PCL_INFLIGHT]); grpc_slice_buffer_add(&t->outbuf, grpc_chttp2_ping_create(false, pq->inflight_id)); + t->ping_state.last_ping_sent_time = now; + t->ping_state.pings_before_data_required -= + (t->ping_state.pings_before_data_required != 0); } static void update_list(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, @@ -165,6 +179,8 @@ bool grpc_chttp2_begin_write(grpc_exec_ctx *exec_ctx, s->sent_initial_metadata = true; sent_initial_metadata = true; now_writing = true; + t->ping_state.pings_before_data_required = + t->ping_policy.max_pings_without_data; } /* send any window updates */ if (s->announce_window > 0) { @@ -202,6 +218,8 @@ bool grpc_chttp2_begin_write(grpc_exec_ctx *exec_ctx, send_bytes); GRPC_CHTTP2_FLOW_DEBIT_TRANSPORT("write", t, outgoing_window, send_bytes); + t->ping_state.pings_before_data_required = + t->ping_policy.max_pings_without_data; if (is_last_frame) { s->send_trailing_metadata = NULL; s->sent_trailing_metadata = true;