1 files changed, 60 insertions, 44 deletions

diff --git a/test/cpp/grpclb/grpclb_test.cc b/test/cpp/grpclb/grpclb_test.cc
index 175786332b..fcdcaba6a2 100644
--- a/test/cpp/grpclb/grpclb_test.cc
+++ b/test/cpp/grpclb/grpclb_test.cc
@@ -79,6 +79,9 @@ extern "C" {
 // - Test against a non-LB server.
 // - Random LB server closing the stream unexpectedly.
 // - Test using DNS-resolvable names (localhost?)
+// - Test handling of creation of faulty RR instance by having the LB return a
+//   serverlist with non-existent backends after having initially returned a
+//   valid one.
 //
 // Findings from end to end testing to be covered here:
 // - Handling of LB servers restart, including reconnection after backing-off
@@ -108,6 +111,7 @@ typedef struct server_fixture {
   grpc_completion_queue *cq;
   char *servers_hostport;
   int port;
+  const char *lb_token_prefix;
   gpr_thd_id tid;
   int num_calls_serviced;
 } server_fixture;
@@ -123,7 +127,8 @@ static void *tag(intptr_t t) { return (void *)t; }
 
 static grpc_slice build_response_payload_slice(
     const char *host, int *ports, size_t nports,
-    int64_t expiration_interval_secs, int32_t expiration_interval_nanos) {
+    int64_t expiration_interval_secs, int32_t expiration_interval_nanos,
+    const char *token_prefix) {
   // server_list {
   //   servers {
   //     ip_address: <in_addr/6 bytes of an IP>
@@ -150,15 +155,15 @@ static grpc_slice build_response_payload_slice(
     struct in_addr ip4;
     GPR_ASSERT(inet_pton(AF_INET, host, &ip4) == 1);
     server->set_ip_address(
-        grpc::string(reinterpret_cast<const char *>(&ip4), sizeof(ip4)));
+        string(reinterpret_cast<const char *>(&ip4), sizeof(ip4)));
     server->set_port(ports[i]);
-    // The following long long int cast is meant to work around the
-    // disfunctional implementation of std::to_string in gcc 4.4, which doesn't
-    // have a version for int but does have one for long long int.
-    string token_data = "token" + std::to_string((long long int)ports[i]);
-    server->set_load_balance_token(token_data);
+    // Missing tokens are acceptable. Test that path.
+    if (strlen(token_prefix) > 0) {
+      string token_data = token_prefix + std::to_string(ports[i]);
+      server->set_load_balance_token(token_data);
+    }
   }
-  const grpc::string &enc_resp = response.SerializeAsString();
+  const string &enc_resp = response.SerializeAsString();
   return grpc_slice_from_copied_buffer(enc_resp.data(), enc_resp.size());
 }
 
