Merge pull request #8219 from markdroth/fail_fast

Change C++ API to expose wait_for_ready instead of fail_fast.

doc/core/pending_api_cleanups.md

@@ -15,3 +15,5 @@ number:
   `include/grpc/impl/codegen/grpc_types.h` (commit `af00d8b`)
 - remove `ServerBuilder::SetMaxMessageSize()` method from
   `include/grpc++/server_builder.h` (commit `6980362`)
+- remove `GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY` macro from
+  `include/grpc/impl/codegen/grpc_types.h` (commit `59c9f90`)

doc/cpp/pending_api_cleanups.md

@@ -0,0 +1,15 @@
+There are times when we make changes that include a temporary shim for
+backward-compatibility (e.g., a macro or some other function to preserve
+the original API) to avoid having to bump the major version number in
+the next release.  However, when we do eventually want to release a
+feature that does change the API in a non-backward-compatible way, we
+will wind up bumping the major version number anyway, at which point we
+can take the opportunity to clean up any pending backward-compatibility
+shims.
+
+This file lists all pending backward-compatibility changes that should
+be cleaned up the next time we are going to bump the major version
+number:
+
+- remove `ClientContext::set_fail_fast()` method from
+  `include/grpc++/impl/codegen/client_context.h` (commit `9477724`)

include/grpc++/impl/codegen/client_context.h

