Merge pull request #11587 from y-zeng/pure_ping

Fix the client-side ping policy

10 files changed, 148 insertions, 87 deletions

diff --git a/include/grpc/impl/codegen/grpc_types.h b/include/grpc/impl/codegen/grpc_types.h
index 748dc717a3..90f03f49a3 100644
--- a/include/grpc/impl/codegen/grpc_types.h
+++ b/include/grpc/impl/codegen/grpc_types.h
@@ -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"
diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.c b/src/core/ext/transport/chttp2/transport/chttp2_transport.c
index 79a9ed827f..144780b67b 100644
--- a/src/core/ext/transport/chttp2/transport/chttp2_transport.c
+++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.c
@@ -64,6 +64,11 @@
 #define DEFAULT_KEEPALIVE_PERMIT_WITHOUT_CALLS false
 #define KEEPALIVE_TIME_BACKOFF_MULTIPLIER 2
 
+#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 0                      /* unlimited */
+#define DEFAULT_MAX_PING_STRIKES 2
+
 static int g_default_client_keepalive_time_ms =
     DEFAULT_CLIENT_KEEPALIVE_TIME_MS;
 static int g_default_client_keepalive_timeout_ms =
@@ -75,6 +80,13 @@ static int g_default_server_keepalive_timeout_ms =
 static bool g_default_keepalive_permit_without_calls =
     DEFAULT_KEEPALIVE_PERMIT_WITHOUT_CALLS;
 
+static int g_default_min_sent_ping_interval_without_data_ms =
+    DEFAULT_MIN_SENT_PING_INTERVAL_WITHOUT_DATA_MS;
+static int g_default_min_recv_ping_interval_without_data_ms =
+    DEFAULT_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS;
+static int g_default_max_pings_without_data = DEFAULT_MAX_PINGS_BETWEEN_DATA;
+static int g_default_max_ping_strikes = DEFAULT_MAX_PING_STRIKES;
+
 #define MAX_CLIENT_STREAM_ID 0x7fffffffu
 grpc_tracer_flag grpc_http_trace = GRPC_TRACER_INITIALIZER(false, "http");
 grpc_tracer_flag grpc_flowctl_trace = GRPC_TRACER_INITIALIZER(false, "flowctl");
@@ -152,11 +164,6 @@ 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_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,
                                        grpc_error *error);
@@ -363,12 +370,12 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,
                        GRPC_CHTTP2_SETTINGS_GRPC_ALLOW_TRUE_BINARY_METADATA, 1);
 
   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),
-      .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),
+      .max_pings_without_data = g_default_max_pings_without_data,
+      .min_sent_ping_interval_without_data = gpr_time_from_millis(
+          g_default_min_sent_ping_interval_without_data_ms, GPR_TIMESPAN),
+      .max_ping_strikes = g_default_max_ping_strikes,
+      .min_recv_ping_interval_without_data = gpr_time_from_millis(
+          g_default_min_recv_ping_interval_without_data_ms, GPR_TIMESPAN),
   };
 
   /* Keepalive setting */
@@ -428,29 +435,37 @@ static void init_transport(grpc_exec_ctx *exec_ctx, grpc_chttp2_transport *t,
                              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});
+            (grpc_integer_options){g_default_max_pings_without_data, 0,
+                                   INT_MAX});
       } else if (0 == strcmp(channel_args->args[i].key,
                              GRPC_ARG_HTTP2_MAX_PING_STRIKES)) {
         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);
