From 23cb073a93388feb244b78a7b5cf313c67610958 Mon Sep 17 00:00:00 2001 From: Yash Tibrewal Date: Wed, 2 Sep 2020 16:32:06 -0700 Subject: [PATCH] Initiate HTTP/2 writes on BDP pings too --- src/core/ext/transport/chttp2/transport/chttp2_transport.cc | 6 ++++++ src/core/ext/transport/chttp2/transport/internal.h | 1 + test/core/transport/chttp2/too_many_pings_test.cc | 3 +++ 3 files changed, 10 insertions(+) diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc index ccaf7fedbd2..8d4ff9a679e 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc @@ -857,6 +857,9 @@ static void inc_initiate_write_reason( case GRPC_CHTTP2_INITIATE_WRITE_APPLICATION_PING: GRPC_STATS_INC_HTTP2_INITIATE_WRITE_DUE_TO_APPLICATION_PING(); break; + case GRPC_CHTTP2_INITIATE_WRITE_BDP_PING: + GRPC_STATS_INC_HTTP2_INITIATE_WRITE_DUE_TO_BDP_ESTIMATOR_PING(); + break; case GRPC_CHTTP2_INITIATE_WRITE_KEEPALIVE_PING: GRPC_STATS_INC_HTTP2_INITIATE_WRITE_DUE_TO_KEEPALIVE_PING(); break; @@ -2579,6 +2582,7 @@ void schedule_bdp_ping_locked(grpc_chttp2_transport* t) { grpc_schedule_on_exec_ctx), GRPC_CLOSURE_INIT(&t->finish_bdp_ping_locked, finish_bdp_ping, t, grpc_schedule_on_exec_ctx)); + grpc_chttp2_initiate_write(t, GRPC_CHTTP2_INITIATE_WRITE_BDP_PING); } static void start_bdp_ping(void* tp, grpc_error* error) { @@ -3260,6 +3264,8 @@ const char* grpc_chttp2_initiate_write_reason_string( return "FLOW_CONTROL_UNSTALLED_BY_UPDATE"; case GRPC_CHTTP2_INITIATE_WRITE_APPLICATION_PING: return "APPLICATION_PING"; + case GRPC_CHTTP2_INITIATE_WRITE_BDP_PING: + return "BDP_PING"; case GRPC_CHTTP2_INITIATE_WRITE_KEEPALIVE_PING: return "KEEPALIVE_PING"; case GRPC_CHTTP2_INITIATE_WRITE_TRANSPORT_FLOW_CONTROL_UNSTALLED: diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h index e5bca6ad56f..7d5576cc194 100644 --- a/src/core/ext/transport/chttp2/transport/internal.h +++ b/src/core/ext/transport/chttp2/transport/internal.h @@ -101,6 +101,7 @@ typedef enum { GRPC_CHTTP2_INITIATE_WRITE_FLOW_CONTROL_UNSTALLED_BY_SETTING, GRPC_CHTTP2_INITIATE_WRITE_FLOW_CONTROL_UNSTALLED_BY_UPDATE, GRPC_CHTTP2_INITIATE_WRITE_APPLICATION_PING, + GRPC_CHTTP2_INITIATE_WRITE_BDP_PING, GRPC_CHTTP2_INITIATE_WRITE_KEEPALIVE_PING, GRPC_CHTTP2_INITIATE_WRITE_TRANSPORT_FLOW_CONTROL_UNSTALLED, GRPC_CHTTP2_INITIATE_WRITE_PING_RESPONSE, diff --git a/test/core/transport/chttp2/too_many_pings_test.cc b/test/core/transport/chttp2/too_many_pings_test.cc index 2aa77b776bb..04f5525eb9a 100644 --- a/test/core/transport/chttp2/too_many_pings_test.cc +++ b/test/core/transport/chttp2/too_many_pings_test.cc @@ -710,6 +710,8 @@ TEST(TooManyPings, BdpPingNotSentWithoutReceiveSideActivity) { ASSERT_EQ(grpc_channel_check_connectivity_state(channel, 0), GRPC_CHANNEL_READY); PerformCallWithResponsePayload(channel, server, cq); + // Wait a bit to make sure that the BDP ping goes out. + cq_verify_empty_timeout(cqv, 1); // The call with a response payload should have triggered a BDP ping. // Send two more pings to verify. The second ping should cause a disconnect. // If BDP was not sent, the second ping would not cause a disconnect. @@ -720,6 +722,7 @@ TEST(TooManyPings, BdpPingNotSentWithoutReceiveSideActivity) { grpc_channel_ping(channel, cq, tag(4), nullptr); CQ_EXPECT_COMPLETION(cqv, tag(4), 1); cq_verify(cqv, 5); + cq_verify_empty_timeout(cqv, 1); ASSERT_NE(grpc_channel_check_connectivity_state(channel, 0), GRPC_CHANNEL_READY); cq_verifier_destroy(cqv);