Set last_ping_sent_time to inf_past after receiving data

pull/11587/head
Yuchen Zeng 8 years ago
parent 27411495f6
commit ad32075af8
  1. 13
      include/grpc/impl/codegen/grpc_types.h
  2. 62
      src/core/ext/transport/chttp2/transport/chttp2_transport.c
  3. 2
      src/core/ext/transport/chttp2/transport/frame_ping.c
  4. 4
      src/core/ext/transport/chttp2/transport/internal.h
  5. 2
      src/core/ext/transport/chttp2/transport/parsing.c
  6. 41
      src/core/ext/transport/chttp2/transport/writing.c
  7. 2
      test/core/client_channel/lb_policies_test.c
  8. 21
      test/core/end2end/tests/bad_ping.c
  9. 21
      test/core/end2end/tests/ping.c

@ -188,9 +188,14 @@ typedef struct {
#define GRPC_ARG_HTTP2_MAX_FRAME_SIZE "grpc.http2.max_frame_size"
/** Should BDP probing be performed? */
#define GRPC_ARG_HTTP2_BDP_PROBE "grpc.http2.bdp_probe"
/** Minimum time (in milliseconds) between successive ping frames being sent */
#define GRPC_ARG_HTTP2_MIN_TIME_BETWEEN_PINGS_MS \
/** Minimum time between sending successive ping frames without receiving any
data frame, Int valued, milliseconds. */
#define GRPC_ARG_HTTP2_MIN_SENT_PING_INTERVAL_WITHOUT_DATA_MS \
"grpc.http2.min_time_between_pings_ms"
/** Minimum allowed time between receiving successive ping frames without
sending any data frame. Int valued, milliseconds */
#define GRPC_ARG_HTTP2_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS \
"grpc.http2.min_ping_interval_without_data_ms"
/** Channel arg to override the http2 :scheme header */
#define GRPC_ARG_HTTP2_SCHEME "grpc.http2_scheme"
/** How many pings can we send before needing to send a data frame or header
@ -202,10 +207,6 @@ typedef struct {
closing the transport? (0 indicates that the server can bear an infinite
number of misbehaving pings) */
#define GRPC_ARG_HTTP2_MAX_PING_STRIKES "grpc.http2.max_ping_strikes"
/** Minimum allowed time between two pings without sending any data frame. Int
valued, seconds */
#define GRPC_ARG_HTTP2_MIN_PING_INTERVAL_WITHOUT_DATA_MS \
"grpc.http2.min_ping_interval_without_data_ms"
/** How much data are we willing to queue up per stream if
GRPC_WRITE_BUFFER_HINT is set? This is an upper bound */
#define GRPC_ARG_HTTP2_WRITE_BUFFER_SIZE "grpc.http2.write_buffer_size"

@ -152,10 +152,10 @@ static void send_ping_locked(
static void retry_initiate_ping_locked(grpc_exec_ctx *exec_ctx, void *tp,
grpc_error *error);
#define DEFAULT_MIN_TIME_BETWEEN_PINGS_MS 0
#define DEFAULT_MAX_PINGS_BETWEEN_DATA 3
#define DEFAULT_MIN_SENT_PING_INTERVAL_WITHOUT_DATA_MS 300000 /* 5 minutes */
#define DEFAULT_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS 300000 /* 5 minutes */
#define DEFAULT_MAX_PINGS_BETWEEN_DATA 2
#define DEFAULT_MAX_PING_STRIKES 2
#define DEFAULT_MIN_PING_INTERVAL_WITHOUT_DATA_MS 300000 /* 5 minutes */
/** keepalive-relevant functions */
static void init_keepalive_ping_locked(grpc_exec_ctx *exec_ctx, void *arg,
@ -364,11 +364,11 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,
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),
.min_sent_ping_interval_without_data = gpr_time_from_millis(
DEFAULT_MIN_SENT_PING_INTERVAL_WITHOUT_DATA_MS, GPR_TIMESPAN),
.max_ping_strikes = DEFAULT_MAX_PING_STRIKES,
.min_ping_interval_without_data = gpr_time_from_millis(
DEFAULT_MIN_PING_INTERVAL_WITHOUT_DATA_MS, GPR_TIMESPAN),
.min_recv_ping_interval_without_data = gpr_time_from_millis(
DEFAULT_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS, GPR_TIMESPAN),
};
/* Keepalive setting */
@ -434,23 +434,30 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,
t->ping_policy.max_ping_strikes = grpc_channel_arg_get_integer(
&channel_args->args[i],
(grpc_integer_options){DEFAULT_MAX_PING_STRIKES, 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 if (0 ==
strcmp(channel_args->args[i].key,
GRPC_ARG_HTTP2_MIN_PING_INTERVAL_WITHOUT_DATA_MS)) {
t->ping_policy.min_ping_interval_without_data = gpr_time_from_millis(
grpc_channel_arg_get_integer(
&channel_args->args[i],
(grpc_integer_options){
DEFAULT_MIN_PING_INTERVAL_WITHOUT_DATA_MS, 0, INT_MAX}),
GPR_TIMESPAN);
strcmp(
channel_args->args[i].key,
GRPC_ARG_HTTP2_MIN_SENT_PING_INTERVAL_WITHOUT_DATA_MS)) {
t->ping_policy.min_sent_ping_interval_without_data =
gpr_time_from_millis(
grpc_channel_arg_get_integer(
&channel_args->args[i],
(grpc_integer_options){
DEFAULT_MIN_SENT_PING_INTERVAL_WITHOUT_DATA_MS, 0,
INT_MAX}),
GPR_TIMESPAN);
} else if (0 ==
strcmp(
channel_args->args[i].key,
GRPC_ARG_HTTP2_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS)) {
t->ping_policy.min_recv_ping_interval_without_data =
gpr_time_from_millis(
grpc_channel_arg_get_integer(
&channel_args->args[i],
(grpc_integer_options){
DEFAULT_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS, 0,
INT_MAX}),
GPR_TIMESPAN);
} else if (0 == strcmp(channel_args->args[i].key,
GRPC_ARG_HTTP2_WRITE_BUFFER_SIZE)) {
t->write_buffer_size = (uint32_t)grpc_channel_arg_get_integer(
@ -625,6 +632,9 @@ static void close_transport_locked(grpc_exec_ctx *exec_ctx,
connectivity_state_set(exec_ctx, t, GRPC_CHANNEL_SHUTDOWN,
GRPC_ERROR_REF(error), "close_transport");
grpc_endpoint_shutdown(exec_ctx, t->ep, GRPC_ERROR_REF(error));
if (t->ping_state.is_delayed_ping_timer_set) {
grpc_timer_cancel(exec_ctx, &t->ping_state.delayed_ping_timer);
}
switch (t->keepalive_state) {
case GRPC_CHTTP2_KEEPALIVE_STATE_WAITING:
grpc_timer_cancel(exec_ctx, &t->keepalive_ping_timer);
@ -1729,8 +1739,10 @@ static void retry_initiate_ping_locked(grpc_exec_ctx *exec_ctx, void *tp,
grpc_error *error) {
grpc_chttp2_transport *t = (grpc_chttp2_transport *)tp;
t->ping_state.is_delayed_ping_timer_set = false;
grpc_chttp2_initiate_write(exec_ctx, t,
GRPC_CHTTP2_INITIATE_WRITE_RETRY_SEND_PING);
if (error == GRPC_ERROR_NONE) {
grpc_chttp2_initiate_write(exec_ctx, t,
GRPC_CHTTP2_INITIATE_WRITE_RETRY_SEND_PING);
}
}
void grpc_chttp2_ack_ping(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,

@ -92,7 +92,7 @@ grpc_error *grpc_chttp2_ping_parser_parse(grpc_exec_ctx *exec_ctx, void *parser,
gpr_timespec now = gpr_now(GPR_CLOCK_MONOTONIC);
gpr_timespec next_allowed_ping =
gpr_time_add(t->ping_recv_state.last_ping_recv_time,
t->ping_policy.min_ping_interval_without_data);
t->ping_policy.min_recv_ping_interval_without_data);
if (t->keepalive_permit_without_calls == 0 &&
grpc_chttp2_stream_map_size(&t->stream_map) == 0) {

@ -112,10 +112,10 @@ typedef struct {
} grpc_chttp2_ping_queue;
typedef struct {
gpr_timespec min_time_between_pings;
int max_pings_without_data;
int max_ping_strikes;
gpr_timespec min_ping_interval_without_data;
gpr_timespec min_sent_ping_interval_without_data;
gpr_timespec min_recv_ping_interval_without_data;
} grpc_chttp2_repeated_ping_policy;
typedef struct {

@ -385,6 +385,7 @@ error_handler:
t->parser_data = &s->data_parser;
t->ping_state.pings_before_data_required =
t->ping_policy.max_pings_without_data;
t->ping_state.last_ping_sent_time = gpr_inf_past(GPR_CLOCK_MONOTONIC);
return GRPC_ERROR_NONE;
} else if (grpc_error_get_int(err, GRPC_ERROR_INT_STREAM_ID, NULL)) {
/* handle stream errors by closing the stream */
@ -563,6 +564,7 @@ static grpc_error *init_header_frame_parser(grpc_exec_ctx *exec_ctx,
t->ping_state.pings_before_data_required =
t->ping_policy.max_pings_without_data;
t->ping_state.last_ping_sent_time = gpr_inf_past(GPR_CLOCK_MONOTONIC);
/* could be a new grpc_chttp2_stream or an existing grpc_chttp2_stream */
s = grpc_chttp2_parsing_lookup_stream(t, t->incoming_stream_id);

@ -66,14 +66,6 @@ static void maybe_initiate_ping(grpc_exec_ctx *exec_ctx,
}
return;
}
if (t->keepalive_permit_without_calls == 0 &&
grpc_chttp2_stream_map_size(&t->stream_map) == 0) {
if (GRPC_TRACER_ON(grpc_http_trace) ||
GRPC_TRACER_ON(grpc_bdp_estimator_trace)) {
gpr_log(GPR_DEBUG, "Ping delayed [%p]: no active stream", t->peer_string);
}
return;
}
if (t->ping_state.pings_before_data_required == 0 &&
t->ping_policy.max_pings_without_data != 0) {
/* need to receive something of substance before sending a ping again */
@ -86,11 +78,18 @@ static void maybe_initiate_ping(grpc_exec_ctx *exec_ctx,
return;
}
gpr_timespec now = gpr_now(GPR_CLOCK_MONOTONIC);
gpr_timespec elapsed = gpr_time_sub(now, t->ping_state.last_ping_sent_time);
/*gpr_log(GPR_DEBUG, "elapsed:%d.%09d min:%d.%09d", (int)elapsed.tv_sec,
elapsed.tv_nsec, (int)t->ping_policy.min_time_between_pings.tv_sec,
(int)t->ping_policy.min_time_between_pings.tv_nsec);*/
if (gpr_time_cmp(elapsed, t->ping_policy.min_time_between_pings) < 0) {
gpr_timespec next_allowed_ping =
gpr_time_add(t->ping_state.last_ping_sent_time,
t->ping_policy.min_sent_ping_interval_without_data);
if (t->keepalive_permit_without_calls == 0 &&
grpc_chttp2_stream_map_size(&t->stream_map) == 0) {
next_allowed_ping = gpr_time_add(t->ping_recv_state.last_ping_recv_time,
gpr_time_from_seconds(7200, GPR_TIMESPAN));
}
/*gpr_log(GPR_DEBUG, "next_allowed_ping:%d.%09d now:%d.%09d",
(int)next_allowed_ping.tv_sec, (int)next_allowed_pingpsed.tv_nsec,
(int)now.tv_sec, (int)now.tv_nsec);*/
if (gpr_time_cmp(next_allowed_ping, now) > 0) {
/* not enough elapsed time between successive pings */
if (GRPC_TRACER_ON(grpc_http_trace) ||
GRPC_TRACER_ON(grpc_bdp_estimator_trace)) {
@ -100,11 +99,11 @@ static void maybe_initiate_ping(grpc_exec_ctx *exec_ctx,
}
if (!t->ping_state.is_delayed_ping_timer_set) {
t->ping_state.is_delayed_ping_timer_set = true;
grpc_timer_init(exec_ctx, &t->ping_state.delayed_ping_timer,
gpr_time_add(t->ping_state.last_ping_sent_time,
t->ping_policy.min_time_between_pings),
&t->retry_initiate_ping_locked,
gpr_now(GPR_CLOCK_MONOTONIC));
grpc_timer_init(
exec_ctx, &t->ping_state.delayed_ping_timer,
gpr_time_add(t->ping_state.last_ping_sent_time,
t->ping_policy.min_sent_ping_interval_without_data),
&t->retry_initiate_ping_locked, gpr_now(GPR_CLOCK_MONOTONIC));
}
return;
}
@ -127,6 +126,12 @@ static void maybe_initiate_ping(grpc_exec_ctx *exec_ctx,
grpc_chttp2_ping_create(false, pq->inflight_id));
GRPC_STATS_INC_HTTP2_PINGS_SENT(exec_ctx);
t->ping_state.last_ping_sent_time = now;
if (GRPC_TRACER_ON(grpc_http_trace) ||
GRPC_TRACER_ON(grpc_bdp_estimator_trace)) {
gpr_log(GPR_DEBUG, "Ping sent [%p]: %d/%d", t->peer_string,
t->ping_state.pings_before_data_required,
t->ping_policy.max_pings_without_data);
}
t->ping_state.pings_before_data_required -=
(t->ping_state.pings_before_data_required != 0);
}

@ -519,7 +519,7 @@ static grpc_channel *create_client(const servers_fixture *f) {
arg_array[1].key = GRPC_ARG_LB_POLICY_NAME;
arg_array[1].value.string = "ROUND_ROBIN";
arg_array[2].type = GRPC_ARG_INTEGER;
arg_array[2].key = GRPC_ARG_HTTP2_MIN_TIME_BETWEEN_PINGS_MS;
arg_array[2].key = GRPC_ARG_HTTP2_MIN_SENT_PING_INTERVAL_WITHOUT_DATA_MS;
arg_array[2].value.integer = 0;
args.num_args = GPR_ARRAY_SIZE(arg_array);
args.args = arg_array;

@ -66,18 +66,19 @@ static void end_test(grpc_end2end_test_fixture *f) {
static void test_bad_ping(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 client_a[] = {{.type = GRPC_ARG_INTEGER,
.key = GRPC_ARG_HTTP2_MIN_TIME_BETWEEN_PINGS_MS,
.value.integer = 0},
{.type = GRPC_ARG_INTEGER,
.key = GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA,
.value.integer = 0},
{.type = GRPC_ARG_INTEGER,
.key = GRPC_ARG_HTTP2_BDP_PROBE,
.value.integer = 0}};
grpc_arg client_a[] = {
{.type = GRPC_ARG_INTEGER,
.key = GRPC_ARG_HTTP2_MIN_SENT_PING_INTERVAL_WITHOUT_DATA_MS,
.value.integer = 10},
{.type = GRPC_ARG_INTEGER,
.key = GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA,
.value.integer = 0},
{.type = GRPC_ARG_INTEGER,
.key = GRPC_ARG_HTTP2_BDP_PROBE,
.value.integer = 0}};
grpc_arg server_a[] = {
{.type = GRPC_ARG_INTEGER,
.key = GRPC_ARG_HTTP2_MIN_PING_INTERVAL_WITHOUT_DATA_MS,
.key = GRPC_ARG_HTTP2_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS,
.value.integer = 300000 /* 5 minutes */},
{.type = GRPC_ARG_INTEGER,
.key = GRPC_ARG_HTTP2_MAX_PING_STRIKES,

@ -37,18 +37,19 @@ static void test_ping(grpc_end2end_test_config config,
grpc_connectivity_state state = GRPC_CHANNEL_IDLE;
int i;
grpc_arg client_a[] = {{.type = GRPC_ARG_INTEGER,
.key = GRPC_ARG_HTTP2_MIN_TIME_BETWEEN_PINGS_MS,
.value.integer = 0},
{.type = GRPC_ARG_INTEGER,
.key = GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA,
.value.integer = 0},
{.type = GRPC_ARG_INTEGER,
.key = GRPC_ARG_KEEPALIVE_PERMIT_WITHOUT_CALLS,
.value.integer = 1}};
grpc_arg client_a[] = {
{.type = GRPC_ARG_INTEGER,
.key = GRPC_ARG_HTTP2_MIN_SENT_PING_INTERVAL_WITHOUT_DATA_MS,
.value.integer = 10},
{.type = GRPC_ARG_INTEGER,
.key = GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA,
.value.integer = 0},
{.type = GRPC_ARG_INTEGER,
.key = GRPC_ARG_KEEPALIVE_PERMIT_WITHOUT_CALLS,
.value.integer = 1}};
grpc_arg server_a[] = {
{.type = GRPC_ARG_INTEGER,
.key = GRPC_ARG_HTTP2_MIN_PING_INTERVAL_WITHOUT_DATA_MS,
.key = GRPC_ARG_HTTP2_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS,
.value.integer = 0},
{.type = GRPC_ARG_INTEGER,
.key = GRPC_ARG_KEEPALIVE_PERMIT_WITHOUT_CALLS,

Loading…
Cancel
Save