@@ -226,8 +226,14 @@ class ClientContext {
   /// EXPERIMENTAL: Set this request to be cacheable
   void set_cacheable(bool cacheable) { cacheable_ = cacheable; }
 
-  /// EXPERIMENTAL: Trigger fail-fast or not on this request
+  /// EXPERIMENTAL: Trigger wait-for-ready or not on this request
-  void set_fail_fast(bool fail_fast) { fail_fast_ = fail_fast; }
+  void set_wait_for_ready(bool wait_for_ready) {
+    wait_for_ready_ = wait_for_ready;
+    wait_for_ready_explicitly_set_ = true;
+  }
+
+  /// DEPRECATED: Use set_wait_for_ready() instead.
+  void set_fail_fast(bool fail_fast) { set_wait_for_ready(!fail_fast); }
 
 #ifndef GRPC_CXX0X_NO_CHRONO
   /// Return the deadline for the client call.
@@ -347,14 +353,18 @@ class ClientContext {
 
   uint32_t initial_metadata_flags() const {
     return (idempotent_ ? GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST : 0) |
-           (fail_fast_ ? 0 : GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY) |
+           (wait_for_ready_ ? GRPC_INITIAL_METADATA_WAIT_FOR_READY : 0) |
-           (cacheable_ ? GRPC_INITIAL_METADATA_CACHEABLE_REQUEST : 0);
+           (cacheable_ ? GRPC_INITIAL_METADATA_CACHEABLE_REQUEST : 0) |
+           (wait_for_ready_explicitly_set_
+                ? GRPC_INITIAL_METADATA_WAIT_FOR_READY_EXPLICITLY_SET
+                : 0);
   }
 
   grpc::string authority() { return authority_; }
 
   bool initial_metadata_received_;
-  bool fail_fast_;
+  bool wait_for_ready_;
+  bool wait_for_ready_explicitly_set_;
   bool idempotent_;
   bool cacheable_;
   std::shared_ptr<Channel> channel_;

include/grpc/impl/codegen/grpc_types.h

@@ -257,15 +257,22 @@ typedef enum grpc_call_error {
 /** Signal that the call is idempotent */
 #define GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST (0x00000010u)
 /** Signal that the call should not return UNAVAILABLE before it has started */
-#define GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY (0x00000020u)
+#define GRPC_INITIAL_METADATA_WAIT_FOR_READY (0x00000020u)
+/** DEPRECATED: for backward compatibility */
+#define GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY \
+  GRPC_INITIAL_METADATA_WAIT_FOR_READY
 /** Signal that the call is cacheable. GRPC is free to use GET verb */
 #define GRPC_INITIAL_METADATA_CACHEABLE_REQUEST (0x00000040u)
+/** Signal that GRPC_INITIAL_METADATA_WAIT_FOR_READY was explicitly set
+    by the calling application. */
+#define GRPC_INITIAL_METADATA_WAIT_FOR_READY_EXPLICITLY_SET (0x00000080u)
 
 /** Mask of all valid flags */
-#define GRPC_INITIAL_METADATA_USED_MASK        \
+#define GRPC_INITIAL_METADATA_USED_MASK       \
-  (GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST |  \
+  (GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST | \
-   GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY | \
+   GRPC_INITIAL_METADATA_WAIT_FOR_READY |     \
-   GRPC_INITIAL_METADATA_CACHEABLE_REQUEST)
+   GRPC_INITIAL_METADATA_CACHEABLE_REQUEST |  \
+   GRPC_INITIAL_METADATA_WAIT_FOR_READY_EXPLICITLY_SET)
 
 /** A single metadata element */
 typedef struct grpc_metadata {

src/core/ext/client_config/client_channel.c

@@ -111,10 +111,10 @@ static void set_channel_connectivity_state_locked(grpc_exec_ctx *exec_ctx,
   if ((state == GRPC_CHANNEL_TRANSIENT_FAILURE ||
        state == GRPC_CHANNEL_SHUTDOWN) &&
       chand->lb_policy != NULL) {
-    /* cancel fail-fast picks */
+    /* cancel picks with wait_for_ready=false */
     grpc_lb_policy_cancel_picks(
         exec_ctx, chand->lb_policy,
-        /* mask= */ GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY,
+        /* mask= */ GRPC_INITIAL_METADATA_WAIT_FOR_READY,
         /* check= */ 0, GRPC_ERROR_REF(error));
   }
   grpc_connectivity_state_set(exec_ctx, &chand->state_tracker, state, error,

src/cpp/client/client_context.cc

@@ -59,7 +59,8 @@ static ClientContext::GlobalCallbacks* g_client_callbacks =
 
 ClientContext::ClientContext()
     : initial_metadata_received_(false),
-      fail_fast_(true),
+      wait_for_ready_(false),
+      wait_for_ready_explicitly_set_(false),
       idempotent_(false),
       cacheable_(false),
       call_(nullptr),

test/core/bad_ssl/bad_ssl_test.c

@@ -88,7 +88,7 @@ static void run_test(const char *target, size_t nops) {
   op = ops;
   op->op = GRPC_OP_SEND_INITIAL_METADATA;
   op->data.send_initial_metadata.count = 0;
-  op->flags = GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY;
+  op->flags = GRPC_INITIAL_METADATA_WAIT_FOR_READY;
   op->reserved = NULL;
   op++;
   op->op = GRPC_OP_RECV_STATUS_ON_CLIENT;

test/core/end2end/connection_refused_test.c

@@ -44,7 +44,7 @@
 
 static void *tag(intptr_t i) { return (void *)i; }
 
-static void run_test(bool fail_fast) {
+static void run_test(bool wait_for_ready) {
   grpc_channel *chan;
   grpc_call *call;
   gpr_timespec deadline = GRPC_TIMEOUT_SECONDS_TO_DEADLINE(2);
@@ -57,7 +57,7 @@ static void run_test(bool fail_fast) {
   char *details = NULL;
   size_t details_capacity = 0;
 
-  gpr_log(GPR_INFO, "TEST: fail_fast=%d", fail_fast);
+  gpr_log(GPR_INFO, "TEST: wait_for_ready=%d", wait_for_ready);
 
   grpc_init();
 
@@ -81,7 +81,7 @@ static void run_test(bool fail_fast) {
   op = ops;
   op->op = GRPC_OP_SEND_INITIAL_METADATA;
   op->data.send_initial_metadata.count = 0;
-  op->flags = fail_fast ? 0 : GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY;
+  op->flags = wait_for_ready ? GRPC_INITIAL_METADATA_WAIT_FOR_READY : 0;
   op->reserved = NULL;
   op++;
   op->op = GRPC_OP_RECV_STATUS_ON_CLIENT;
@@ -98,10 +98,10 @@ static void run_test(bool fail_fast) {
   CQ_EXPECT_COMPLETION(cqv, tag(1), 1);
   cq_verify(cqv);
 
-  if (fail_fast) {
+  if (wait_for_ready) {
-    GPR_ASSERT(status == GRPC_STATUS_UNAVAILABLE);
-  } else {
     GPR_ASSERT(status == GRPC_STATUS_DEADLINE_EXCEEDED);
+  } else {
+    GPR_ASSERT(status == GRPC_STATUS_UNAVAILABLE);
   }
 
   grpc_completion_queue_shutdown(cq);
@@ -122,7 +122,7 @@ static void run_test(bool fail_fast) {
 
 int main(int argc, char **argv) {
   grpc_test_init(argc, argv);
-  run_test(true);
   run_test(false);
+  run_test(true);
   return 0;
 }

test/core/end2end/dualstack_socket_test.c

@@ -171,7 +171,7 @@ void test_connect(const char *server_host, const char *client_host, int port,
   op = ops;
   op->op = GRPC_OP_SEND_INITIAL_METADATA;
   op->data.send_initial_metadata.count = 0;
-  op->flags = expect_ok ? GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY : 0;
+  op->flags = expect_ok ? GRPC_INITIAL_METADATA_WAIT_FOR_READY : 0;
   op->reserved = NULL;
   op++;
   op->op = GRPC_OP_SEND_CLOSE_FROM_CLIENT;

test/core/end2end/tests/simple_delayed_request.c

@@ -119,7 +119,7 @@ static void simple_delayed_request_body(grpc_end2end_test_config config,
   op = ops;
   op->op = GRPC_OP_SEND_INITIAL_METADATA;
   op->data.send_initial_metadata.count = 0;
-  op->flags = GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY;
+  op->flags = GRPC_INITIAL_METADATA_WAIT_FOR_READY;
   op->reserved = NULL;
   op++;
   op->op = GRPC_OP_SEND_CLOSE_FROM_CLIENT;

test/cpp/end2end/client_crash_test.cc

@@ -89,7 +89,7 @@ TEST_F(CrashTest, KillBeforeWrite) {
   EchoRequest request;
   EchoResponse response;
   ClientContext context;
-  context.set_fail_fast(false);
+  context.set_wait_for_ready(true);
 
   auto stream = stub->BidiStream(&context);
 
@@ -115,7 +115,7 @@ TEST_F(CrashTest, KillAfterWrite) {
   EchoRequest request;
   EchoResponse response;
   ClientContext context;
-  context.set_fail_fast(false);
+  context.set_wait_for_ready(true);
 
   auto stream = stub->BidiStream(&context);
 

test/cpp/end2end/hybrid_end2end_test.cc

@@ -261,7 +261,7 @@ class HybridEnd2endTest : public ::testing::Test {
     EchoRequest send_request;
     EchoResponse recv_response;
     ClientContext cli_ctx;
-    cli_ctx.set_fail_fast(false);
+    cli_ctx.set_wait_for_ready(true);
     send_request.set_message("Hello");
     Status recv_status = stub_->Echo(&cli_ctx, send_request, &recv_response);
     EXPECT_EQ(send_request.message(), recv_response.message());
@@ -275,7 +275,7 @@ class HybridEnd2endTest : public ::testing::Test {
     EchoRequest send_request;
     EchoResponse recv_response;
     ClientContext cli_ctx;
-    cli_ctx.set_fail_fast(false);
+    cli_ctx.set_wait_for_ready(true);
     send_request.set_message("Hello");
     Status recv_status = stub->Echo(&cli_ctx, send_request, &recv_response);
     EXPECT_EQ(send_request.message() + "_dup", recv_response.message());
@@ -287,7 +287,7 @@ class HybridEnd2endTest : public ::testing::Test {
     EchoResponse recv_response;
     grpc::string expected_message;
     ClientContext cli_ctx;
-    cli_ctx.set_fail_fast(false);
+    cli_ctx.set_wait_for_ready(true);
     send_request.set_message("Hello");
     auto stream = stub_->RequestStream(&cli_ctx, &recv_response);
     for (int i = 0; i < 5; i++) {
@@ -304,7 +304,7 @@ class HybridEnd2endTest : public ::testing::Test {
     EchoRequest request;
     EchoResponse response;
     ClientContext context;
-    context.set_fail_fast(false);
+    context.set_wait_for_ready(true);
     request.set_message("hello");
 
     auto stream = stub_->ResponseStream(&context, request);
@@ -324,7 +324,7 @@ class HybridEnd2endTest : public ::testing::Test {
     EchoRequest request;
     EchoResponse response;
     ClientContext context;
-    context.set_fail_fast(false);
+    context.set_wait_for_ready(true);
     grpc::string msg("hello");
 
     auto stream = stub_->BidiStream(&context);

test/cpp/end2end/server_crash_test_client.cc

@@ -65,7 +65,7 @@ int main(int argc, char** argv) {
   EchoRequest request;
   EchoResponse response;
   grpc::ClientContext context;
-  context.set_fail_fast(false);
+  context.set_wait_for_ready(true);
 
   if (FLAGS_mode == "bidi") {
     auto stream = stub->BidiStream(&context);

test/cpp/qps/driver.cc

@@ -83,7 +83,7 @@ static std::unordered_map<string, std::deque<int>> get_hosts_and_cores(
       auto stub = WorkerService::NewStub(
           CreateChannel(*it, InsecureChannelCredentials()));
       grpc::ClientContext ctx;
-      ctx.set_fail_fast(false);
+      ctx.set_wait_for_ready(true);
       CoreRequest dummy;
       CoreResponse cores;
       grpc::Status s = stub->CoreCount(&ctx, dummy, &cores);
@@ -167,7 +167,7 @@ namespace runsc {
 static ClientContext* AllocContext(list<ClientContext>* contexts) {
   contexts->emplace_back();
   auto context = &contexts->back();
-  context->set_fail_fast(false);
+  context->set_wait_for_ready(true);
   return context;
 }
 
@@ -527,7 +527,7 @@ bool RunQuit() {
         CreateChannel(workers[i], InsecureChannelCredentials()));
     Void dummy;
     grpc::ClientContext ctx;
-    ctx.set_fail_fast(false);
+    ctx.set_wait_for_ready(true);
     Status s = stub->QuitWorker(&ctx, dummy, &dummy);
     if (!s.ok()) {
       gpr_log(GPR_ERROR, "Worker %zu could not be properly quit because %s", i,