Better ping mgmt

3 files changed, 26 insertions, 3 deletions

diff --git a/src/core/ext/transport/chttp2/transport/frame_ping.c b/src/core/ext/transport/chttp2/transport/frame_ping.c
index 66a9bc0966..20bc3107ce 100644
--- a/src/core/ext/transport/chttp2/transport/frame_ping.c
+++ b/src/core/ext/transport/chttp2/transport/frame_ping.c
@@ -101,8 +101,11 @@ grpc_error *grpc_chttp2_ping_parser_parse(grpc_exec_ctx *exec_ctx, void *parser,
     if (p->is_ack) {
       grpc_chttp2_ack_ping(exec_ctx, t, p->opaque_8bytes);
     } else {
-      grpc_slice_buffer_add(&t->qbuf,
-                            grpc_chttp2_ping_create(1, p->opaque_8bytes));
+      if (t->ping_ack_count == t->ping_ack_capacity) {
+        t->ping_ack_capacity = GPR_MAX(t->ping_ack_capacity * 3 / 2, 3);
+        t->ping_acks = gpr_realloc(t->ping_acks, t->ping_ack_capacity);
+      }
+      t->ping_acks[t->ping_ack_count++] = p->opaque_8bytes;
       grpc_chttp2_initiate_write(exec_ctx, t, false, "ping response");
     }
   }
diff --git a/src/core/ext/transport/chttp2/transport/internal.h b/src/core/ext/transport/chttp2/transport/internal.h
index dfcb296ba3..6003eebe8d 100644
--- a/src/core/ext/transport/chttp2/transport/internal.h
+++ b/src/core/ext/transport/chttp2/transport/internal.h
@@ -303,6 +303,11 @@ struct grpc_chttp2_transport {
   grpc_chttp2_repeated_ping_state ping_state;
   uint64_t ping_ctr; /* unique id for pings */
 
+  /** ping acks */
+  size_t ping_ack_count;
+  size_t ping_ack_capacity;
+  uint64_t *ping_acks;
+
   /** parser for headers */
   grpc_chttp2_hpack_parser hpack_parser;
   /** simple one shot parsers */
diff --git a/src/core/ext/transport/chttp2/transport/writing.c b/src/core/ext/transport/chttp2/transport/writing.c
index 5df979d59a..343bc7aaae 100644
--- a/src/core/ext/transport/chttp2/transport/writing.c
+++ b/src/core/ext/transport/chttp2/transport/writing.c
@@ -74,17 +74,22 @@ static void maybe_initiate_ping(grpc_exec_ctx *exec_ctx,
   }
   if (!grpc_closure_list_empty(pq->lists[GRPC_CHTTP2_PCL_INFLIGHT])) {
     /* ping already in-flight: wait */
+    gpr_log(GPR_DEBUG, "already pinging");
     return;
   }
-  if (t->ping_state.pings_before_data_required > 0 &&
+  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 */
+    gpr_log(GPR_DEBUG, "too many pings: %d/%d",
+            t->ping_state.pings_before_data_required,
+            t->ping_policy.max_pings_without_data);
     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 */
+    gpr_log(GPR_DEBUG, "not enough time");
     return;
   }
   /* coalesce equivalent pings into this one */
@@ -201,6 +206,8 @@ bool grpc_chttp2_begin_write(grpc_exec_ctx *exec_ctx,
       grpc_slice_buffer_add(&t->outbuf,
                             grpc_chttp2_window_update_create(
                                 s->id, s->announce_window, &s->stats.outgoing));
+      t->ping_state.pings_before_data_required =
+          t->ping_policy.max_pings_without_data;
       GRPC_CHTTP2_FLOW_DEBIT_STREAM("write", t, s, announce_window, announce);
     }
     if (sent_initial_metadata) {
@@ -290,6 +297,12 @@ bool grpc_chttp2_begin_write(grpc_exec_ctx *exec_ctx,
     }
   }
 
+  for (size_t i = 0; i < t->ping_ack_count; i++) {
+    grpc_slice_buffer_add(&t->outbuf,
+                          grpc_chttp2_ping_create(1, t->ping_acks[i]));
+  }
+  t->ping_ack_count = 0;
+
   /* if the grpc_chttp2_transport is ready to send a window update, do so here
      also; 3/4 is a magic number that will likely get tuned soon */
   uint32_t target_incoming_window = GPR_MAX(
@@ -307,6 +320,8 @@ bool grpc_chttp2_begin_write(grpc_exec_ctx *exec_ctx,
     grpc_transport_one_way_stats throwaway_stats;
     grpc_slice_buffer_add(&t->outbuf, grpc_chttp2_window_update_create(
                                           0, announced, &throwaway_stats));
+    t->ping_state.pings_before_data_required =
+        t->ping_policy.max_pings_without_data;
   }
 
   maybe_initiate_ping(exec_ctx, t, GRPC_CHTTP2_PING_ON_NEXT_WRITE);