Merge pull request #14954 from markdroth/retry_fix

Fix retry code handling of internally triggered recv_trailing_metadata.

CMakeLists.txt

@@ -5262,6 +5262,7 @@ add_library(end2end_tests
   test/core/end2end/tests/retry_exceeds_buffer_size_in_initial_batch.cc
   test/core/end2end/tests/retry_exceeds_buffer_size_in_subsequent_batch.cc
   test/core/end2end/tests/retry_non_retriable_status.cc
+  test/core/end2end/tests/retry_non_retriable_status_before_recv_trailing_metadata_started.cc
   test/core/end2end/tests/retry_recv_initial_metadata.cc
   test/core/end2end/tests/retry_recv_message.cc
   test/core/end2end/tests/retry_server_pushback_delay.cc
@@ -5379,6 +5380,7 @@ add_library(end2end_nosec_tests
   test/core/end2end/tests/retry_exceeds_buffer_size_in_initial_batch.cc
   test/core/end2end/tests/retry_exceeds_buffer_size_in_subsequent_batch.cc
   test/core/end2end/tests/retry_non_retriable_status.cc
+  test/core/end2end/tests/retry_non_retriable_status_before_recv_trailing_metadata_started.cc
   test/core/end2end/tests/retry_recv_initial_metadata.cc
   test/core/end2end/tests/retry_recv_message.cc
   test/core/end2end/tests/retry_server_pushback_delay.cc

Makefile

@@ -9910,6 +9910,7 @@ LIBEND2END_TESTS_SRC = \
     test/core/end2end/tests/retry_exceeds_buffer_size_in_initial_batch.cc \
     test/core/end2end/tests/retry_exceeds_buffer_size_in_subsequent_batch.cc \
     test/core/end2end/tests/retry_non_retriable_status.cc \
+    test/core/end2end/tests/retry_non_retriable_status_before_recv_trailing_metadata_started.cc \
     test/core/end2end/tests/retry_recv_initial_metadata.cc \
     test/core/end2end/tests/retry_recv_message.cc \
     test/core/end2end/tests/retry_server_pushback_delay.cc \
@@ -10025,6 +10026,7 @@ LIBEND2END_NOSEC_TESTS_SRC = \
     test/core/end2end/tests/retry_exceeds_buffer_size_in_initial_batch.cc \
     test/core/end2end/tests/retry_exceeds_buffer_size_in_subsequent_batch.cc \
     test/core/end2end/tests/retry_non_retriable_status.cc \
+    test/core/end2end/tests/retry_non_retriable_status_before_recv_trailing_metadata_started.cc \
     test/core/end2end/tests/retry_recv_initial_metadata.cc \
     test/core/end2end/tests/retry_recv_message.cc \
     test/core/end2end/tests/retry_server_pushback_delay.cc \

gRPC-Core.podspec

@@ -1214,6 +1214,7 @@ Pod::Spec.new do |s|
                       'test/core/end2end/tests/retry_exceeds_buffer_size_in_initial_batch.cc',
                       'test/core/end2end/tests/retry_exceeds_buffer_size_in_subsequent_batch.cc',
                       'test/core/end2end/tests/retry_non_retriable_status.cc',
+                      'test/core/end2end/tests/retry_non_retriable_status_before_recv_trailing_metadata_started.cc',
                       'test/core/end2end/tests/retry_recv_initial_metadata.cc',
                       'test/core/end2end/tests/retry_recv_message.cc',
                       'test/core/end2end/tests/retry_server_pushback_delay.cc',

grpc.gyp

@@ -2624,6 +2624,7 @@
         'test/core/end2end/tests/retry_exceeds_buffer_size_in_initial_batch.cc',
         'test/core/end2end/tests/retry_exceeds_buffer_size_in_subsequent_batch.cc',
         'test/core/end2end/tests/retry_non_retriable_status.cc',
+        'test/core/end2end/tests/retry_non_retriable_status_before_recv_trailing_metadata_started.cc',
         'test/core/end2end/tests/retry_recv_initial_metadata.cc',
         'test/core/end2end/tests/retry_recv_message.cc',
         'test/core/end2end/tests/retry_server_pushback_delay.cc',
@@ -2713,6 +2714,7 @@
         'test/core/end2end/tests/retry_exceeds_buffer_size_in_initial_batch.cc',
         'test/core/end2end/tests/retry_exceeds_buffer_size_in_subsequent_batch.cc',
         'test/core/end2end/tests/retry_non_retriable_status.cc',
+        'test/core/end2end/tests/retry_non_retriable_status_before_recv_trailing_metadata_started.cc',
         'test/core/end2end/tests/retry_recv_initial_metadata.cc',
         'test/core/end2end/tests/retry_recv_message.cc',
         'test/core/end2end/tests/retry_server_pushback_delay.cc',

src/core/ext/filters/client_channel/client_channel.cc

@@ -842,10 +842,11 @@ typedef struct {
   bool completed_recv_trailing_metadata : 1;
   // State for callback processing.
   bool retry_dispatched : 1;
-  bool recv_initial_metadata_ready_deferred : 1;
-  bool recv_message_ready_deferred : 1;
+  subchannel_batch_data* recv_initial_metadata_ready_deferred_batch;
   grpc_error* recv_initial_metadata_error;
+  subchannel_batch_data* recv_message_ready_deferred_batch;
   grpc_error* recv_message_error;
+  subchannel_batch_data* recv_trailing_metadata_internal_batch;
 } subchannel_call_retry_state;
 
 // Pending batches stored in call data.
@@ -994,6 +995,39 @@ static void maybe_cache_send_ops_for_batch(call_data* calld,
   }
 }
 
+// Frees cached send_initial_metadata.
+static void free_cached_send_initial_metadata(channel_data* chand,
+                                              call_data* calld) {
+  if (grpc_client_channel_trace.enabled()) {
+    gpr_log(GPR_DEBUG,
+            "chand=%p calld=%p: destroying calld->send_initial_metadata", chand,
+            calld);
+  }
+  grpc_metadata_batch_destroy(&calld->send_initial_metadata);
+}
+
+// Frees cached send_message at index idx.
+static void free_cached_send_message(channel_data* chand, call_data* calld,
+                                     size_t idx) {
+  if (grpc_client_channel_trace.enabled()) {
+    gpr_log(GPR_DEBUG,
+            "chand=%p calld=%p: destroying calld->send_messages[%" PRIuPTR "]",
+            chand, calld, idx);
+  }
+  (*calld->send_messages)[idx]->Destroy();
+}
+
+// Frees cached send_trailing_metadata.
+static void free_cached_send_trailing_metadata(channel_data* chand,
+                                               call_data* calld) {
+  if (grpc_client_channel_trace.enabled()) {
+    gpr_log(GPR_DEBUG,
+            "chand=%p calld=%p: destroying calld->send_trailing_metadata",
+            chand, calld);
+  }
+  grpc_metadata_batch_destroy(&calld->send_trailing_metadata);
+}
+
 // Frees cached send ops that have already been completed after
 // committing the call.
 static void free_cached_send_op_data_after_commit(
@@ -1001,19 +1035,13 @@ static void free_cached_send_op_data_after_commit(
   channel_data* chand = static_cast<channel_data*>(elem->channel_data);
   call_data* calld = static_cast<call_data*>(elem->call_data);
   if (retry_state->completed_send_initial_metadata) {
-    grpc_metadata_batch_destroy(&calld->send_initial_metadata);
+    free_cached_send_initial_metadata(chand, calld);
   }
   for (size_t i = 0; i < retry_state->completed_send_message_count; ++i) {
-    if (grpc_client_channel_trace.enabled()) {
-      gpr_log(GPR_DEBUG,
-              "chand=%p calld=%p: destroying calld->send_messages[%" PRIuPTR
-              "]",
-              chand, calld, i);
-    }
-    (*calld->send_messages)[i]->Destroy();
+    free_cached_send_message(chand, calld, i);
   }
   if (retry_state->completed_send_trailing_metadata) {
-    grpc_metadata_batch_destroy(&calld->send_trailing_metadata);
+    free_cached_send_trailing_metadata(chand, calld);
   }
 }
 
@@ -1025,20 +1053,14 @@ static void free_cached_send_op_data_for_completed_batch(
   channel_data* chand = static_cast<channel_data*>(elem->channel_data);
   call_data* calld = static_cast<call_data*>(elem->call_data);
   if (batch_data->batch.send_initial_metadata) {
-    grpc_metadata_batch_destroy(&calld->send_initial_metadata);
+    free_cached_send_initial_metadata(chand, calld);
   }
   if (batch_data->batch.send_message) {
-    if (grpc_client_channel_trace.enabled()) {
-      gpr_log(GPR_DEBUG,
-              "chand=%p calld=%p: destroying calld->send_messages[%" PRIuPTR
-              "]",
-              chand, calld, retry_state->completed_send_message_count - 1);
-    }
-    (*calld->send_messages)[retry_state->completed_send_message_count - 1]
-        ->Destroy();
+    free_cached_send_message(chand, calld,
+                             retry_state->completed_send_message_count - 1);
   }
   if (batch_data->batch.send_trailing_metadata) {
-    grpc_metadata_batch_destroy(&calld->send_trailing_metadata);
+    free_cached_send_trailing_metadata(chand, calld);
   }
 }
 
@@ -1642,7 +1664,7 @@ static void recv_initial_metadata_ready(void* arg, grpc_error* error) {
               "(Trailers-Only)",
               chand, calld);
     }
-    retry_state->recv_initial_metadata_ready_deferred = true;
+    retry_state->recv_initial_metadata_ready_deferred_batch = batch_data;
     retry_state->recv_initial_metadata_error = GRPC_ERROR_REF(error);
     if (!retry_state->started_recv_trailing_metadata) {
       // recv_trailing_metadata not yet started by application; start it
@@ -1731,7 +1753,7 @@ static void recv_message_ready(void* arg, grpc_error* error) {
               "message and recv_trailing_metadata pending)",
               chand, calld);
     }
-    retry_state->recv_message_ready_deferred = true;
+    retry_state->recv_message_ready_deferred_batch = batch_data;
     retry_state->recv_message_error = GRPC_ERROR_REF(error);
     if (!retry_state->started_recv_trailing_metadata) {
       // recv_trailing_metadata not yet started by application; start it
@@ -1749,6 +1771,59 @@ static void recv_message_ready(void* arg, grpc_error* error) {
   GRPC_ERROR_UNREF(error);
 }
 
+//
+// list of closures to execute in call combiner
+//
+
+// Represents a closure that needs to run in the call combiner as part of
+// starting or completing a batch.
+typedef struct {
+  grpc_closure* closure;
+  grpc_error* error;
+  const char* reason;
+  bool free_reason = false;
+} closure_to_execute;
+
+static void execute_closures_in_call_combiner(grpc_call_element* elem,
+                                              const char* caller,
+                                              closure_to_execute* closures,
+                                              size_t num_closures) {
+  channel_data* chand = static_cast<channel_data*>(elem->channel_data);
+  call_data* calld = static_cast<call_data*>(elem->call_data);
+  // Note that the call combiner will be yielded for each closure that
+  // we schedule.  We're already running in the call combiner, so one of
+  // the closures can be scheduled directly, but the others will
+  // have to re-enter the call combiner.
+  if (num_closures > 0) {
+    if (grpc_client_channel_trace.enabled()) {
+      gpr_log(GPR_DEBUG, "chand=%p calld=%p: %s starting closure: %s", chand,
+              calld, caller, closures[0].reason);
+    }
+    GRPC_CLOSURE_SCHED(closures[0].closure, closures[0].error);
+    if (closures[0].free_reason) {
+      gpr_free(const_cast<char*>(closures[0].reason));
+    }
+    for (size_t i = 1; i < num_closures; ++i) {
+      if (grpc_client_channel_trace.enabled()) {
+        gpr_log(GPR_DEBUG,
+                "chand=%p calld=%p: %s starting closure in call combiner: %s",
+                chand, calld, caller, closures[i].reason);
+      }
+      GRPC_CALL_COMBINER_START(calld->call_combiner, closures[i].closure,
+                               closures[i].error, closures[i].reason);
+      if (closures[i].free_reason) {
+        gpr_free(const_cast<char*>(closures[i].reason));
+      }
+    }
+  } else {
+    if (grpc_client_channel_trace.enabled()) {
+      gpr_log(GPR_DEBUG, "chand=%p calld=%p: no closures to run for %s", chand,
+              calld, caller);
+    }
+    GRPC_CALL_COMBINER_STOP(calld->call_combiner, "no closures to run");
+  }
+}
+
 //
 // on_complete callback handling
 //
@@ -1777,36 +1852,35 @@ static void update_retry_state_for_completed_batch(
   }
 }
 
-// Represents a closure that needs to run as a result of a completed batch.
-typedef struct {
-  grpc_closure* closure;
-  grpc_error* error;
-  const char* reason;
-} closure_to_execute;
-
 // Adds any necessary closures for deferred recv_initial_metadata and
 // recv_message callbacks to closures, updating *num_closures as needed.
 static void add_closures_for_deferred_recv_callbacks(
     subchannel_batch_data* batch_data, subchannel_call_retry_state* retry_state,
     closure_to_execute* closures, size_t* num_closures) {
-  if (batch_data->batch.recv_trailing_metadata &&
-      retry_state->recv_initial_metadata_ready_deferred) {
-    closure_to_execute* closure = &closures[(*num_closures)++];
-    closure->closure =
-        GRPC_CLOSURE_INIT(&batch_data->recv_initial_metadata_ready,
-                          invoke_recv_initial_metadata_callback, batch_data,
-                          grpc_schedule_on_exec_ctx);
-    closure->error = retry_state->recv_initial_metadata_error;
-    closure->reason = "resuming recv_initial_metadata_ready";
-  }
-  if (batch_data->batch.recv_trailing_metadata &&
-      retry_state->recv_message_ready_deferred) {
-    closure_to_execute* closure = &closures[(*num_closures)++];
-    closure->closure = GRPC_CLOSURE_INIT(&batch_data->recv_message_ready,
-                                         invoke_recv_message_callback,
-                                         batch_data, grpc_schedule_on_exec_ctx);
-    closure->error = retry_state->recv_message_error;
-    closure->reason = "resuming recv_message_ready";
+  if (batch_data->batch.recv_trailing_metadata) {
+    // Add closure for deferred recv_initial_metadata_ready.
+    if (retry_state->recv_initial_metadata_ready_deferred_batch != nullptr) {
+      closure_to_execute* closure = &closures[(*num_closures)++];
+      closure->closure = GRPC_CLOSURE_INIT(
+          &batch_data->recv_initial_metadata_ready,
+          invoke_recv_initial_metadata_callback,
+          retry_state->recv_initial_metadata_ready_deferred_batch,
+          grpc_schedule_on_exec_ctx);
+      closure->error = retry_state->recv_initial_metadata_error;
+      closure->reason = "resuming recv_initial_metadata_ready";
+      retry_state->recv_initial_metadata_ready_deferred_batch = nullptr;
+    }
+    // Add closure for deferred recv_message_ready.
+    if (retry_state->recv_message_ready_deferred_batch != nullptr) {
+      closure_to_execute* closure = &closures[(*num_closures)++];
+      closure->closure = GRPC_CLOSURE_INIT(
+          &batch_data->recv_message_ready, invoke_recv_message_callback,
+          retry_state->recv_message_ready_deferred_batch,
+          grpc_schedule_on_exec_ctx);
+      closure->error = retry_state->recv_message_error;
+      closure->reason = "resuming recv_message_ready";
+      retry_state->recv_message_ready_deferred_batch = nullptr;
+    }
   }
 }
 
@@ -1951,6 +2025,8 @@ static void on_complete(void* arg, grpc_error* error) {
   // If we have previously completed recv_trailing_metadata, then the
   // call is finished.
   bool call_finished = retry_state->completed_recv_trailing_metadata;
+  // Record whether we were already committed before receiving this callback.
+  const bool previously_committed = calld->retry_committed;
   // Update bookkeeping in retry_state.
   update_retry_state_for_completed_batch(batch_data, retry_state);
   if (call_finished) {
@@ -1979,35 +2055,39 @@ static void on_complete(void* arg, grpc_error* error) {
       if (md_batch->idx.named.grpc_retry_pushback_ms != nullptr) {
         server_pushback_md = &md_batch->idx.named.grpc_retry_pushback_ms->md;
       }
-    } else if (retry_state->completed_recv_trailing_metadata) {
-      call_finished = true;
-    }
-    if (call_finished && grpc_client_channel_trace.enabled()) {
-      gpr_log(GPR_DEBUG, "chand=%p calld=%p: call finished, status=%s", chand,
-              calld, grpc_status_code_to_string(status));
     }
-    // If the call is finished, check if we should retry.
-    if (call_finished &&
-        maybe_retry(elem, batch_data, status, server_pushback_md)) {
-      // Unref batch_data for deferred recv_initial_metadata_ready or
-      // recv_message_ready callbacks, if any.
-      if (batch_data->batch.recv_trailing_metadata &&
-          retry_state->recv_initial_metadata_ready_deferred) {
-        batch_data_unref(batch_data);
-        GRPC_ERROR_UNREF(retry_state->recv_initial_metadata_error);
+    // If the call just finished, check if we should retry.
+    if (call_finished) {
+      if (grpc_client_channel_trace.enabled()) {
+        gpr_log(GPR_DEBUG, "chand=%p calld=%p: call finished, status=%s", chand,
+                calld, grpc_status_code_to_string(status));
       }
-      if (batch_data->batch.recv_trailing_metadata &&
-          retry_state->recv_message_ready_deferred) {
+      if (maybe_retry(elem, batch_data, status, server_pushback_md)) {
+        // Unref batch_data for deferred recv_initial_metadata_ready or
+        // recv_message_ready callbacks, if any.
+        if (batch_data->batch.recv_trailing_metadata &&
+            retry_state->recv_initial_metadata_ready_deferred_batch !=
+                nullptr) {
+          batch_data_unref(batch_data);
+          GRPC_ERROR_UNREF(retry_state->recv_initial_metadata_error);
+        }
+        if (batch_data->batch.recv_trailing_metadata &&
+            retry_state->recv_message_ready_deferred_batch != nullptr) {
+          batch_data_unref(batch_data);
+          GRPC_ERROR_UNREF(retry_state->recv_message_error);
+        }
         batch_data_unref(batch_data);
-        GRPC_ERROR_UNREF(retry_state->recv_message_error);
+        return;
       }
-      batch_data_unref(batch_data);
-      return;
+      // Not retrying, so commit the call.
+      retry_commit(elem, retry_state);
     }
   }
-  // If the call is finished or retries are committed, free cached data for
-  // send ops that we've just completed.
-  if (call_finished || calld->retry_committed) {
+  // If we were already committed before receiving this callback, free
+  // cached data for send ops that we've just completed.  (If the call has
+  // just now finished, the call to retry_commit() above will have freed all
+  // cached send ops, so we don't need to do it here.)
+  if (previously_committed) {
     free_cached_send_op_data_for_completed_batch(elem, batch_data, retry_state);
   }
   // Call not being retried.
@@ -2042,20 +2122,8 @@ static void on_complete(void* arg, grpc_error* error) {
   // Don't need batch_data anymore.
   batch_data_unref(batch_data);
   // Schedule all of the closures identified above.
-  // Note that the call combiner will be yielded for each closure that
-  // we schedule.  We're already running in the call combiner, so one of
-  // the closures can be scheduled directly, but the others will
-  // have to re-enter the call combiner.
-  if (num_closures > 0) {
-    GRPC_CLOSURE_SCHED(closures[0].closure, closures[0].error);
-    for (size_t i = 1; i < num_closures; ++i) {
-      GRPC_CALL_COMBINER_START(calld->call_combiner, closures[i].closure,
-                               closures[i].error, closures[i].reason);
-    }
-  } else {
-    GRPC_CALL_COMBINER_STOP(calld->call_combiner,
-                            "no closures to run for on_complete");
-  }
+  execute_closures_in_call_combiner(elem, "on_complete", closures,
+                                    num_closures);
 }
 
 //
@@ -2072,6 +2140,31 @@ static void start_batch_in_call_combiner(void* arg, grpc_error* ignored) {
   grpc_subchannel_call_process_op(subchannel_call, batch);
 }
 
+// Adds a closure to closures that will execute batch in the call combiner.
+static void add_closure_for_subchannel_batch(
+    call_data* calld, grpc_transport_stream_op_batch* batch,
+    closure_to_execute* closures, size_t* num_closures) {
+  batch->handler_private.extra_arg = calld->subchannel_call;
+  GRPC_CLOSURE_INIT(&batch->handler_private.closure,
+                    start_batch_in_call_combiner, batch,
+                    grpc_schedule_on_exec_ctx);
+  closure_to_execute* closure = &closures[(*num_closures)++];
+  closure->closure = &batch->handler_private.closure;
+  closure->error = GRPC_ERROR_NONE;
+  // If the tracer is enabled, we log a more detailed message, which
+  // requires dynamic allocation.  This will be freed in
+  // start_retriable_subchannel_batches().
+  if (grpc_client_channel_trace.enabled()) {
+    char* batch_str = grpc_transport_stream_op_batch_string(batch);
+    gpr_asprintf(const_cast<char**>(&closure->reason),
+                 "starting batch in call combiner: %s", batch_str);
+    gpr_free(batch_str);
+    closure->free_reason = true;
+  } else {
+    closure->reason = "start_subchannel_batch";
+  }
+}
+
 // Adds retriable send_initial_metadata op to batch_data.
 static void add_retriable_send_initial_metadata_op(
     call_data* calld, subchannel_call_retry_state* retry_state,
@@ -2227,8 +2320,12 @@ static void start_internal_recv_trailing_metadata(grpc_call_element* elem) {
       static_cast<subchannel_call_retry_state*>(
           grpc_connected_subchannel_call_get_parent_data(
               calld->subchannel_call));
-  subchannel_batch_data* batch_data = batch_data_create(elem, 1);
+  // Create batch_data with 2 refs, since this batch will be unreffed twice:
+  // once when the subchannel batch returns, and again when we actually get
+  // a recv_trailing_metadata op from the surface.
+  subchannel_batch_data* batch_data = batch_data_create(elem, 2);
   add_retriable_recv_trailing_metadata_op(calld, retry_state, batch_data);
+  retry_state->recv_trailing_metadata_internal_batch = batch_data;
   // Note: This will release the call combiner.
   grpc_subchannel_call_process_op(calld->subchannel_call, &batch_data->batch);
 }
@@ -2299,7 +2396,7 @@ static subchannel_batch_data* maybe_create_subchannel_batch_for_replay(
 // *num_batches as needed.
 static void add_subchannel_batches_for_pending_batches(
     grpc_call_element* elem, subchannel_call_retry_state* retry_state,
-    grpc_transport_stream_op_batch** batches, size_t* num_batches) {
+    closure_to_execute* closures, size_t* num_closures) {
   call_data* calld = static_cast<call_data*>(elem->call_data);
   for (size_t i = 0; i < GPR_ARRAY_SIZE(calld->pending_batches); ++i) {
     pending_batch* pending = &calld->pending_batches[i];
@@ -2342,13 +2439,37 @@ static void add_subchannel_batches_for_pending_batches(
     }
     if (batch->recv_trailing_metadata &&
         retry_state->started_recv_trailing_metadata) {
+      // If we previously completed a recv_trailing_metadata op
+      // initiated by start_internal_recv_trailing_metadata(), use the
+      // result of that instead of trying to re-start this op.
+      if (retry_state->recv_trailing_metadata_internal_batch != nullptr) {
+        // If the batch completed, then trigger the completion callback
+        // directly, so that we return the previously returned results to
+        // the application.  Otherwise, just unref the internally
+        // started subchannel batch, since we'll propagate the
+        // completion when it completes.
+        if (retry_state->completed_recv_trailing_metadata) {
+          subchannel_batch_data* batch_data =
+              retry_state->recv_trailing_metadata_internal_batch;
+          closure_to_execute* closure = &closures[(*num_closures)++];
+          closure->closure = &batch_data->on_complete;
+          // Batches containing recv_trailing_metadata always succeed.
+          closure->error = GRPC_ERROR_NONE;
+          closure->reason =
+              "re-executing on_complete for recv_trailing_metadata "
+              "to propagate internally triggered result";
+        } else {
+          batch_data_unref(retry_state->recv_trailing_metadata_internal_batch);
+        }
+        retry_state->recv_trailing_metadata_internal_batch = nullptr;
+      }
       continue;
     }
     // If we're not retrying, just send the batch as-is.
     if (calld->method_params == nullptr ||
         calld->method_params->retry_policy() == nullptr ||
         calld->retry_committed) {
-      batches[(*num_batches)++] = batch;
+      add_closure_for_subchannel_batch(calld, batch, closures, num_closures);
       pending_batch_clear(calld, pending);
       continue;
     }
@@ -2385,7 +2506,8 @@ static void add_subchannel_batches_for_pending_batches(
       GPR_ASSERT(batch->collect_stats);
       add_retriable_recv_trailing_metadata_op(calld, retry_state, batch_data);
     }
-    batches[(*num_batches)++] = &batch_data->batch;
+    add_closure_for_subchannel_batch(calld, &batch_data->batch, closures,
+                                     num_closures);
   }
 }
 
@@ -2403,62 +2525,29 @@ static void start_retriable_subchannel_batches(void* arg, grpc_error* ignored) {
       static_cast<subchannel_call_retry_state*>(
           grpc_connected_subchannel_call_get_parent_data(
               calld->subchannel_call));
+  // Construct list of closures to execute, one for each pending batch.
   // We can start up to 6 batches.
-  grpc_transport_stream_op_batch*
-      batches[GPR_ARRAY_SIZE(calld->pending_batches)];
-  size_t num_batches = 0;
+  closure_to_execute closures[GPR_ARRAY_SIZE(calld->pending_batches)];
+  size_t num_closures = 0;
   // Replay previously-returned send_* ops if needed.
   subchannel_batch_data* replay_batch_data =
       maybe_create_subchannel_batch_for_replay(elem, retry_state);
   if (replay_batch_data != nullptr) {
-    batches[num_batches++] = &replay_batch_data->batch;
+    add_closure_for_subchannel_batch(calld, &replay_batch_data->batch, closures,
+                                     &num_closures);
   }
   // Now add pending batches.
-  add_subchannel_batches_for_pending_batches(elem, retry_state, batches,
-                                             &num_batches);
+  add_subchannel_batches_for_pending_batches(elem, retry_state, closures,
+                                             &num_closures);
   // Start batches on subchannel call.
-  // Note that the call combiner will be yielded for each batch that we
-  // send down.  We're already running in the call combiner, so one of
-  // the batches can be started directly, but the others will have to
-  // re-enter the call combiner.
   if (grpc_client_channel_trace.enabled()) {
     gpr_log(GPR_DEBUG,
             "chand=%p calld=%p: starting %" PRIuPTR
             " retriable batches on subchannel_call=%p",
-            chand, calld, num_batches, calld->subchannel_call);
-  }
-  if (num_batches == 0) {
-    // This should be fairly rare, but it can happen when (e.g.) an
-    // attempt completes before it has finished replaying all
-    // previously sent messages.
-    GRPC_CALL_COMBINER_STOP(calld->call_combiner,
-                            "no retriable subchannel batches to start");
-  } else {
-    for (size_t i = 1; i < num_batches; ++i) {
-      if (grpc_client_channel_trace.enabled()) {
-        char* batch_str = grpc_transport_stream_op_batch_string(batches[i]);
-        gpr_log(GPR_DEBUG,
-                "chand=%p calld=%p: starting batch in call combiner: %s", chand,
-                calld, batch_str);
-        gpr_free(batch_str);
-      }
-      batches[i]->handler_private.extra_arg = calld->subchannel_call;
-      GRPC_CLOSURE_INIT(&batches[i]->handler_private.closure,
-                        start_batch_in_call_combiner, batches[i],
-                        grpc_schedule_on_exec_ctx);
-      GRPC_CALL_COMBINER_START(calld->call_combiner,
-                               &batches[i]->handler_private.closure,
-                               GRPC_ERROR_NONE, "start_subchannel_batch");
-    }
-    if (grpc_client_channel_trace.enabled()) {
-      char* batch_str = grpc_transport_stream_op_batch_string(batches[0]);
-      gpr_log(GPR_DEBUG, "chand=%p calld=%p: starting batch: %s", chand, calld,
-              batch_str);
-      gpr_free(batch_str);
-    }
-    // Note: This will release the call combiner.
-    grpc_subchannel_call_process_op(calld->subchannel_call, batches[0]);
+            chand, calld, num_closures, calld->subchannel_call);
   }
+  execute_closures_in_call_combiner(elem, "start_retriable_subchannel_batches",
+                                    closures, num_closures);
 }
 
 //

test/core/end2end/end2end_nosec_tests.cc

@@ -132,6 +132,8 @@ extern void retry_exceeds_buffer_size_in_subsequent_batch(grpc_end2end_test_conf
 extern void retry_exceeds_buffer_size_in_subsequent_batch_pre_init(void);
 extern void retry_non_retriable_status(grpc_end2end_test_config config);
 extern void retry_non_retriable_status_pre_init(void);
+extern void retry_non_retriable_status_before_recv_trailing_metadata_started(grpc_end2end_test_config config);
+extern void retry_non_retriable_status_before_recv_trailing_metadata_started_pre_init(void);
 extern void retry_recv_initial_metadata(grpc_end2end_test_config config);
 extern void retry_recv_initial_metadata_pre_init(void);
 extern void retry_recv_message(grpc_end2end_test_config config);
@@ -236,6 +238,7 @@ void grpc_end2end_tests_pre_init(void) {
   retry_exceeds_buffer_size_in_initial_batch_pre_init();
   retry_exceeds_buffer_size_in_subsequent_batch_pre_init();
   retry_non_retriable_status_pre_init();
+  retry_non_retriable_status_before_recv_trailing_metadata_started_pre_init();
   retry_recv_initial_metadata_pre_init();
   retry_recv_message_pre_init();
   retry_server_pushback_delay_pre_init();
@@ -320,6 +323,7 @@ void grpc_end2end_tests(int argc, char **argv,
     retry_exceeds_buffer_size_in_initial_batch(config);
     retry_exceeds_buffer_size_in_subsequent_batch(config);
     retry_non_retriable_status(config);
+    retry_non_retriable_status_before_recv_trailing_metadata_started(config);
     retry_recv_initial_metadata(config);
     retry_recv_message(config);
     retry_server_pushback_delay(config);
@@ -552,6 +556,10 @@ void grpc_end2end_tests(int argc, char **argv,
       retry_non_retriable_status(config);
       continue;
     }
+    if (0 == strcmp("retry_non_retriable_status_before_recv_trailing_metadata_started", argv[i])) {
+      retry_non_retriable_status_before_recv_trailing_metadata_started(config);
+      continue;
+    }
     if (0 == strcmp("retry_recv_initial_metadata", argv[i])) {
       retry_recv_initial_metadata(config);
       continue;

test/core/end2end/end2end_tests.cc

@@ -134,6 +134,8 @@ extern void retry_exceeds_buffer_size_in_subsequent_batch(grpc_end2end_test_conf
 extern void retry_exceeds_buffer_size_in_subsequent_batch_pre_init(void);
 extern void retry_non_retriable_status(grpc_end2end_test_config config);
 extern void retry_non_retriable_status_pre_init(void);
+extern void retry_non_retriable_status_before_recv_trailing_metadata_started(grpc_end2end_test_config config);
+extern void retry_non_retriable_status_before_recv_trailing_metadata_started_pre_init(void);
 extern void retry_recv_initial_metadata(grpc_end2end_test_config config);
 extern void retry_recv_initial_metadata_pre_init(void);
 extern void retry_recv_message(grpc_end2end_test_config config);
@@ -239,6 +241,7 @@ void grpc_end2end_tests_pre_init(void) {
   retry_exceeds_buffer_size_in_initial_batch_pre_init();
   retry_exceeds_buffer_size_in_subsequent_batch_pre_init();
   retry_non_retriable_status_pre_init();
+  retry_non_retriable_status_before_recv_trailing_metadata_started_pre_init();
   retry_recv_initial_metadata_pre_init();
   retry_recv_message_pre_init();
   retry_server_pushback_delay_pre_init();
@@ -324,6 +327,7 @@ void grpc_end2end_tests(int argc, char **argv,
     retry_exceeds_buffer_size_in_initial_batch(config);
     retry_exceeds_buffer_size_in_subsequent_batch(config);
     retry_non_retriable_status(config);
+    retry_non_retriable_status_before_recv_trailing_metadata_started(config);
     retry_recv_initial_metadata(config);
     retry_recv_message(config);
     retry_server_pushback_delay(config);
@@ -560,6 +564,10 @@ void grpc_end2end_tests(int argc, char **argv,
       retry_non_retriable_status(config);
       continue;
     }
+    if (0 == strcmp("retry_non_retriable_status_before_recv_trailing_metadata_started", argv[i])) {
+      retry_non_retriable_status_before_recv_trailing_metadata_started(config);
+      continue;
+    }
     if (0 == strcmp("retry_recv_initial_metadata", argv[i])) {
       retry_recv_initial_metadata(config);
       continue;

test/core/end2end/gen_build_yaml.py

@@ -170,6 +170,9 @@ END2END_TESTS = {
                                       proxyable=False),
     'retry_non_retriable_status': default_test_options._replace(
         cpu_cost=LOWCPU, needs_client_channel=True, proxyable=False),
+    'retry_non_retriable_status_before_recv_trailing_metadata_started':
+        default_test_options._replace(
+            cpu_cost=LOWCPU, needs_client_channel=True, proxyable=False),
     'retry_recv_initial_metadata': default_test_options._replace(
         cpu_cost=LOWCPU, needs_client_channel=True, proxyable=False),
     'retry_recv_message': default_test_options._replace(

test/core/end2end/generate_tests.bzl

@@ -158,6 +158,8 @@ END2END_TESTS = {
         needs_client_channel=True, proxyable=False),
     'retry_non_retriable_status': test_options(needs_client_channel=True,
                                                proxyable=False),
+    'retry_non_retriable_status_before_recv_trailing_metadata_started':
+        test_options(needs_client_channel=True, proxyable=False),
     'retry_recv_initial_metadata': test_options(needs_client_channel=True,
                                                 proxyable=False),
     'retry_recv_message': test_options(needs_client_channel=True,

test/core/end2end/tests/retry_non_retriable_status_before_recv_trailing_metadata_started.cc

@@ -0,0 +1,266 @@
+/*
+ *
+ * Copyright 2018 gRPC authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+#include "test/core/end2end/end2end_tests.h"
+
+#include <stdio.h>
+#include <string.h>
+
+#include <grpc/byte_buffer.h>
+#include <grpc/grpc.h>
+#include <grpc/support/alloc.h>
+#include <grpc/support/log.h>
+#include <grpc/support/string_util.h>
+#include <grpc/support/time.h>
+
+#include "src/core/lib/channel/channel_args.h"
+#include "src/core/lib/gpr/string.h"
+#include "src/core/lib/gpr/useful.h"
+#include "src/core/lib/iomgr/exec_ctx.h"
+#include "src/core/lib/transport/static_metadata.h"
+
+#include "test/core/end2end/cq_verifier.h"
+#include "test/core/end2end/tests/cancel_test_helpers.h"
+
+static void* tag(intptr_t t) { return (void*)t; }
+
+static grpc_end2end_test_fixture begin_test(grpc_end2end_test_config config,
+                                            const char* test_name,
+                                            grpc_channel_args* client_args,
+                                            grpc_channel_args* server_args) {
+  grpc_end2end_test_fixture f;
+  gpr_log(GPR_INFO, "Running test: %s/%s", test_name, config.name);
+  f = config.create_fixture(client_args, server_args);
+  config.init_server(&f, server_args);
+  config.init_client(&f, client_args);
+  return f;
+}
+
+static gpr_timespec n_seconds_from_now(int n) {
+  return grpc_timeout_seconds_to_deadline(n);
+}
+
+static gpr_timespec five_seconds_from_now(void) {
+  return n_seconds_from_now(5);
+}
+
+static void drain_cq(grpc_completion_queue* cq) {
+  grpc_event ev;
+  do {
+    ev = grpc_completion_queue_next(cq, five_seconds_from_now(), nullptr);
+  } while (ev.type != GRPC_QUEUE_SHUTDOWN);
+}
+
+static void shutdown_server(grpc_end2end_test_fixture* f) {
+  if (!f->server) return;
+  grpc_server_shutdown_and_notify(f->server, f->shutdown_cq, tag(1000));
+  GPR_ASSERT(grpc_completion_queue_pluck(f->shutdown_cq, tag(1000),
+                                         grpc_timeout_seconds_to_deadline(5),
+                                         nullptr)
+                 .type == GRPC_OP_COMPLETE);
+  grpc_server_destroy(f->server);
+  f->server = nullptr;
+}
+
+static void shutdown_client(grpc_end2end_test_fixture* f) {
+  if (!f->client) return;
+  grpc_channel_destroy(f->client);
+  f->client = nullptr;
+}
+
+static void end_test(grpc_end2end_test_fixture* f) {
+  shutdown_server(f);
+  shutdown_client(f);
+
+  grpc_completion_queue_shutdown(f->cq);
+  drain_cq(f->cq);
+  grpc_completion_queue_destroy(f->cq);
+  grpc_completion_queue_destroy(f->shutdown_cq);
+}
+
+// Tests that we don't retry for non-retryable status codes, even if
+// status is received before the recv_trailing_metadata op is started.
+// - 1 retry allowed for ABORTED status
+// - first attempt gets INVALID_ARGUMENT, so no retry is done
+static void
+test_retry_non_retriable_status_before_recv_trailing_metadata_started(
+    grpc_end2end_test_config config) {
+  grpc_call* c;
+  grpc_call* s;
+  grpc_op ops[6];
+  grpc_op* op;
+  grpc_metadata_array initial_metadata_recv;
+  grpc_metadata_array trailing_metadata_recv;
+  grpc_metadata_array request_metadata_recv;
+  grpc_call_details call_details;
+  grpc_slice request_payload_slice = grpc_slice_from_static_string("foo");
+  grpc_slice response_payload_slice = grpc_slice_from_static_string("bar");
+  grpc_byte_buffer* request_payload =
+      grpc_raw_byte_buffer_create(&request_payload_slice, 1);
+  grpc_byte_buffer* response_payload =
+      grpc_raw_byte_buffer_create(&response_payload_slice, 1);
+  grpc_byte_buffer* request_payload_recv = nullptr;
+  grpc_byte_buffer* response_payload_recv = nullptr;
+  grpc_status_code status;
+  grpc_call_error error;
+  grpc_slice details;
+  int was_cancelled = 2;
+  char* peer;
+
+  grpc_arg arg;
+  arg.type = GRPC_ARG_STRING;
+  arg.key = const_cast<char*>(GRPC_ARG_SERVICE_CONFIG);
+  arg.value.string = const_cast<char*>(
+      "{\n"
+      "  \"methodConfig\": [ {\n"
+      "    \"name\": [\n"
+      "      { \"service\": \"service\", \"method\": \"method\" }\n"
+      "    ],\n"
+      "    \"retryPolicy\": {\n"
+      "      \"maxAttempts\": 2,\n"
+      "      \"initialBackoff\": \"1s\",\n"
+      "      \"maxBackoff\": \"120s\",\n"
+      "      \"backoffMultiplier\": 1.6,\n"
+      "      \"retryableStatusCodes\": [ \"ABORTED\" ]\n"
+      "    }\n"
+      "  } ]\n"
+      "}");
+  grpc_channel_args client_args = {1, &arg};
+  grpc_end2end_test_fixture f =
+      begin_test(config, "retry_non_retriable_status", &client_args, nullptr);
+
+  cq_verifier* cqv = cq_verifier_create(f.cq);
+
+  gpr_timespec deadline = five_seconds_from_now();
+  c = grpc_channel_create_call(f.client, nullptr, GRPC_PROPAGATE_DEFAULTS, f.cq,
+                               grpc_slice_from_static_string("/service/method"),
+                               nullptr, deadline, nullptr);
+  GPR_ASSERT(c);
+
+  peer = grpc_call_get_peer(c);
+  GPR_ASSERT(peer != nullptr);
+  gpr_log(GPR_DEBUG, "client_peer_before_call=%s", peer);
+  gpr_free(peer);
+
+  grpc_metadata_array_init(&initial_metadata_recv);
+  grpc_metadata_array_init(&trailing_metadata_recv);
+  grpc_metadata_array_init(&request_metadata_recv);
+  grpc_call_details_init(&call_details);
+  grpc_slice status_details = grpc_slice_from_static_string("xyz");
+
+  memset(ops, 0, sizeof(ops));
+  op = ops;
+  op->op = GRPC_OP_SEND_INITIAL_METADATA;
+  op->data.send_initial_metadata.count = 0;
+  op++;
+  op->op = GRPC_OP_SEND_MESSAGE;
+  op->data.send_message.send_message = request_payload;
+  op++;
+  op->op = GRPC_OP_RECV_MESSAGE;
+  op->data.recv_message.recv_message = &response_payload_recv;
+  op++;
+  op->op = GRPC_OP_SEND_CLOSE_FROM_CLIENT;
+  op++;
+  op->op = GRPC_OP_RECV_INITIAL_METADATA;
+  op->data.recv_initial_metadata.recv_initial_metadata = &initial_metadata_recv;
+  op++;
+  error = grpc_call_start_batch(c, ops, (size_t)(op - ops), tag(1), nullptr);
+  GPR_ASSERT(GRPC_CALL_OK == error);
+
+  error =
+      grpc_server_request_call(f.server, &s, &call_details,
+                               &request_metadata_recv, f.cq, f.cq, tag(101));
+  GPR_ASSERT(GRPC_CALL_OK == error);
+  CQ_EXPECT_COMPLETION(cqv, tag(101), true);
+  cq_verify(cqv);
+
+  peer = grpc_call_get_peer(s);
+  GPR_ASSERT(peer != nullptr);
+  gpr_log(GPR_DEBUG, "server_peer=%s", peer);
+  gpr_free(peer);
+  peer = grpc_call_get_peer(c);
+  GPR_ASSERT(peer != nullptr);
+  gpr_log(GPR_DEBUG, "client_peer=%s", peer);
+  gpr_free(peer);
+
+  memset(ops, 0, sizeof(ops));
+  op = ops;
+  op->op = GRPC_OP_SEND_INITIAL_METADATA;
+  op->data.send_initial_metadata.count = 0;
+  op++;
+  op->op = GRPC_OP_SEND_STATUS_FROM_SERVER;
+  op->data.send_status_from_server.trailing_metadata_count = 0;
+  op->data.send_status_from_server.status = GRPC_STATUS_INVALID_ARGUMENT;
+  op->data.send_status_from_server.status_details = &status_details;
+  op++;
+  op->op = GRPC_OP_RECV_CLOSE_ON_SERVER;
+  op->data.recv_close_on_server.cancelled = &was_cancelled;
+  op++;
+  error = grpc_call_start_batch(s, ops, (size_t)(op - ops), tag(102), nullptr);
+  GPR_ASSERT(GRPC_CALL_OK == error);
+
+  CQ_EXPECT_COMPLETION(cqv, tag(102), true);
+  CQ_EXPECT_COMPLETION(cqv, tag(1), true);
+  cq_verify(cqv);
+
+  memset(ops, 0, sizeof(ops));
+  op = ops;
+  op->op = GRPC_OP_RECV_STATUS_ON_CLIENT;
+  op->data.recv_status_on_client.trailing_metadata = &trailing_metadata_recv;
+  op->data.recv_status_on_client.status = &status;
+  op->data.recv_status_on_client.status_details = &details;
+  op++;
+  error = grpc_call_start_batch(c, ops, (size_t)(op - ops), tag(2), nullptr);
+  GPR_ASSERT(GRPC_CALL_OK == error);
+
+  CQ_EXPECT_COMPLETION(cqv, tag(2), true);
+  cq_verify(cqv);
+
+  GPR_ASSERT(status == GRPC_STATUS_INVALID_ARGUMENT);
+  GPR_ASSERT(0 == grpc_slice_str_cmp(details, "xyz"));
+  GPR_ASSERT(0 == grpc_slice_str_cmp(call_details.method, "/service/method"));
+  GPR_ASSERT(0 == call_details.flags);
+  GPR_ASSERT(was_cancelled == 1);
+
+  grpc_slice_unref(details);
+  grpc_metadata_array_destroy(&initial_metadata_recv);
+  grpc_metadata_array_destroy(&trailing_metadata_recv);
+  grpc_metadata_array_destroy(&request_metadata_recv);
+  grpc_call_details_destroy(&call_details);
+  grpc_byte_buffer_destroy(request_payload);
+  grpc_byte_buffer_destroy(response_payload);
+  grpc_byte_buffer_destroy(request_payload_recv);
+  grpc_byte_buffer_destroy(response_payload_recv);
+
+  grpc_call_unref(c);
+  grpc_call_unref(s);
+
+  cq_verifier_destroy(cqv);
+
+  end_test(&f);
+  config.tear_down_data(&f);
+}
+
+void retry_non_retriable_status_before_recv_trailing_metadata_started(
+    grpc_end2end_test_config config) {
+  GPR_ASSERT(config.feature_mask & FEATURE_MASK_SUPPORTS_CLIENT_CHANNEL);
+  test_retry_non_retriable_status_before_recv_trailing_metadata_started(config);
+}
+
+void retry_non_retriable_status_before_recv_trailing_metadata_started_pre_init() {
+}

tools/run_tests/generated/sources_and_headers.json

@@ -8584,6 +8584,7 @@
       "test/core/end2end/tests/retry_exceeds_buffer_size_in_initial_batch.cc", 
       "test/core/end2end/tests/retry_exceeds_buffer_size_in_subsequent_batch.cc", 
       "test/core/end2end/tests/retry_non_retriable_status.cc", 
+      "test/core/end2end/tests/retry_non_retriable_status_before_recv_trailing_metadata_started.cc", 
       "test/core/end2end/tests/retry_recv_initial_metadata.cc", 
       "test/core/end2end/tests/retry_recv_message.cc", 
       "test/core/end2end/tests/retry_server_pushback_delay.cc", 
@@ -8682,6 +8683,7 @@
       "test/core/end2end/tests/retry_exceeds_buffer_size_in_initial_batch.cc", 
       "test/core/end2end/tests/retry_exceeds_buffer_size_in_subsequent_batch.cc", 
       "test/core/end2end/tests/retry_non_retriable_status.cc", 
+      "test/core/end2end/tests/retry_non_retriable_status_before_recv_trailing_metadata_started.cc", 
       "test/core/end2end/tests/retry_recv_initial_metadata.cc", 
       "test/core/end2end/tests/retry_recv_message.cc", 
       "test/core/end2end/tests/retry_server_pushback_delay.cc", 

tools/run_tests/generated/tests.json

@@ -8029,6 +8029,29 @@
       "posix"
     ]
   }, 
+  {
+    "args": [
+      "retry_non_retriable_status_before_recv_trailing_metadata_started"
+    ], 
+    "ci_platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ], 
+    "cpu_cost": 0.1, 
+    "exclude_configs": [], 
+    "exclude_iomgrs": [], 
+    "flaky": false, 
+    "language": "c", 
+    "name": "h2_census_test", 
+    "platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ]
+  }, 
   {
     "args": [
       "retry_recv_initial_metadata"
@@ -9735,6 +9758,29 @@
       "posix"
     ]
   }, 
+  {
+    "args": [
+      "retry_non_retriable_status_before_recv_trailing_metadata_started"
+    ], 
+    "ci_platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ], 
+    "cpu_cost": 0.1, 
+    "exclude_configs": [], 
+    "exclude_iomgrs": [], 
+    "flaky": false, 
+    "language": "c", 
+    "name": "h2_compress_test", 
+    "platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ]
+  }, 
   {
     "args": [
       "retry_recv_initial_metadata"
@@ -11413,6 +11459,28 @@
       "posix"
     ]
   }, 
+  {
+    "args": [
+      "retry_non_retriable_status_before_recv_trailing_metadata_started"
+    ], 
+    "ci_platforms": [
+      "windows", 
+      "linux", 
+      "posix"
+    ], 
+    "cpu_cost": 0.1, 
+    "exclude_configs": [], 
+    "exclude_iomgrs": [], 
+    "flaky": false, 
+    "language": "c", 
+    "name": "h2_fakesec_test", 
+    "platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ]
+  }, 
   {
     "args": [
       "retry_recv_initial_metadata"
@@ -14314,6 +14382,29 @@
       "posix"
     ]
   }, 
+  {
+    "args": [
+      "retry_non_retriable_status_before_recv_trailing_metadata_started"
+    ], 
+    "ci_platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ], 
+    "cpu_cost": 0.1, 
+    "exclude_configs": [], 
+    "exclude_iomgrs": [], 
+    "flaky": false, 
+    "language": "c", 
+    "name": "h2_full_test", 
+    "platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ]
+  }, 
   {
     "args": [
       "retry_recv_initial_metadata"
@@ -15835,6 +15926,25 @@
       "linux"
     ]
   }, 
+  {
+    "args": [
+      "retry_non_retriable_status_before_recv_trailing_metadata_started"
+    ], 
+    "ci_platforms": [
+      "linux"
+    ], 
+    "cpu_cost": 0.1, 
+    "exclude_configs": [], 
+    "exclude_iomgrs": [
+      "uv"
+    ], 
+    "flaky": false, 
+    "language": "c", 
+    "name": "h2_full+pipe_test", 
+    "platforms": [
+      "linux"
+    ]
+  }, 
   {
     "args": [
       "retry_recv_initial_metadata"
@@ -17422,6 +17532,29 @@
       "posix"
     ]
   }, 
+  {
+    "args": [
+      "retry_non_retriable_status_before_recv_trailing_metadata_started"
+    ], 
+    "ci_platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ], 
+    "cpu_cost": 0.1, 
+    "exclude_configs": [], 
+    "exclude_iomgrs": [], 
+    "flaky": false, 
+    "language": "c", 
+    "name": "h2_full+trace_test", 
+    "platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ]
+  }, 
   {
     "args": [
       "retry_recv_initial_metadata"
@@ -19151,6 +19284,29 @@
       "posix"
     ]
   }, 
+  {
+    "args": [
+      "retry_non_retriable_status_before_recv_trailing_metadata_started"
+    ], 
+    "ci_platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ], 
+    "cpu_cost": 0.1, 
+    "exclude_configs": [], 
+    "exclude_iomgrs": [], 
+    "flaky": false, 
+    "language": "c", 
+    "name": "h2_full+workarounds_test", 
+    "platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ]
+  }, 
   {
     "args": [
       "retry_recv_initial_metadata"
@@ -20951,6 +21107,30 @@
       "posix"
     ]
   }, 
+  {
+    "args": [
+      "retry_non_retriable_status_before_recv_trailing_metadata_started"
+    ], 
+    "ci_platforms": [
+      "windows", 
+      "linux", 
+      "posix"
+    ], 
+    "cpu_cost": 0.1, 
+    "exclude_configs": [], 
+    "exclude_iomgrs": [
+      "uv"
+    ], 
+    "flaky": false, 
+    "language": "c", 
+    "name": "h2_http_proxy_test", 
+    "platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ]
+  }, 
   {
     "args": [
       "retry_recv_initial_metadata"
@@ -22704,6 +22884,29 @@
       "posix"
     ]
   }, 
+  {
+    "args": [
+      "retry_non_retriable_status_before_recv_trailing_metadata_started"
+    ], 
+    "ci_platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ], 
+    "cpu_cost": 0.1, 
+    "exclude_configs": [], 
+    "exclude_iomgrs": [], 
+    "flaky": false, 
+    "language": "c", 
+    "name": "h2_load_reporting_test", 
+    "platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ]
+  }, 
   {
     "args": [
       "retry_recv_initial_metadata"
@@ -24480,6 +24683,30 @@
       "posix"
     ]
   }, 
+  {
+    "args": [
+      "retry_non_retriable_status_before_recv_trailing_metadata_started"
+    ], 
+    "ci_platforms": [
+      "windows", 
+      "linux", 
+      "posix"
+    ], 
+    "cpu_cost": 0.1, 
+    "exclude_configs": [], 
+    "exclude_iomgrs": [
+      "uv"
+    ], 
+    "flaky": false, 
+    "language": "c", 
+    "name": "h2_oauth2_test", 
+    "platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ]
+  }, 
   {
     "args": [
       "retry_recv_initial_metadata"
@@ -31159,6 +31386,29 @@
       "posix"
     ]
   }, 
+  {
+    "args": [
+      "retry_non_retriable_status_before_recv_trailing_metadata_started"
+    ], 
+    "ci_platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ], 
+    "cpu_cost": 0.1, 
+    "exclude_configs": [], 
+    "exclude_iomgrs": [], 
+    "flaky": false, 
+    "language": "c", 
+    "name": "h2_ssl_test", 
+    "platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ]
+  }, 
   {
     "args": [
       "retry_recv_initial_metadata"
@@ -34014,6 +34264,29 @@
       "posix"
     ]
   }, 
+  {
+    "args": [
+      "retry_non_retriable_status_before_recv_trailing_metadata_started"
+    ], 
+    "ci_platforms": [
+      "linux", 
+      "mac", 
+      "posix"
+    ], 
+    "cpu_cost": 0.1, 
+    "exclude_configs": [], 
+    "exclude_iomgrs": [
+      "uv"
+    ], 
+    "flaky": false, 
+    "language": "c", 
+    "name": "h2_uds_test", 
+    "platforms": [
+      "linux", 
+      "mac", 
+      "posix"
+    ]
+  }, 
   {
     "args": [
       "retry_recv_initial_metadata"
@@ -36663,6 +36936,29 @@
       "posix"
     ]
   }, 
+  {
+    "args": [
+      "retry_non_retriable_status_before_recv_trailing_metadata_started"
+    ], 
+    "ci_platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ], 
+    "cpu_cost": 0.1, 
+    "exclude_configs": [], 
+    "exclude_iomgrs": [], 
+    "flaky": false, 
+    "language": "c", 
+    "name": "h2_census_nosec_test", 
+    "platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ]
+  }, 
   {
     "args": [
       "retry_recv_initial_metadata"
@@ -38346,6 +38642,29 @@
       "posix"
     ]
   }, 
+  {
+    "args": [
+      "retry_non_retriable_status_before_recv_trailing_metadata_started"
+    ], 
+    "ci_platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ], 
+    "cpu_cost": 0.1, 
+    "exclude_configs": [], 
+    "exclude_iomgrs": [], 
+    "flaky": false, 
+    "language": "c", 
+    "name": "h2_compress_nosec_test", 
+    "platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ]
+  }, 
   {
     "args": [
       "retry_recv_initial_metadata"
@@ -41225,6 +41544,29 @@
       "posix"
     ]
   }, 
+  {
+    "args": [
+      "retry_non_retriable_status_before_recv_trailing_metadata_started"
+    ], 
+    "ci_platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ], 
+    "cpu_cost": 0.1, 
+    "exclude_configs": [], 
+    "exclude_iomgrs": [], 
+    "flaky": false, 
+    "language": "c", 
+    "name": "h2_full_nosec_test", 
+    "platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ]
+  }, 
   {
     "args": [
       "retry_recv_initial_metadata"
@@ -42727,6 +43069,25 @@
       "linux"
     ]
   }, 
+  {
+    "args": [
+      "retry_non_retriable_status_before_recv_trailing_metadata_started"
+    ], 
+    "ci_platforms": [
+      "linux"
+    ], 
+    "cpu_cost": 0.1, 
+    "exclude_configs": [], 
+    "exclude_iomgrs": [
+      "uv"
+    ], 
+    "flaky": false, 
+    "language": "c", 
+    "name": "h2_full+pipe_nosec_test", 
+    "platforms": [
+      "linux"
+    ]
+  }, 
   {
     "args": [
       "retry_recv_initial_metadata"
@@ -44291,6 +44652,29 @@
       "posix"
     ]
   }, 
+  {
+    "args": [
+      "retry_non_retriable_status_before_recv_trailing_metadata_started"
+    ], 
+    "ci_platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ], 
+    "cpu_cost": 0.1, 
+    "exclude_configs": [], 
+    "exclude_iomgrs": [], 
+    "flaky": false, 
+    "language": "c", 
+    "name": "h2_full+trace_nosec_test", 
+    "platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ]
+  }, 
   {
     "args": [
       "retry_recv_initial_metadata"
@@ -45997,6 +46381,29 @@
       "posix"
     ]
   }, 
+  {
+    "args": [
+      "retry_non_retriable_status_before_recv_trailing_metadata_started"
+    ], 
+    "ci_platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ], 
+    "cpu_cost": 0.1, 
+    "exclude_configs": [], 
+    "exclude_iomgrs": [], 
+    "flaky": false, 
+    "language": "c", 
+    "name": "h2_full+workarounds_nosec_test", 
+    "platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ]
+  }, 
   {
     "args": [
       "retry_recv_initial_metadata"
@@ -47773,6 +48180,30 @@
       "posix"
     ]
   }, 
+  {
+    "args": [
+      "retry_non_retriable_status_before_recv_trailing_metadata_started"
+    ], 
+    "ci_platforms": [
+      "windows", 
+      "linux", 
+      "posix"
+    ], 
+    "cpu_cost": 0.1, 
+    "exclude_configs": [], 
+    "exclude_iomgrs": [
+      "uv"
+    ], 
+    "flaky": false, 
+    "language": "c", 
+    "name": "h2_http_proxy_nosec_test", 
+    "platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ]
+  }, 
   {
     "args": [
       "retry_recv_initial_metadata"
@@ -49503,6 +49934,29 @@
       "posix"
     ]
   }, 
+  {
+    "args": [
+      "retry_non_retriable_status_before_recv_trailing_metadata_started"
+    ], 
+    "ci_platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ], 
+    "cpu_cost": 0.1, 
+    "exclude_configs": [], 
+    "exclude_iomgrs": [], 
+    "flaky": false, 
+    "language": "c", 
+    "name": "h2_load_reporting_nosec_test", 
+    "platforms": [
+      "windows", 
+      "linux", 
+      "mac", 
+      "posix"
+    ]
+  }, 
   {
     "args": [
       "retry_recv_initial_metadata"
@@ -55987,6 +56441,29 @@
       "posix"
     ]
   }, 
+  {
+    "args": [
+      "retry_non_retriable_status_before_recv_trailing_metadata_started"
+    ], 
+    "ci_platforms": [
+      "linux", 
+      "mac", 
+      "posix"
+    ], 
+    "cpu_cost": 0.1, 
+    "exclude_configs": [], 
+    "exclude_iomgrs": [
+      "uv"
+    ], 
+    "flaky": false, 
+    "language": "c", 
+    "name": "h2_uds_nosec_test", 
+    "platforms": [
+      "linux", 
+      "mac", 
+      "posix"
+    ]
+  }, 
   {
     "args": [
       "retry_recv_initial_metadata"