+            (grpc_integer_options){g_default_max_ping_strikes, 0, INT_MAX});
       } 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){
+                        g_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){
+                        g_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(
@@ -557,8 +572,8 @@ 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;
+  /* No pings allowed before receiving a header or data frame. */
+  t->ping_state.pings_before_data_required = 0;
   t->ping_state.is_delayed_ping_timer_set = false;
 
   t->ping_recv_state.last_ping_recv_time = gpr_inf_past(GPR_CLOCK_MONOTONIC);
@@ -625,6 +640,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 +1747,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,
@@ -2631,6 +2651,36 @@ void grpc_chttp2_config_default_keepalive_args(grpc_channel_args *args,
                 &args->args[i],
                 (grpc_integer_options){g_default_keepalive_permit_without_calls,
                                        0, 1});
+      } else if (0 ==
+                 strcmp(args->args[i].key, GRPC_ARG_HTTP2_MAX_PING_STRIKES)) {
+        g_default_max_ping_strikes = grpc_channel_arg_get_integer(
+            &args->args[i],
+            (grpc_integer_options){g_default_max_ping_strikes, 0, INT_MAX});
+      } else if (0 == strcmp(args->args[i].key,
+                             GRPC_ARG_HTTP2_MAX_PINGS_WITHOUT_DATA)) {
+        g_default_max_pings_without_data = grpc_channel_arg_get_integer(
+            &args->args[i], (grpc_integer_options){
+                                g_default_max_pings_without_data, 0, INT_MAX});
+      } else if (0 ==
+                 strcmp(
+                     args->args[i].key,
+                     GRPC_ARG_HTTP2_MIN_SENT_PING_INTERVAL_WITHOUT_DATA_MS)) {
+        g_default_min_sent_ping_interval_without_data_ms =
+            grpc_channel_arg_get_integer(
+                &args->args[i],
+                (grpc_integer_options){
+                    g_default_min_sent_ping_interval_without_data_ms, 0,
+                    INT_MAX});
+      } else if (0 ==
+                 strcmp(
+                     args->args[i].key,
+                     GRPC_ARG_HTTP2_MIN_RECV_PING_INTERVAL_WITHOUT_DATA_MS)) {
+        g_default_min_recv_ping_interval_without_data_ms =
+            grpc_channel_arg_get_integer(
+                &args->args[i],
+                (grpc_integer_options){
+                    g_default_min_recv_ping_interval_without_data_ms, 0,
+                    INT_MAX});
       }
     }
   }
diff --git a/src/core/ext/transport/chttp2/transport/frame_ping.c b/src/core/ext/transport/chttp2/transport/frame_ping.c
index 81bd02ae70..d431d6b2df 100644
--- a/src/core/ext/transport/chttp2/transport/frame_ping.c
+++ b/src/core/ext/transport/chttp2/transport/frame_ping.c
@@ -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) {
diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h
index c2dfce7c9c..1682be28dd 100644
--- a/src/core/ext/transport/chttp2/transport/internal.h
+++ b/src/core/ext/transport/chttp2/transport/internal.h
@@ -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 {
diff --git a/src/core/ext/transport/chttp2/transport/parsing.c b/src/core/ext/transport/chttp2/transport/parsing.c
index 6c12c91365..3db1ad4123 100644
--- a/src/core/ext/transport/chttp2/transport/parsing.c
+++ b/src/core/ext/transport/chttp2/transport/parsing.c
@@ -383,6 +383,9 @@ error_handler:
     /* t->parser = grpc_chttp2_data_parser_parse;*/
     t->parser = grpc_chttp2_data_parser_parse;
     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 */
@@ -559,6 +562,10 @@ static grpc_error *init_header_frame_parser(grpc_exec_ctx *exec_ctx,
         (t->incoming_frame_flags & GRPC_CHTTP2_DATA_FLAG_END_STREAM) != 0;
   }
 
+  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);
   if (s == NULL) {
diff --git a/src/core/ext/transport/chttp2/transport/writing.c b/src/core/ext/transport/chttp2/transport/writing.c
index 3ded801985..7d9d4867e1 100644
--- a/src/core/ext/transport/chttp2/transport/writing.c
+++ b/src/core/ext/transport/chttp2/transport/writing.c
@@ -68,7 +68,7 @@ static void maybe_initiate_ping(grpc_exec_ctx *exec_ctx,
   }
   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 */
+    /* need to receive something of substance before sending a ping again */
     if (GRPC_TRACER_ON(grpc_http_trace) ||
         GRPC_TRACER_ON(grpc_bdp_estimator_trace)) {
       gpr_log(GPR_DEBUG, "Ping delayed [%p]: too many recent pings: %d/%d",
@@ -78,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_ping.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)) {
@@ -93,9 +100,7 @@ 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,
+                      next_allowed_ping, &t->retry_initiate_ping_locked,
                       gpr_now(GPR_CLOCK_MONOTONIC));
     }
     return;
@@ -119,6 +124,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);
 }
@@ -257,8 +268,7 @@ grpc_chttp2_begin_write_result grpc_chttp2_begin_write(
             .stats = &s->stats.outgoing};
         grpc_chttp2_encode_header(exec_ctx, &t->hpack_compressor, NULL, 0,
                                   s->send_initial_metadata, &hopt, &t->outbuf);
