diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c index c90ff6a01dd45d49c2c1745cecf1bf086bb8a587..73f9454f7a49f0e49b00613aab05ea67ce13a39d 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c @@ -148,6 +148,8 @@ static void send_ping_locked(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, grpc_chttp2_ping_type ping_type, grpc_closure *on_initiate, grpc_closure *on_complete); +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 @@ -273,6 +275,8 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, grpc_closure_init(&t->destructive_reclaimer_locked, destructive_reclaimer_locked, t, grpc_combiner_scheduler(t->combiner, false)); + grpc_closure_init(&t->retry_initiate_ping_locked, retry_initiate_ping_locked, + t, grpc_combiner_scheduler(t->combiner, false)); grpc_closure_init(&t->start_bdp_ping_locked, start_bdp_ping_locked, t, grpc_combiner_scheduler(t->combiner, false)); grpc_closure_init(&t->finish_bdp_ping_locked, finish_bdp_ping_locked, t, @@ -482,6 +486,7 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, t->ping_state.pings_before_data_required = t->ping_policy.max_pings_without_data; + t->ping_state.is_delayed_ping_timer_set = false; /** Start client-side keepalive pings */ if (t->is_client) { @@ -1407,6 +1412,13 @@ static void send_ping_locked(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, } } +static void retry_initiate_ping_locked(grpc_exec_ctx *exec_ctx, void *tp, + grpc_error *error) { + grpc_chttp2_transport *t = tp; + t->ping_state.is_delayed_ping_timer_set = false; + grpc_chttp2_initiate_write(exec_ctx, t, false, "retry_send_ping"); +} + void grpc_chttp2_ack_ping(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t, uint64_t id) { grpc_chttp2_ping_queue *pq = @@ -2185,9 +2197,7 @@ static void finish_keepalive_ping_locked(grpc_exec_ctx *exec_ctx, void *arg, grpc_timer_init( exec_ctx, &t->keepalive_ping_timer, gpr_time_add(gpr_now(GPR_CLOCK_MONOTONIC), t->keepalive_time), - grpc_closure_create(init_keepalive_ping_locked, t, - grpc_combiner_scheduler(t->combiner, false)), - gpr_now(GPR_CLOCK_MONOTONIC)); + &t->init_keepalive_ping_locked, gpr_now(GPR_CLOCK_MONOTONIC)); } } GRPC_CHTTP2_UNREF_TRANSPORT(exec_ctx, t, "keepalive ping end"); diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h index 891ae2f281229489af42cea83ab10800b051c840..8b718e963cdcf623d2cd79986b2c317399e396c7 100644 --- a/src/core/ext/transport/chttp2/transport/internal.h +++ b/src/core/ext/transport/chttp2/transport/internal.h @@ -102,6 +102,8 @@ typedef struct { typedef struct { gpr_timespec last_ping_sent_time; int pings_before_data_required; + grpc_timer delayed_ping_timer; + bool is_delayed_ping_timer_set; } grpc_chttp2_repeated_ping_state; /* deframer state for the overall http2 stream of bytes */ @@ -308,6 +310,7 @@ struct grpc_chttp2_transport { grpc_chttp2_repeated_ping_policy ping_policy; grpc_chttp2_repeated_ping_state ping_state; uint64_t ping_ctr; /* unique id for pings */ + grpc_closure retry_initiate_ping_locked; /** ping acks */ size_t ping_ack_count; diff --git a/src/core/ext/transport/chttp2/transport/writing.c b/src/core/ext/transport/chttp2/transport/writing.c index 2b9d93cae7cc0b3db995d081c3684a57e4509891..0869056f56d3c5994bb67808ddd6db6c0073f241 100644 --- a/src/core/ext/transport/chttp2/transport/writing.c +++ b/src/core/ext/transport/chttp2/transport/writing.c @@ -101,6 +101,14 @@ static void maybe_initiate_ping(grpc_exec_ctx *exec_ctx, "Ping delayed [%p]: not enough time elapsed since last ping", t->peer_string); } + 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)); + } return; } /* coalesce equivalent pings into this one */ diff --git a/test/core/end2end/tests/ping.c b/test/core/end2end/tests/ping.c index f5bfac2255c8c69d87485640dab4a666af3d3f71..082ac641f0bb98a0344e847c54a0a64f1e12b46c 100644 --- a/test/core/end2end/tests/ping.c +++ b/test/core/end2end/tests/ping.c @@ -41,9 +41,12 @@ #include "test/core/end2end/cq_verifier.h" +#define PING_NUM 5 + static void *tag(intptr_t t) { return (void *)t; } -static void test_ping(grpc_end2end_test_config config) { +static void test_ping(grpc_end2end_test_config config, + int min_time_between_pings_ms) { grpc_end2end_test_fixture f = config.create_fixture(NULL, NULL); cq_verifier *cqv = cq_verifier_create(f.cq); grpc_connectivity_state state = GRPC_CHANNEL_IDLE; @@ -51,7 +54,7 @@ static void test_ping(grpc_end2end_test_config config) { grpc_arg a[] = {{.type = GRPC_ARG_INTEGER, .key = GRPC_ARG_HTTP2_MIN_TIME_BETWEEN_PINGS_MS, - .value.integer = 0}, + .value.integer = min_time_between_pings_ms}, {.type = GRPC_ARG_INTEGER, .key = GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA, .value.integer = 20}}; @@ -70,7 +73,11 @@ static void test_ping(grpc_end2end_test_config config) { 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)); + f.client, state, + gpr_time_add(grpc_timeout_seconds_to_deadline(3), + gpr_time_from_millis(min_time_between_pings_ms * PING_NUM, + GPR_TIMESPAN)), + f.cq, tag(99)); CQ_EXPECT_COMPLETION(cqv, tag(99), 1); cq_verify(cqv); state = grpc_channel_check_connectivity_state(f.client, 0); @@ -79,7 +86,7 @@ static void test_ping(grpc_end2end_test_config config) { state == GRPC_CHANNEL_TRANSIENT_FAILURE); } - for (i = 1; i <= 5; i++) { + for (i = 1; i <= PING_NUM; i++) { grpc_channel_ping(f.client, f.cq, tag(i), NULL); CQ_EXPECT_COMPLETION(cqv, tag(i), 1); cq_verify(cqv); @@ -102,7 +109,8 @@ static void test_ping(grpc_end2end_test_config config) { void ping(grpc_end2end_test_config config) { GPR_ASSERT(config.feature_mask & FEATURE_MASK_SUPPORTS_DELAYED_CONNECTION); - test_ping(config); + test_ping(config, 0); + test_ping(config, 100); } void ping_pre_init(void) {}