Merge branch 'master' of https://github.com/grpc/grpc into channelz

10 files changed, 284 insertions, 24 deletions

diff --git a/test/core/end2end/end2end_nosec_tests.cc b/test/core/end2end/end2end_nosec_tests.cc
index 5ee0bb83fd..061b23b5d6 100644
--- a/test/core/end2end/end2end_nosec_tests.cc
+++ b/test/core/end2end/end2end_nosec_tests.cc
@@ -102,6 +102,8 @@ extern void negative_deadline(grpc_end2end_test_config config);
 extern void negative_deadline_pre_init(void);
 extern void network_status_change(grpc_end2end_test_config config);
 extern void network_status_change_pre_init(void);
+extern void no_error_on_hotpath(grpc_end2end_test_config config);
+extern void no_error_on_hotpath_pre_init(void);
 extern void no_logging(grpc_end2end_test_config config);
 extern void no_logging_pre_init(void);
 extern void no_op(grpc_end2end_test_config config);
@@ -225,6 +227,7 @@ void grpc_end2end_tests_pre_init(void) {
   max_message_length_pre_init();
   negative_deadline_pre_init();
   network_status_change_pre_init();
+  no_error_on_hotpath_pre_init();
   no_logging_pre_init();
   no_op_pre_init();
   payload_pre_init();
@@ -311,6 +314,7 @@ void grpc_end2end_tests(int argc, char **argv,
     max_message_length(config);
     negative_deadline(config);
     network_status_change(config);
+    no_error_on_hotpath(config);
     no_logging(config);
     no_op(config);
     payload(config);
@@ -500,6 +504,10 @@ void grpc_end2end_tests(int argc, char **argv,
       network_status_change(config);
       continue;
     }
+    if (0 == strcmp("no_error_on_hotpath", argv[i])) {
+      no_error_on_hotpath(config);
+      continue;
+    }
     if (0 == strcmp("no_logging", argv[i])) {
       no_logging(config);
       continue;
diff --git a/test/core/end2end/end2end_tests.cc b/test/core/end2end/end2end_tests.cc
index ca5c18bcc2..7ae475cdef 100644
--- a/test/core/end2end/end2end_tests.cc
+++ b/test/core/end2end/end2end_tests.cc
@@ -104,6 +104,8 @@ extern void negative_deadline(grpc_end2end_test_config config);
 extern void negative_deadline_pre_init(void);
 extern void network_status_change(grpc_end2end_test_config config);
 extern void network_status_change_pre_init(void);
+extern void no_error_on_hotpath(grpc_end2end_test_config config);
+extern void no_error_on_hotpath_pre_init(void);
 extern void no_logging(grpc_end2end_test_config config);
 extern void no_logging_pre_init(void);
 extern void no_op(grpc_end2end_test_config config);
@@ -228,6 +230,7 @@ void grpc_end2end_tests_pre_init(void) {
   max_message_length_pre_init();
   negative_deadline_pre_init();
   network_status_change_pre_init();
+  no_error_on_hotpath_pre_init();
   no_logging_pre_init();
   no_op_pre_init();
   payload_pre_init();
@@ -315,6 +318,7 @@ void grpc_end2end_tests(int argc, char **argv,
     max_message_length(config);
     negative_deadline(config);
     network_status_change(config);
+    no_error_on_hotpath(config);
     no_logging(config);
     no_op(config);
     payload(config);
@@ -508,6 +512,10 @@ void grpc_end2end_tests(int argc, char **argv,
       network_status_change(config);
       continue;
     }
+    if (0 == strcmp("no_error_on_hotpath", argv[i])) {
+      no_error_on_hotpath(config);
+      continue;
+    }
     if (0 == strcmp("no_logging", argv[i])) {
       no_logging(config);
       continue;
diff --git a/test/core/end2end/fixtures/h2_http_proxy.cc b/test/core/end2end/fixtures/h2_http_proxy.cc
index 0af8a29a15..c2ac209cf9 100644
--- a/test/core/end2end/fixtures/h2_http_proxy.cc
+++ b/test/core/end2end/fixtures/h2_http_proxy.cc
@@ -69,9 +69,8 @@ void chttp2_init_client_fullstack(grpc_end2end_test_fixture* f,
   char* proxy_uri;
 
   /* If testing for proxy auth, add credentials to proxy uri */
-  const grpc_arg* proxy_auth_arg =
-      grpc_channel_args_find(client_args, GRPC_ARG_HTTP_PROXY_AUTH_CREDS);
-  const char* proxy_auth_str = grpc_channel_arg_get_string(proxy_auth_arg);
+  const char* proxy_auth_str =
+      grpc_channel_args_get_string(client_args, GRPC_ARG_HTTP_PROXY_AUTH_CREDS);
   if (proxy_auth_str == nullptr) {
     gpr_asprintf(&proxy_uri, "http://%s",
                  grpc_end2end_http_proxy_get_proxy_name(ffd->proxy));
diff --git a/test/core/end2end/fixtures/http_proxy_fixture.cc b/test/core/end2end/fixtures/http_proxy_fixture.cc
index f02fa9d998..5caddee09e 100644
--- a/test/core/end2end/fixtures/http_proxy_fixture.cc
+++ b/test/core/end2end/fixtures/http_proxy_fixture.cc
@@ -411,9 +411,8 @@ static void on_read_request_done(void* arg, grpc_error* error) {
     return;
   }
   // If proxy auth is being used, check if the header is present and as expected
-  const grpc_arg* proxy_auth_arg = grpc_channel_args_find(
+  char* proxy_auth_str = grpc_channel_args_get_string(
       conn->proxy->channel_args, GRPC_ARG_HTTP_PROXY_AUTH_CREDS);
-  char* proxy_auth_str = grpc_channel_arg_get_string(proxy_auth_arg);
   if (proxy_auth_str != nullptr) {
     bool client_authenticated = false;
     for (size_t i = 0; i < conn->http_request.hdr_count; i++) {
diff --git a/test/core/end2end/fuzzers/api_fuzzer.cc b/test/core/end2end/fuzzers/api_fuzzer.cc
index ba9c04fd0e..d0d82ea43b 100644
--- a/test/core/end2end/fuzzers/api_fuzzer.cc
+++ b/test/core/end2end/fuzzers/api_fuzzer.cc
@@ -374,13 +374,11 @@ void my_resolve_address(const char* addr, const char* default_port,
 static grpc_address_resolver_vtable fuzzer_resolver = {my_resolve_address,
                                                        nullptr};
 
-grpc_ares_request* my_dns_lookup_ares(const char* dns_server, const char* addr,
-                                      const char* default_port,
-                                      grpc_pollset_set* interested_parties,
-                                      grpc_closure* on_done,
-                                      grpc_lb_addresses** lb_addrs,
-                                      bool check_grpclb,
-                                      char** service_config_json) {
+grpc_ares_request* my_dns_lookup_ares_locked(
+    const char* dns_server, const char* addr, const char* default_port,
+    grpc_pollset_set* interested_parties, grpc_closure* on_done,
+    grpc_lb_addresses** lb_addrs, bool check_grpclb, char** service_config_json,
+    grpc_combiner* combiner) {
   addr_req* r = static_cast<addr_req*>(gpr_malloc(sizeof(*r)));
   r->addr = gpr_strdup(addr);
   r->on_done = on_done;
@@ -706,7 +704,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
     grpc_executor_set_threading(false);
   }
   grpc_set_resolver_impl(&fuzzer_resolver);
-  grpc_dns_lookup_ares = my_dns_lookup_ares;
+  grpc_dns_lookup_ares_locked = my_dns_lookup_ares_locked;
 
   GPR_ASSERT(g_channel == nullptr);
   GPR_ASSERT(g_server == nullptr);
diff --git a/test/core/end2end/gen_build_yaml.py b/test/core/end2end/gen_build_yaml.py
index 53f4ab0d52..04dc2a8785 100755
--- a/test/core/end2end/gen_build_yaml.py
+++ b/test/core/end2end/gen_build_yaml.py
@@ -139,6 +139,7 @@ END2END_TESTS = {
     'max_message_length': default_test_options._replace(cpu_cost=LOWCPU),
     'negative_deadline': default_test_options,
     'network_status_change': default_test_options._replace(cpu_cost=LOWCPU),
+    'no_error_on_hotpath': default_test_options._replace(proxyable=False),
     'no_logging': default_test_options._replace(traceable=False),
     'no_op': default_test_options,
     'payload': default_test_options,
diff --git a/test/core/end2end/generate_tests.bzl b/test/core/end2end/generate_tests.bzl
index 94bc092b59..67769a8cb1 100755
--- a/test/core/end2end/generate_tests.bzl
+++ b/test/core/end2end/generate_tests.bzl
@@ -135,6 +135,7 @@ END2END_TESTS = {
     'max_message_length': test_options(),
     'negative_deadline': test_options(),
     'network_status_change': test_options(),
+    'no_error_on_hotpath': test_options(proxyable=False),
     'no_logging': test_options(traceable=False),
     'no_op': test_options(),
     'payload': test_options(),
diff --git a/test/core/end2end/goaway_server_test.cc b/test/core/end2end/goaway_server_test.cc
index 0188698f17..8904c3d325 100644
--- a/test/core/end2end/goaway_server_test.cc
+++ b/test/core/end2end/goaway_server_test.cc
@@ -44,11 +44,11 @@ static void* tag(intptr_t i) { return (void*)i; }
 static gpr_mu g_mu;
 static int g_resolve_port = -1;
 
-static grpc_ares_request* (*iomgr_dns_lookup_ares)(
+static grpc_ares_request* (*iomgr_dns_lookup_ares_locked)(
     const char* dns_server, const char* addr, const char* default_port,
     grpc_pollset_set* interested_parties, grpc_closure* on_done,
     grpc_lb_addresses** addresses, bool check_grpclb,
-    char** service_config_json);
+    char** service_config_json, grpc_combiner* combiner);
 
 static void set_resolve_port(int port) {
   gpr_mu_lock(&g_mu);
@@ -98,15 +98,15 @@ static grpc_error* my_blocking_resolve_address(
 static grpc_address_resolver_vtable test_resolver = {
     my_resolve_address, my_blocking_resolve_address};
 
-static grpc_ares_request* my_dns_lookup_ares(
+static grpc_ares_request* my_dns_lookup_ares_locked(
     const char* dns_server, const char* addr, const char* default_port,
     grpc_pollset_set* interested_parties, grpc_closure* on_done,
-    grpc_lb_addresses** lb_addrs, bool check_grpclb,
-    char** service_config_json) {
+    grpc_lb_addresses** lb_addrs, bool check_grpclb, char** service_config_json,
+    grpc_combiner* combiner) {
   if (0 != strcmp(addr, "test")) {
-    return iomgr_dns_lookup_ares(dns_server, addr, default_port,
-                                 interested_parties, on_done, lb_addrs,
-                                 check_grpclb, service_config_json);
+    return iomgr_dns_lookup_ares_locked(
+        dns_server, addr, default_port, interested_parties, on_done, lb_addrs,
+        check_grpclb, service_config_json, combiner);
   }
 
   grpc_error* error = GRPC_ERROR_NONE;
@@ -142,8 +142,8 @@ int main(int argc, char** argv) {
   grpc_init();
   default_resolver = grpc_resolve_address_impl;
   grpc_set_resolver_impl(&test_resolver);
-  iomgr_dns_lookup_ares = grpc_dns_lookup_ares;
-  grpc_dns_lookup_ares = my_dns_lookup_ares;
+  iomgr_dns_lookup_ares_locked = grpc_dns_lookup_ares_locked;
+  grpc_dns_lookup_ares_locked = my_dns_lookup_ares_locked;
 
   int was_cancelled1;
   int was_cancelled2;
diff --git a/test/core/end2end/tests/no_error_on_hotpath.cc b/test/core/end2end/tests/no_error_on_hotpath.cc
new file mode 100644
index 0000000000..7afadf7d3a
--- /dev/null
+++ b/test/core/end2end/tests/no_error_on_hotpath.cc
@@ -0,0 +1,246 @@
+/*
+ *
+ * Copyright 2016 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/gpr/string.h"
+#include "src/core/lib/iomgr/error.h"
+#include "test/core/end2end/cq_verifier.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);
+}
+
+static void simple_request_body(grpc_end2end_test_config config,
+                                grpc_end2end_test_fixture f) {
+  grpc_call* c;
+  grpc_call* s;
+  cq_verifier* cqv = cq_verifier_create(f.cq);
+  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_status_code status;
+  grpc_call_error error;
+  grpc_slice details;
+  int was_cancelled = 2;
+  char* peer;
+
+  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("/foo"), nullptr,
+                               deadline, nullptr);
+  GPR_ASSERT(c);
+
+  peer = grpc_call_get_peer(c);
+  GPR_ASSERT(peer != nullptr);
+  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);
+
+  memset(ops, 0, sizeof(ops));
+  op = ops;
+  op->op = GRPC_OP_SEND_INITIAL_METADATA;
+  op->data.send_initial_metadata.count = 0;
+  op->flags = 0;
+  op->reserved = nullptr;
+  op++;
+  op->op = GRPC_OP_SEND_CLOSE_FROM_CLIENT;
+  op->flags = 0;
+  op->reserved = nullptr;
+  op++;
+  op->op = GRPC_OP_RECV_INITIAL_METADATA;
+  op->data.recv_initial_metadata.recv_initial_metadata = &initial_metadata_recv;
+  op->flags = 0;
+  op->reserved = nullptr;
+  op++;
+  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->flags = 0;
+  op->reserved = nullptr;
+  op++;
+  error = grpc_call_start_batch(c, ops, static_cast<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), 1);
+  cq_verify(cqv);
+
+  peer = grpc_call_get_peer(s);
+  GPR_ASSERT(peer != nullptr);
+  gpr_free(peer);
+  peer = grpc_call_get_peer(c);
+  GPR_ASSERT(peer != nullptr);
+  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->flags = 0;
+  op->reserved = nullptr;
+  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_OK;
+  op->data.send_status_from_server.status_details = nullptr;
+  op->flags = 0;
+  op->reserved = nullptr;
+  op++;
+  op->op = GRPC_OP_RECV_CLOSE_ON_SERVER;
+  op->data.recv_close_on_server.cancelled = &was_cancelled;
+  op->flags = 0;
+  op->reserved = nullptr;
+  op++;
+  error = grpc_call_start_batch(s, ops, static_cast<size_t>(op - ops), tag(102),
+                                nullptr);
+  GPR_ASSERT(GRPC_CALL_OK == error);
+
+  CQ_EXPECT_COMPLETION(cqv, tag(102), 1);
+  CQ_EXPECT_COMPLETION(cqv, tag(1), 1);
+  cq_verify(cqv);
+
+  GPR_ASSERT(status == GRPC_STATUS_OK);
+  GPR_ASSERT(GRPC_SLICE_LENGTH(details) == 0);
+  GPR_ASSERT(0 == grpc_slice_str_cmp(call_details.method, "/foo"));
+  GPR_ASSERT(0 == call_details.flags);
+  GPR_ASSERT(was_cancelled == 0);
+
+  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_call_unref(c);
+  grpc_call_unref(s);
+
+  cq_verifier_destroy(cqv);
+}
+
+static void test_no_error_on_hotpath_one_request(
+    grpc_end2end_test_config config) {
+  grpc_end2end_test_fixture f;
+
+  f = begin_test(config, "test_invoke_simple_request_with_no_error_logging",
+                 nullptr, nullptr);
+  // First RPC is not considered the hotpath, since there are lots of things to
+  // set up.
+  simple_request_body(config, f);
+  grpc_disable_error_creation();
+  simple_request_body(config, f);
+  grpc_enable_error_creation();
+  end_test(&f);
+  config.tear_down_data(&f);
+}
+
+static void test_no_error_on_hotpath_10_requests(
+    grpc_end2end_test_config config) {
+  int i;
+  grpc_end2end_test_fixture f = begin_test(
+      config, "test_no_error_on_hotpath_in_one_request", nullptr, nullptr);
+  // First RPC is not considered the hotpath, since there are lots of things to
+  // set up.
+  simple_request_body(config, f);
+  grpc_disable_error_creation();
+  for (i = 0; i < 10; i++) {
+    simple_request_body(config, f);
+  }
+  grpc_enable_error_creation();
+  end_test(&f);
+  config.tear_down_data(&f);
+}
+
+void no_error_on_hotpath(grpc_end2end_test_config config) {
+  test_no_error_on_hotpath_one_request(config);
+  test_no_error_on_hotpath_10_requests(config);
+}
+
+void no_error_on_hotpath_pre_init(void) {}
diff --git a/test/core/end2end/tests/retry_server_pushback_delay.cc b/test/core/end2end/tests/retry_server_pushback_delay.cc
index f6a979613d..221b416828 100644
--- a/test/core/end2end/tests/retry_server_pushback_delay.cc
+++ b/test/core/end2end/tests/retry_server_pushback_delay.cc
@@ -249,7 +249,7 @@ static void test_retry_server_pushback_delay(grpc_end2end_test_config config) {
           retry_delay.tv_sec, retry_delay.tv_nsec);
   GPR_ASSERT(retry_delay.tv_sec >= 1);
   if (retry_delay.tv_sec == 1) {
-    GPR_ASSERT(retry_delay.tv_nsec >= 900000000);
+    GPR_ASSERT(retry_delay.tv_nsec >= 800000000);
   }
 
   peer = grpc_call_get_peer(s);