@@ -250,14 +255,14 @@ static void start_lb_server(server_fixture *sf, int *ports, size_t nports,
   for (int i = 0; i < 2; i++) {
     if (i == 0) {
       // First half of the ports.
-      response_payload_slice =
-          build_response_payload_slice("127.0.0.1", ports, nports / 2, -1, -1);
+      response_payload_slice = build_response_payload_slice(
+          "127.0.0.1", ports, nports / 2, -1, -1, sf->lb_token_prefix);
     } else {
       // Second half of the ports.
       sleep_ms(update_delay_ms);
-      response_payload_slice =
-          build_response_payload_slice("127.0.0.1", ports + (nports / 2),
-                                       (nports + 1) / 2 /* ceil */, -1, -1);
+      response_payload_slice = build_response_payload_slice(
+          "127.0.0.1", ports + (nports / 2), (nports + 1) / 2 /* ceil */, -1,
+          -1, "" /* this half doesn't get to receive an LB token */);
     }
 
     response_payload = grpc_raw_byte_buffer_create(&response_payload_slice, 1);
@@ -339,11 +344,9 @@ static void start_backend_server(server_fixture *sf) {
       return;
     }
     GPR_ASSERT(ev.type == GRPC_OP_COMPLETE);
-
-    // The following long long int cast is meant to work around the
-    // disfunctional implementation of std::to_string in gcc 4.4, which doesn't
-    // have a version for int but does have one for long long int.
-    string expected_token = "token" + std::to_string((long long int)sf->port);
+    const string expected_token =
+        strlen(sf->lb_token_prefix) == 0 ? "" : sf->lb_token_prefix +
+                                                    std::to_string(sf->port);
     GPR_ASSERT(contains_metadata(&request_metadata_recv, "lb-token",
                                  expected_token.c_str()));
 
@@ -520,6 +523,7 @@ static void perform_request(client_fixture *cf) {
 
     CQ_EXPECT_COMPLETION(cqv, tag(2), 1);
     cq_verify(cqv);
+    gpr_log(GPR_INFO, "Client after sending msg %d / 4", i + 1);
     GPR_ASSERT(byte_buffer_eq_string(response_payload_recv, PAYLOAD));
 
     grpc_byte_buffer_destroy(request_payload);
@@ -541,16 +545,17 @@ static void perform_request(client_fixture *cf) {
   cq_verify(cqv);
   peer = grpc_call_get_peer(c);
   gpr_log(GPR_INFO, "Client DONE WITH SERVER %s ", peer);
-  gpr_free(peer);
 
   grpc_call_destroy(c);
 
-  cq_verify_empty_timeout(cqv, 1);
+  cq_verify_empty_timeout(cqv, 1 /* seconds */);
   cq_verifier_destroy(cqv);
 
   grpc_metadata_array_destroy(&initial_metadata_recv);
   grpc_metadata_array_destroy(&trailing_metadata_recv);
   gpr_free(details);
+  gpr_log(GPR_INFO, "Client call (peer %s) DESTROYED.", peer);
+  gpr_free(peer);
 }
 
 static void setup_client(const char *server_hostport, client_fixture *cf) {
@@ -626,6 +631,7 @@ static void fork_lb_server(void *arg) {
                   tf->lb_server_update_delay_ms);
 }
 
+#define LB_TOKEN_PREFIX "token"
 static test_fixture setup_test_fixture(int lb_server_update_delay_ms) {
   test_fixture tf;
   memset(&tf, 0, sizeof(tf));
@@ -635,11 +641,18 @@ static test_fixture setup_test_fixture(int lb_server_update_delay_ms) {
   gpr_thd_options_set_joinable(&options);
 
   for (int i = 0; i < NUM_BACKENDS; ++i) {
+    // Only the first half of the servers expect an LB token.
+    if (i < NUM_BACKENDS / 2) {
+      tf.lb_backends[i].lb_token_prefix = LB_TOKEN_PREFIX;
+    } else {
+      tf.lb_backends[i].lb_token_prefix = "";
+    }
     setup_server("127.0.0.1", &tf.lb_backends[i]);
     gpr_thd_new(&tf.lb_backends[i].tid, fork_backend_server, &tf.lb_backends[i],
                 &options);
   }
 
+  tf.lb_server.lb_token_prefix = LB_TOKEN_PREFIX;
   setup_server("127.0.0.1", &tf.lb_server);
   gpr_thd_new(&tf.lb_server.tid, fork_lb_server, &tf.lb_server, &options);
 
@@ -686,39 +699,42 @@ static test_fixture test_update(int lb_server_update_delay_ms) {
 
 TEST(GrpclbTest, Updates) {
   grpc::test_fixture tf_result;
-  // Clients take a bit over one second to complete a call (the last part of the
+  // Clients take at least one second to complete a call (the last part of the
   // call sleeps for 1 second while verifying the client's completion queue is
-  // empty). Therefore:
+  // empty), more if the system is under load. Therefore:
   //
   // If the LB server waits 800ms before sending an update, it will arrive
-  // before the first client request is done, skipping the second server from
-  // batch 1 altogether: the 2nd client request will go to the 1st server of
-  // batch 2 (ie, the third one out of the four total servers).
+  // before the first client request finishes, skipping the second server from
+  // batch 1. All subsequent picks will come from the second half of the
+  // backends, those coming in the LB update.
   tf_result = grpc::test_update(800);
   GPR_ASSERT(tf_result.lb_backends[0].num_calls_serviced == 1);
   GPR_ASSERT(tf_result.lb_backends[1].num_calls_serviced == 0);
-  GPR_ASSERT(tf_result.lb_backends[2].num_calls_serviced == 2);
-  GPR_ASSERT(tf_result.lb_backends[3].num_calls_serviced == 1);
+  GPR_ASSERT(tf_result.lb_backends[2].num_calls_serviced +
+                 tf_result.lb_backends[3].num_calls_serviced >
+             0);
+  int num_serviced_calls = 0;
+  for (int i = 0; i < 4; i++) {
+    num_serviced_calls += tf_result.lb_backends[i].num_calls_serviced;
+  }
+  GPR_ASSERT(num_serviced_calls == 4);
 
-  // If the LB server waits 1500ms, the update arrives after having picked the
-  // 2nd server from batch 1 but before the next pick for the first server of
-  // batch 2. All server are used.
-  tf_result = grpc::test_update(1500);
-  GPR_ASSERT(tf_result.lb_backends[0].num_calls_serviced == 1);
-  GPR_ASSERT(tf_result.lb_backends[1].num_calls_serviced == 1);
-  GPR_ASSERT(tf_result.lb_backends[2].num_calls_serviced == 1);
-  GPR_ASSERT(tf_result.lb_backends[3].num_calls_serviced == 1);
-
-  // If the LB server waits > 2000ms, the update arrives after the first two
-  // request are done and the third pick is performed, which returns, in RR
-  // fashion, the 1st server of the 1st update. Therefore, the second server of
-  // batch 1 is hit at least one, whereas the first server of batch 2 is never
-  // hit.
+  // If the LB server waits 2500ms, the update arrives after two calls and three
+  // picks. The third pick will be the 1st server of the 1st update (RR policy
+  // going around). The fourth and final pick will come from the second LB
+  // update. In any case, the total number of serviced calls must again be equal
+  // to four across all the backends.
   tf_result = grpc::test_update(2500);
   GPR_ASSERT(tf_result.lb_backends[0].num_calls_serviced >= 1);
-  GPR_ASSERT(tf_result.lb_backends[1].num_calls_serviced > 0);
-  GPR_ASSERT(tf_result.lb_backends[2].num_calls_serviced > 0);
-  GPR_ASSERT(tf_result.lb_backends[3].num_calls_serviced == 0);
+  GPR_ASSERT(tf_result.lb_backends[1].num_calls_serviced == 1);
+  GPR_ASSERT(tf_result.lb_backends[2].num_calls_serviced +
+                 tf_result.lb_backends[3].num_calls_serviced >
+             0);
+  num_serviced_calls = 0;
+  for (int i = 0; i < 4; i++) {
+    num_serviced_calls += tf_result.lb_backends[i].num_calls_serviced;
+  }
+  GPR_ASSERT(num_serviced_calls == 4);
 }
 
 TEST(GrpclbTest, InvalidAddressInServerlist) {}