-        t->ping_state.pings_before_data_required =
-            t->ping_policy.max_pings_without_data;
+        now_writing = true;
         if (!t->is_client) {
           t->ping_recv_state.last_ping_recv_time =
               gpr_inf_past(GPR_CLOCK_MONOTONIC);
@@ -297,8 +307,6 @@ grpc_chttp2_begin_write_result grpc_chttp2_begin_write(
       grpc_slice_buffer_add(
           &t->outbuf, grpc_chttp2_window_update_create(s->id, stream_announce,
                                                        &s->stats.outgoing));
-      t->ping_state.pings_before_data_required =
-          t->ping_policy.max_pings_without_data;
       if (!t->is_client) {
         t->ping_recv_state.last_ping_recv_time =
             gpr_inf_past(GPR_CLOCK_MONOTONIC);
@@ -375,8 +383,6 @@ grpc_chttp2_begin_write_result grpc_chttp2_begin_write(
                                           send_bytes);
             s->sending_bytes += send_bytes;
           }
-          t->ping_state.pings_before_data_required =
-              t->ping_policy.max_pings_without_data;
           if (!t->is_client) {
             t->ping_recv_state.last_ping_recv_time =
                 gpr_inf_past(GPR_CLOCK_MONOTONIC);
@@ -487,8 +493,6 @@ grpc_chttp2_begin_write_result grpc_chttp2_begin_write(
     grpc_slice_buffer_add(
         &t->outbuf, grpc_chttp2_window_update_create(0, transport_announce,
                                                      &throwaway_stats));
-    t->ping_state.pings_before_data_required =
-        t->ping_policy.max_pings_without_data;
     if (!t->is_client) {
       t->ping_recv_state.last_ping_recv_time =
           gpr_inf_past(GPR_CLOCK_MONOTONIC);
diff --git a/test/core/client_channel/lb_policies_test.c b/test/core/client_channel/lb_policies_test.c
index f70a9fc880..ba37cd673f 100644
--- a/test/core/client_channel/lb_policies_test.c
+++ b/test/core/client_channel/lb_policies_test.c
@@ -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;
diff --git a/test/core/end2end/tests/bad_ping.c b/test/core/end2end/tests/bad_ping.c
index 12aceda688..c97d11b306 100644
--- a/test/core/end2end/tests/bad_ping.c
+++ b/test/core/end2end/tests/bad_ping.c
@@ -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 = 20},
-                         {.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,
diff --git a/test/core/end2end/tests/keepalive_timeout.c b/test/core/end2end/tests/keepalive_timeout.c
index e0ead4ab62..8d01f23c00 100644
--- a/test/core/end2end/tests/keepalive_timeout.c
+++ b/test/core/end2end/tests/keepalive_timeout.c
@@ -106,13 +106,13 @@ static void test_keepalive_timeout(grpc_end2end_test_config config) {
                                 .value.integer = 0},
                                {.type = GRPC_ARG_INTEGER,
                                 .key = GRPC_ARG_HTTP2_BDP_PROBE,
-                                .value.integer = 1}};
+                                .value.integer = 0}};
 
-  grpc_channel_args *client_args = NULL;
-  client_args = grpc_channel_args_copy_and_add(client_args, keepalive_args, 2);
+  grpc_channel_args client_args = {.num_args = GPR_ARRAY_SIZE(keepalive_args),
+                                   .args = keepalive_args};
 
   grpc_end2end_test_fixture f =
-      begin_test(config, "keepalive_timeout", client_args, NULL);
+      begin_test(config, "keepalive_timeout", &client_args, NULL);
   cq_verifier *cqv = cq_verifier_create(f.cq);
   grpc_op ops[6];
   grpc_op *op;
@@ -216,12 +216,6 @@ static void test_keepalive_timeout(grpc_end2end_test_config config) {
   grpc_byte_buffer_destroy(response_payload);
   grpc_byte_buffer_destroy(response_payload_recv);
 
-  if (client_args != NULL) {
-    grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT;
-    grpc_channel_args_destroy(&exec_ctx, client_args);
-    grpc_exec_ctx_finish(&exec_ctx);
-  }
-
   end_test(&f);
   config.tear_down_data(&f);
 }
diff --git a/test/core/end2end/tests/ping.c b/test/core/end2end/tests/ping.c
index 112ad9d7d2..23c82569ba 100644
--- a/test/core/end2end/tests/ping.c
+++ b/test/core/end2end/tests/ping.c
@@ -37,15 +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 = 20}};
+  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,