From c4b61e24b351fad7aac82e1be112a19980b068f7 Mon Sep 17 00:00:00 2001 From: Adele Zhou Date: Thu, 25 Aug 2016 16:32:06 -0700 Subject: [PATCH 01/13] Use the VIP that routes to GFE. --- tools/run_tests/run_interop_tests.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/run_tests/run_interop_tests.py b/tools/run_tests/run_interop_tests.py index 0d402d67e5e..5feda65de8f 100755 --- a/tools/run_tests/run_interop_tests.py +++ b/tools/run_tests/run_interop_tests.py @@ -623,15 +623,15 @@ def aggregate_http2_results(stdout): # TODO(adelez): implement logic for errors_allowed where if the indicated tests # fail, they don't impact the overall test result. prod_servers = { - 'default': ('grpc-test.sandbox.googleapis.com', + 'default': ('216.239.32.254', 'grpc-test.sandbox.googleapis.com', False), - 'gateway_v2': ('grpc-test2.sandbox.googleapis.com', + 'gateway_v2': ('216.239.32.254', 'grpc-test2.sandbox.googleapis.com', True), 'cloud_gateway': ('216.239.32.255', 'grpc-test.sandbox.googleapis.com', False), 'cloud_gateway_v2': ('216.239.32.255', 'grpc-test2.sandbox.googleapis.com', True), - 'gateway_v4': ('grpc-test4.sandbox.googleapis.com', + 'gateway_v4': ('216.239.32.254', 'grpc-test4.sandbox.googleapis.com', True), 'cloud_gateway_v4': ('216.239.32.255', 'grpc-test4.sandbox.googleapis.com', True), From 650ae16bf7911bad69707c27ae3fd88e375771e1 Mon Sep 17 00:00:00 2001 From: Egor Suvorov Date: Fri, 26 Aug 2016 16:26:23 -0400 Subject: [PATCH 02/13] Add IWYU export pragmas to grpc++/grpc++.h. IWYU is a C++ tool which ensures that end users explicitly #include every header they need. Without pragmas IWYU have no way of distinguishing between 'implementation-specific' headers and headers which are shortcuts for groups of other headers, like grpc++.h. --- include/grpc++/grpc++.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/grpc++/grpc++.h b/include/grpc++/grpc++.h index ef07e199759..e4b0478d770 100644 --- a/include/grpc++/grpc++.h +++ b/include/grpc++/grpc++.h @@ -51,6 +51,7 @@ #ifndef GRPCXX_GRPCXX_H #define GRPCXX_GRPCXX_H +// IWYU pragma: begin_exports #include #include @@ -62,5 +63,6 @@ #include #include #include +// IWYU pragma: end_exports #endif // GRPCXX_GRPCXX_H From 93901a7a8696d0496fa336e0b74ed821da9e340e Mon Sep 17 00:00:00 2001 From: Egor Suvorov Date: Fri, 26 Aug 2016 16:48:38 -0400 Subject: [PATCH 03/13] Add clarifying comment about IWYU in grpc++.h --- include/grpc++/grpc++.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/grpc++/grpc++.h b/include/grpc++/grpc++.h index e4b0478d770..afb1c555bb6 100644 --- a/include/grpc++/grpc++.h +++ b/include/grpc++/grpc++.h @@ -51,6 +51,8 @@ #ifndef GRPCXX_GRPCXX_H #define GRPCXX_GRPCXX_H +// Pragma for http://include-what-you-use.org/ tool, tells that following +// headers are not private for grpc++.h and are part of its interface. // IWYU pragma: begin_exports #include From 7176a828c5418cfb1084c7b776be65a0ea7b138e Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Mon, 29 Aug 2016 17:04:50 -0700 Subject: [PATCH 04/13] Use --protofiles --- test/cpp/util/grpc_tool.cc | 22 +++++++++++----------- test/cpp/util/grpc_tool.h | 2 +- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/test/cpp/util/grpc_tool.cc b/test/cpp/util/grpc_tool.cc index f06053ca23e..4af00cdc966 100644 --- a/test/cpp/util/grpc_tool.cc +++ b/test/cpp/util/grpc_tool.cc @@ -57,7 +57,7 @@ DEFINE_bool(remotedb, true, "Use server types to parse and format messages"); DEFINE_string(metadata, "", "Metadata to send to server, in the form of key1:val1:key2:val2"); DEFINE_string(proto_path, ".", "Path to look for the proto file."); -DEFINE_string(proto_file, "", "Name of the proto file."); +DEFINE_string(protofiles, "", "Name of the proto file."); DEFINE_bool(binary_input, false, "Input in binary format"); DEFINE_bool(binary_output, false, "Output in binary format"); DEFINE_string(infile, "", "Input file (default is stdin)"); @@ -71,9 +71,9 @@ class GrpcTool { explicit GrpcTool(); virtual ~GrpcTool() {} - bool Help(int argc, const char** argv, CliCredentials cred, + bool Help(int argc, const char** argv, const CliCredentials& cred, GrpcToolOutputCallback callback); - bool CallMethod(int argc, const char** argv, CliCredentials cred, + bool CallMethod(int argc, const char** argv, const CliCredentials& cred, GrpcToolOutputCallback callback); // TODO(zyc): implement the following methods // bool ListServices(int argc, const char** argv, GrpcToolOutputCallback @@ -101,7 +101,7 @@ class GrpcTool { }; template -std::function BindWith5Args(T&& func) { return std::bind(std::forward(func), std::placeholders::_1, @@ -156,7 +156,7 @@ void PrintMetadata(const T& m, const grpc::string& message) { struct Command { const char* command; - std::function function; int min_args; @@ -201,7 +201,7 @@ const Command* FindCommand(const grpc::string& name) { } } // namespace -int GrpcToolMainLib(int argc, const char** argv, const CliCredentials cred, +int GrpcToolMainLib(int argc, const char** argv, const CliCredentials& cred, GrpcToolOutputCallback callback) { if (argc < 2) { Usage("No command specified"); @@ -238,7 +238,7 @@ void GrpcTool::CommandUsage(const grpc::string& usage) const { } } -bool GrpcTool::Help(int argc, const char** argv, const CliCredentials cred, +bool GrpcTool::Help(int argc, const char** argv, const CliCredentials& cred, GrpcToolOutputCallback callback) { CommandUsage( "Print help\n" @@ -258,7 +258,7 @@ bool GrpcTool::Help(int argc, const char** argv, const CliCredentials cred, } bool GrpcTool::CallMethod(int argc, const char** argv, - const CliCredentials cred, + const CliCredentials& cred, GrpcToolOutputCallback callback) { CommandUsage( "Call method\n" @@ -267,10 +267,10 @@ bool GrpcTool::CallMethod(int argc, const char** argv, " ; Exported service name\n" " ; Method name\n" " ; Text protobuffer (overrides infile)\n" - " --proto_file ; Comma separated proto files used as a" + " --protofiles ; Comma separated proto files used as a" " fallback when parsing request/response\n" " --proto_path ; The search path of proto files, valid" - " only when --proto_file is given\n" + " only when --protofiles is given\n" " --metadata ; The metadata to be sent to the server\n" " --infile ; Input filename (defaults to stdin)\n" " --outfile ; Output filename (defaults to stdout)\n" @@ -310,7 +310,7 @@ bool GrpcTool::CallMethod(int argc, const char** argv, if (!FLAGS_binary_input || !FLAGS_binary_output) { parser.reset( new grpc::testing::ProtoFileParser(FLAGS_remotedb ? channel : nullptr, - FLAGS_proto_path, FLAGS_proto_file)); + FLAGS_proto_path, FLAGS_protofiles)); if (parser->HasError()) { return false; } diff --git a/test/cpp/util/grpc_tool.h b/test/cpp/util/grpc_tool.h index 3da86937c2d..df3680258b8 100644 --- a/test/cpp/util/grpc_tool.h +++ b/test/cpp/util/grpc_tool.h @@ -45,7 +45,7 @@ namespace testing { typedef std::function GrpcToolOutputCallback; -int GrpcToolMainLib(int argc, const char **argv, CliCredentials cred, +int GrpcToolMainLib(int argc, const char **argv, const CliCredentials &cred, GrpcToolOutputCallback callback); } // namespace testing From 11d3c8f3d4463a63facba3d53f7e6e7f9008aad0 Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Mon, 29 Aug 2016 18:18:41 -0700 Subject: [PATCH 05/13] Add ServerBuilderPlugin::UpdateChannelArguments --- include/grpc++/impl/server_builder_plugin.h | 5 +++++ src/cpp/server/server_builder.cc | 12 +++++------- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/include/grpc++/impl/server_builder_plugin.h b/include/grpc++/impl/server_builder_plugin.h index 1e157efa111..61632e32fa4 100644 --- a/include/grpc++/impl/server_builder_plugin.h +++ b/include/grpc++/impl/server_builder_plugin.h @@ -41,6 +41,7 @@ namespace grpc { class ServerInitializer; +class ChannelArguments; class ServerBuilderPlugin { public: @@ -58,6 +59,10 @@ class ServerBuilderPlugin { // ServerBuilderOption::UpdatePlugins virtual void ChangeArguments(const grpc::string& name, void* value) = 0; + // UpdateChannelArguments will be called in ServerBuilder::BuildAndStart(), + // before the Server instance is created. + virtual void UpdateChannelArguments(ChannelArguments* args) {} + virtual bool has_sync_methods() const { return false; } virtual bool has_async_methods() const { return false; } }; diff --git a/src/cpp/server/server_builder.cc b/src/cpp/server/server_builder.cc index 45bb858e2ed..669af9da783 100644 --- a/src/cpp/server/server_builder.cc +++ b/src/cpp/server/server_builder.cc @@ -154,14 +154,12 @@ std::unique_ptr ServerBuilder::BuildAndStart() { (*option)->UpdateArguments(&args); (*option)->UpdatePlugins(&plugins_); } - if (!thread_pool) { - for (auto plugin = plugins_.begin(); plugin != plugins_.end(); plugin++) { - if ((*plugin)->has_sync_methods()) { - thread_pool.reset(CreateDefaultThreadPool()); - has_sync_methods = true; - break; - } + for (auto plugin = plugins_.begin(); plugin != plugins_.end(); plugin++) { + if (!thread_pool && (*plugin)->has_sync_methods()) { + thread_pool.reset(CreateDefaultThreadPool()); + has_sync_methods = true; } + (*plugin)->UpdateChannelArguments(&args); } if (max_message_size_ > 0) { args.SetInt(GRPC_ARG_MAX_MESSAGE_LENGTH, max_message_size_); From 96ace25a83a032ef7fab170b4a6c0d33c3c02ffc Mon Sep 17 00:00:00 2001 From: Adele Zhou Date: Tue, 30 Aug 2016 10:25:04 -0700 Subject: [PATCH 06/13] Fix full cloud to prod --- .../{run_full_interop.sh => run_full_cloud_prod.sh} | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) rename tools/jenkins/{run_full_interop.sh => run_full_cloud_prod.sh} (87%) mode change 100755 => 100644 diff --git a/tools/jenkins/run_full_interop.sh b/tools/jenkins/run_full_cloud_prod.sh old mode 100755 new mode 100644 similarity index 87% rename from tools/jenkins/run_full_interop.sh rename to tools/jenkins/run_full_cloud_prod.sh index a82da1cb68c..ea51b500668 --- a/tools/jenkins/run_full_interop.sh +++ b/tools/jenkins/run_full_cloud_prod.sh @@ -31,7 +31,14 @@ # This script is invoked by Jenkins and runs interop test suite. set -ex +export LANG=en_US.UTF-8 + # Enter the gRPC repo root cd $(dirname $0)/../.. -tools/run_tests/run_interop_tests.py -l all -s all --cloud_to_prod --cloud_to_prod_auth --prod_servers default cloud_gateway gateway_v2 cloud_gateway_v2 gateway_v4 cloud_gateway_v4 --use_docker --http2_interop -t -j 12 $@ || true +tools/run_tests/run_interop_tests.py \ + -l all \ + --cloud_to_prod \ + --cloud_to_prod_auth \ + --prod_servers default cloud_gateway gateway_v4 cloud_gateway_v4 \ + --use_docker -t -j 12 $@ || true From 916079de1bbb815d28ce8d10367308fc13cf2a35 Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Tue, 30 Aug 2016 15:52:09 -0700 Subject: [PATCH 07/13] Add no-logging test --- Makefile | 2 + test/core/end2end/end2end_nosec_tests.c | 8 + test/core/end2end/end2end_tests.c | 8 + test/core/end2end/gen_build_yaml.py | 1 + test/core/end2end/tests/no_logging.c | 293 ++++++++++ tools/run_tests/sources_and_headers.json | 2 + tools/run_tests/tests.json | 523 ++++++++++++++++++ .../end2end_nosec_tests.vcxproj | 2 + .../end2end_nosec_tests.vcxproj.filters | 3 + .../tests/end2end_tests/end2end_tests.vcxproj | 2 + .../end2end_tests.vcxproj.filters | 3 + 11 files changed, 847 insertions(+) create mode 100644 test/core/end2end/tests/no_logging.c diff --git a/Makefile b/Makefile index 4bd4c306c17..21527993668 100644 --- a/Makefile +++ b/Makefile @@ -6770,6 +6770,7 @@ LIBEND2END_TESTS_SRC = \ test/core/end2end/tests/max_message_length.c \ test/core/end2end/tests/negative_deadline.c \ test/core/end2end/tests/network_status_change.c \ + test/core/end2end/tests/no_logging.c \ test/core/end2end/tests/no_op.c \ test/core/end2end/tests/payload.c \ test/core/end2end/tests/ping.c \ @@ -6850,6 +6851,7 @@ LIBEND2END_NOSEC_TESTS_SRC = \ test/core/end2end/tests/max_message_length.c \ test/core/end2end/tests/negative_deadline.c \ test/core/end2end/tests/network_status_change.c \ + test/core/end2end/tests/no_logging.c \ test/core/end2end/tests/no_op.c \ test/core/end2end/tests/payload.c \ test/core/end2end/tests/ping.c \ diff --git a/test/core/end2end/end2end_nosec_tests.c b/test/core/end2end/end2end_nosec_tests.c index 3efd18cf2e2..2e8da051d5f 100644 --- a/test/core/end2end/end2end_nosec_tests.c +++ b/test/core/end2end/end2end_nosec_tests.c @@ -95,6 +95,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_logging(grpc_end2end_test_config config); +extern void no_logging_pre_init(void); extern void no_op(grpc_end2end_test_config config); extern void no_op_pre_init(void); extern void payload(grpc_end2end_test_config config); @@ -155,6 +157,7 @@ void grpc_end2end_tests_pre_init(void) { max_message_length_pre_init(); negative_deadline_pre_init(); network_status_change_pre_init(); + no_logging_pre_init(); no_op_pre_init(); payload_pre_init(); ping_pre_init(); @@ -205,6 +208,7 @@ void grpc_end2end_tests(int argc, char **argv, max_message_length(config); negative_deadline(config); network_status_change(config); + no_logging(config); no_op(config); payload(config); ping(config); @@ -328,6 +332,10 @@ void grpc_end2end_tests(int argc, char **argv, network_status_change(config); continue; } + if (0 == strcmp("no_logging", argv[i])) { + no_logging(config); + continue; + } if (0 == strcmp("no_op", argv[i])) { no_op(config); continue; diff --git a/test/core/end2end/end2end_tests.c b/test/core/end2end/end2end_tests.c index e3d791abc16..22a914e8ec2 100644 --- a/test/core/end2end/end2end_tests.c +++ b/test/core/end2end/end2end_tests.c @@ -97,6 +97,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_logging(grpc_end2end_test_config config); +extern void no_logging_pre_init(void); extern void no_op(grpc_end2end_test_config config); extern void no_op_pre_init(void); extern void payload(grpc_end2end_test_config config); @@ -158,6 +160,7 @@ void grpc_end2end_tests_pre_init(void) { max_message_length_pre_init(); negative_deadline_pre_init(); network_status_change_pre_init(); + no_logging_pre_init(); no_op_pre_init(); payload_pre_init(); ping_pre_init(); @@ -209,6 +212,7 @@ void grpc_end2end_tests(int argc, char **argv, max_message_length(config); negative_deadline(config); network_status_change(config); + no_logging(config); no_op(config); payload(config); ping(config); @@ -336,6 +340,10 @@ void grpc_end2end_tests(int argc, char **argv, network_status_change(config); continue; } + if (0 == strcmp("no_logging", argv[i])) { + no_logging(config); + continue; + } if (0 == strcmp("no_op", argv[i])) { no_op(config); continue; diff --git a/test/core/end2end/gen_build_yaml.py b/test/core/end2end/gen_build_yaml.py index e59b7dc9fb2..b8ff5736147 100755 --- a/test/core/end2end/gen_build_yaml.py +++ b/test/core/end2end/gen_build_yaml.py @@ -114,6 +114,7 @@ END2END_TESTS = { 'max_message_length': default_test_options, 'negative_deadline': default_test_options, 'network_status_change': default_test_options, + 'no_logging': default_test_options._replace(traceable=False), 'no_op': default_test_options, 'payload': default_test_options, 'load_reporting_hook': default_test_options, diff --git a/test/core/end2end/tests/no_logging.c b/test/core/end2end/tests/no_logging.c new file mode 100644 index 00000000000..aa96118fc31 --- /dev/null +++ b/test/core/end2end/tests/no_logging.c @@ -0,0 +1,293 @@ +/* + * + * Copyright 2016, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#include "test/core/end2end/end2end_tests.h" + +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include "src/core/lib/iomgr/error.h" +#include "src/core/lib/support/string.h" +#include "test/core/end2end/cq_verifier.h" + +enum { TIMEOUT = 200000 }; + +static void *tag(intptr_t t) { return (void *)t; } + +extern void gpr_default_log(gpr_log_func_args *args); + +static void test_no_log(gpr_log_func_args *args) { + char *message = NULL; + gpr_asprintf(&message, "Unwanted log: %s", args->message); + args->message = message; + gpr_default_log(args); + gpr_free(message); + abort(); +} + +static void test_no_error_log(gpr_log_func_args *args) { + if (args->severity == GPR_LOG_SEVERITY_ERROR) { + test_no_log(args); + } +} + +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, "%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_time(int n) { + return GRPC_TIMEOUT_SECONDS_TO_DEADLINE(n); +} + +static gpr_timespec five_seconds_time(void) { return n_seconds_time(5); } + +static void drain_cq(grpc_completion_queue *cq) { + grpc_event ev; + do { + ev = grpc_completion_queue_next(cq, five_seconds_time(), NULL); + } 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->cq, tag(1000)); + GPR_ASSERT(grpc_completion_queue_pluck( + f->cq, tag(1000), GRPC_TIMEOUT_SECONDS_TO_DEADLINE(5), NULL) + .type == GRPC_OP_COMPLETE); + grpc_server_destroy(f->server); + f->server = NULL; +} + +static void shutdown_client(grpc_end2end_test_fixture *f) { + if (!f->client) return; + grpc_channel_destroy(f->client); + f->client = NULL; +} + +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); +} + +static void simple_request_body(grpc_end2end_test_fixture f) { + grpc_call *c; + grpc_call *s; + gpr_timespec deadline = five_seconds_time(); + 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; + char *details = NULL; + size_t details_capacity = 0; + int was_cancelled = 2; + char *peer; + + c = grpc_channel_create_call(f.client, NULL, GRPC_PROPAGATE_DEFAULTS, f.cq, + "/foo", "foo.test.google.fr:1234", deadline, + NULL); + GPR_ASSERT(c); + + peer = grpc_call_get_peer(c); + GPR_ASSERT(peer != NULL); + 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 = NULL; + op++; + op->op = GRPC_OP_SEND_CLOSE_FROM_CLIENT; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_RECV_INITIAL_METADATA; + op->data.recv_initial_metadata = &initial_metadata_recv; + op->flags = 0; + op->reserved = NULL; + 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->data.recv_status_on_client.status_details_capacity = &details_capacity; + op->flags = 0; + op->reserved = NULL; + op++; + error = grpc_call_start_batch(c, ops, (size_t)(op - ops), tag(1), NULL); + 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 != NULL); + gpr_free(peer); + peer = grpc_call_get_peer(c); + GPR_ASSERT(peer != NULL); + 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 = NULL; + 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_UNIMPLEMENTED; + op->data.send_status_from_server.status_details = "xyz"; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_RECV_CLOSE_ON_SERVER; + op->data.recv_close_on_server.cancelled = &was_cancelled; + op->flags = 0; + op->reserved = NULL; + op++; + error = grpc_call_start_batch(s, ops, (size_t)(op - ops), tag(102), NULL); + 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_UNIMPLEMENTED); + GPR_ASSERT(0 == strcmp(details, "xyz")); + GPR_ASSERT(0 == strcmp(call_details.method, "/foo")); + GPR_ASSERT(0 == strcmp(call_details.host, "foo.test.google.fr:1234")); + GPR_ASSERT(0 == call_details.flags); + GPR_ASSERT(was_cancelled == 1); + + gpr_free(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_destroy(c); + grpc_call_destroy(s); + + cq_verifier_destroy(cqv); +} + +static void test_invoke_simple_request(grpc_end2end_test_config config) { + grpc_end2end_test_fixture f; + + f = begin_test(config, "test_invoke_simple_request_with_no_error_logging", + NULL, NULL); + simple_request_body(f); + end_test(&f); + config.tear_down_data(&f); +} + +static void test_invoke_10_simple_requests(grpc_end2end_test_config config) { + int i; + grpc_end2end_test_fixture f = + begin_test(config, "test_invoke_10_simple_requests_with_no_error_logging", + NULL, NULL); + for (i = 0; i < 10; i++) { + simple_request_body(f); + gpr_log(GPR_INFO, "Passed simple request %d", i); + } + simple_request_body(f); + end_test(&f); + config.tear_down_data(&f); +} + +static void test_no_error_logging_in_entire_process( + grpc_end2end_test_config config) { + int i; + gpr_set_log_function(test_no_error_log); + for (i = 0; i < 10; i++) { + test_invoke_simple_request(config); + } + test_invoke_10_simple_requests(config); + gpr_set_log_function(gpr_default_log); +} + +static void test_no_logging_in_one_request(grpc_end2end_test_config config) { + int i; + grpc_end2end_test_fixture f = + begin_test(config, "test_no_logging_in_last_request", NULL, NULL); + for (i = 0; i < 10; i++) { + simple_request_body(f); + } + gpr_set_log_function(test_no_log); + simple_request_body(f); + gpr_set_log_function(gpr_default_log); + end_test(&f); + config.tear_down_data(&f); +} + +void no_logging(grpc_end2end_test_config config) { + test_no_logging_in_one_request(config); + test_no_error_logging_in_entire_process(config); +} + +void no_logging_pre_init(void) {} diff --git a/tools/run_tests/sources_and_headers.json b/tools/run_tests/sources_and_headers.json index ce94c5d5e89..7b9193734d4 100644 --- a/tools/run_tests/sources_and_headers.json +++ b/tools/run_tests/sources_and_headers.json @@ -5501,6 +5501,7 @@ "test/core/end2end/tests/max_message_length.c", "test/core/end2end/tests/negative_deadline.c", "test/core/end2end/tests/network_status_change.c", + "test/core/end2end/tests/no_logging.c", "test/core/end2end/tests/no_op.c", "test/core/end2end/tests/payload.c", "test/core/end2end/tests/ping.c", @@ -5563,6 +5564,7 @@ "test/core/end2end/tests/max_message_length.c", "test/core/end2end/tests/negative_deadline.c", "test/core/end2end/tests/network_status_change.c", + "test/core/end2end/tests/no_logging.c", "test/core/end2end/tests/no_op.c", "test/core/end2end/tests/payload.c", "test/core/end2end/tests/ping.c", diff --git a/tools/run_tests/tests.json b/tools/run_tests/tests.json index b0c09ace5b2..a59abba391d 100644 --- a/tools/run_tests/tests.json +++ b/tools/run_tests/tests.json @@ -5046,6 +5046,28 @@ "posix" ] }, + { + "args": [ + "no_logging" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_census_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "no_op" @@ -5970,6 +5992,28 @@ "posix" ] }, + { + "args": [ + "no_logging" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_compress_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "no_op" @@ -6867,6 +6911,27 @@ "posix" ] }, + { + "args": [ + "no_logging" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_fakesec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "no_op" @@ -7662,6 +7727,26 @@ "posix" ] }, + { + "args": [ + "no_logging" + ], + "ci_platforms": [ + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_fd_test", + "platforms": [ + "linux", + "mac", + "posix" + ] + }, { "args": [ "no_op" @@ -8516,6 +8601,28 @@ "posix" ] }, + { + "args": [ + "no_logging" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_full_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "no_op" @@ -9278,6 +9385,22 @@ "linux" ] }, + { + "args": [ + "no_logging" + ], + "ci_platforms": [ + "linux" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_full+pipe_test", + "platforms": [ + "linux" + ] + }, { "args": [ "no_op" @@ -11014,6 +11137,28 @@ "posix" ] }, + { + "args": [ + "no_logging" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_load_reporting_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "no_op" @@ -11911,6 +12056,27 @@ "posix" ] }, + { + "args": [ + "no_logging" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_oauth2_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "no_op" @@ -12709,6 +12875,27 @@ "posix" ] }, + { + "args": [ + "no_logging" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_proxy_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "no_op" @@ -13486,6 +13673,27 @@ "posix" ] }, + { + "args": [ + "no_logging" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_sockpair_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "no_op" @@ -15019,6 +15227,27 @@ "posix" ] }, + { + "args": [ + "no_logging" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_sockpair_1byte_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "no_op" @@ -15886,6 +16115,28 @@ "posix" ] }, + { + "args": [ + "no_logging" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_ssl_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "no_op" @@ -16810,6 +17061,28 @@ "posix" ] }, + { + "args": [ + "no_logging" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_ssl_cert_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "no_op" @@ -17623,6 +17896,27 @@ "posix" ] }, + { + "args": [ + "no_logging" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_ssl_proxy_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "no_op" @@ -18416,6 +18710,26 @@ "posix" ] }, + { + "args": [ + "no_logging" + ], + "ci_platforms": [ + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_uds_test", + "platforms": [ + "linux", + "mac", + "posix" + ] + }, { "args": [ "no_op" @@ -19288,6 +19602,28 @@ "posix" ] }, + { + "args": [ + "no_logging" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_census_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "no_op" @@ -20190,6 +20526,28 @@ "posix" ] }, + { + "args": [ + "no_logging" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_compress_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "no_op" @@ -20980,6 +21338,26 @@ "posix" ] }, + { + "args": [ + "no_logging" + ], + "ci_platforms": [ + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_fd_nosec_test", + "platforms": [ + "linux", + "mac", + "posix" + ] + }, { "args": [ "no_op" @@ -21812,6 +22190,28 @@ "posix" ] }, + { + "args": [ + "no_logging" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_full_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "no_op" @@ -22558,6 +22958,22 @@ "linux" ] }, + { + "args": [ + "no_logging" + ], + "ci_platforms": [ + "linux" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_full+pipe_nosec_test", + "platforms": [ + "linux" + ] + }, { "args": [ "no_op" @@ -24250,6 +24666,28 @@ "posix" ] }, + { + "args": [ + "no_logging" + ], + "ci_platforms": [ + "windows", + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_load_reporting_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "no_op" @@ -25042,6 +25480,27 @@ "posix" ] }, + { + "args": [ + "no_logging" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_proxy_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "no_op" @@ -25798,6 +26257,27 @@ "posix" ] }, + { + "args": [ + "no_logging" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_sockpair_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "no_op" @@ -27335,6 +27815,29 @@ "posix" ] }, + { + "args": [ + "no_logging" + ], + "ci_platforms": [ + "windows", + "linux", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [ + "msan" + ], + "flaky": false, + "language": "c", + "name": "h2_sockpair_1byte_nosec_test", + "platforms": [ + "windows", + "linux", + "mac", + "posix" + ] + }, { "args": [ "no_op" @@ -28134,6 +28637,26 @@ "posix" ] }, + { + "args": [ + "no_logging" + ], + "ci_platforms": [ + "linux", + "mac", + "posix" + ], + "cpu_cost": 1.0, + "exclude_configs": [], + "flaky": false, + "language": "c", + "name": "h2_uds_nosec_test", + "platforms": [ + "linux", + "mac", + "posix" + ] + }, { "args": [ "no_op" diff --git a/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj b/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj index a500fe3a397..96c95746d35 100644 --- a/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj +++ b/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj @@ -205,6 +205,8 @@ + + diff --git a/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj.filters b/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj.filters index 6f2294c7503..42f1b8e07a2 100644 --- a/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj.filters +++ b/vsprojects/vcxproj/test/end2end/tests/end2end_nosec_tests/end2end_nosec_tests.vcxproj.filters @@ -82,6 +82,9 @@ test\core\end2end\tests + + test\core\end2end\tests + test\core\end2end\tests diff --git a/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj b/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj index 76140894cad..4336f3689bb 100644 --- a/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj +++ b/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj @@ -207,6 +207,8 @@ + + diff --git a/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj.filters b/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj.filters index db336f09a92..a94dbe873b9 100644 --- a/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj.filters +++ b/vsprojects/vcxproj/test/end2end/tests/end2end_tests/end2end_tests.vcxproj.filters @@ -85,6 +85,9 @@ test\core\end2end\tests + + test\core\end2end\tests + test\core\end2end\tests From 468e16d97ad99c6346e57a51472ed5463c166366 Mon Sep 17 00:00:00 2001 From: David Garcia Quintas Date: Wed, 31 Aug 2016 13:29:40 +0200 Subject: [PATCH 08/13] server builder include fix --- src/cpp/server/server_builder.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/cpp/server/server_builder.cc b/src/cpp/server/server_builder.cc index 45bb858e2ed..87a526fe988 100644 --- a/src/cpp/server/server_builder.cc +++ b/src/cpp/server/server_builder.cc @@ -35,10 +35,9 @@ #include #include -#include #include +#include -#include "include/grpc/support/useful.h" #include "src/cpp/server/thread_pool_interface.h" namespace grpc { From a0c69f9099f5345d7dfe189812329eb841f5006c Mon Sep 17 00:00:00 2001 From: Alistair Veitch Date: Wed, 31 Aug 2016 12:01:27 -0700 Subject: [PATCH 09/13] add trace context proto --- BUILD | 6 ++ CMakeLists.txt | 2 + Makefile | 2 + binding.gyp | 1 + build.yaml | 2 + config.m4 | 1 + gRPC-Core.podspec | 3 + grpc.gemspec | 2 + package.xml | 2 + src/core/ext/census/gen/README.md | 4 + src/core/ext/census/gen/trace_context.pb.c | 81 +++++++++++++++ src/core/ext/census/gen/trace_context.pb.h | 99 +++++++++++++++++++ src/proto/census/trace_context.options | 0 src/proto/census/trace_context.proto | 48 +++++++++ src/python/grpcio/grpc_core_dependencies.py | 1 + tools/doxygen/Doxyfile.core.internal | 2 + tools/run_tests/sources_and_headers.json | 3 + vsprojects/vcxproj/grpc/grpc.vcxproj | 3 + vsprojects/vcxproj/grpc/grpc.vcxproj.filters | 6 ++ .../grpc_unsecure/grpc_unsecure.vcxproj | 3 + .../grpc_unsecure.vcxproj.filters | 6 ++ 21 files changed, 277 insertions(+) create mode 100644 src/core/ext/census/gen/trace_context.pb.c create mode 100644 src/core/ext/census/gen/trace_context.pb.h create mode 100644 src/proto/census/trace_context.options create mode 100644 src/proto/census/trace_context.proto diff --git a/BUILD b/BUILD index 7dc5b6ca398..ac1db59b444 100644 --- a/BUILD +++ b/BUILD @@ -312,6 +312,7 @@ cc_library( "src/core/ext/census/census_interface.h", "src/core/ext/census/census_rpc_stats.h", "src/core/ext/census/gen/census.pb.h", + "src/core/ext/census/gen/trace_context.pb.h", "src/core/ext/census/grpc_filter.h", "src/core/ext/census/mlog.h", "src/core/ext/census/resource.h", @@ -493,6 +494,7 @@ cc_library( "src/core/ext/census/base_resources.c", "src/core/ext/census/context.c", "src/core/ext/census/gen/census.pb.c", + "src/core/ext/census/gen/trace_context.pb.c", "src/core/ext/census/grpc_context.c", "src/core/ext/census/grpc_filter.c", "src/core/ext/census/grpc_plugin.c", @@ -1037,6 +1039,7 @@ cc_library( "src/core/ext/census/census_interface.h", "src/core/ext/census/census_rpc_stats.h", "src/core/ext/census/gen/census.pb.h", + "src/core/ext/census/gen/trace_context.pb.h", "src/core/ext/census/grpc_filter.h", "src/core/ext/census/mlog.h", "src/core/ext/census/resource.h", @@ -1189,6 +1192,7 @@ cc_library( "src/core/ext/census/base_resources.c", "src/core/ext/census/context.c", "src/core/ext/census/gen/census.pb.c", + "src/core/ext/census/gen/trace_context.pb.c", "src/core/ext/census/grpc_context.c", "src/core/ext/census/grpc_filter.c", "src/core/ext/census/grpc_plugin.c", @@ -2339,6 +2343,7 @@ objc_library( "src/core/ext/census/base_resources.c", "src/core/ext/census/context.c", "src/core/ext/census/gen/census.pb.c", + "src/core/ext/census/gen/trace_context.pb.c", "src/core/ext/census/grpc_context.c", "src/core/ext/census/grpc_filter.c", "src/core/ext/census/grpc_plugin.c", @@ -2532,6 +2537,7 @@ objc_library( "src/core/ext/census/census_interface.h", "src/core/ext/census/census_rpc_stats.h", "src/core/ext/census/gen/census.pb.h", + "src/core/ext/census/gen/trace_context.pb.h", "src/core/ext/census/grpc_filter.h", "src/core/ext/census/mlog.h", "src/core/ext/census/resource.h", diff --git a/CMakeLists.txt b/CMakeLists.txt index b8194652519..3e9c44c09ac 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -470,6 +470,7 @@ add_library(grpc src/core/ext/census/base_resources.c src/core/ext/census/context.c src/core/ext/census/gen/census.pb.c + src/core/ext/census/gen/trace_context.pb.c src/core/ext/census/grpc_context.c src/core/ext/census/grpc_filter.c src/core/ext/census/grpc_plugin.c @@ -930,6 +931,7 @@ add_library(grpc_unsecure src/core/ext/census/base_resources.c src/core/ext/census/context.c src/core/ext/census/gen/census.pb.c + src/core/ext/census/gen/trace_context.pb.c src/core/ext/census/grpc_context.c src/core/ext/census/grpc_filter.c src/core/ext/census/grpc_plugin.c diff --git a/Makefile b/Makefile index 4bd4c306c17..53109b02d4a 100644 --- a/Makefile +++ b/Makefile @@ -2686,6 +2686,7 @@ LIBGRPC_SRC = \ src/core/ext/census/base_resources.c \ src/core/ext/census/context.c \ src/core/ext/census/gen/census.pb.c \ + src/core/ext/census/gen/trace_context.pb.c \ src/core/ext/census/grpc_context.c \ src/core/ext/census/grpc_filter.c \ src/core/ext/census/grpc_plugin.c \ @@ -3391,6 +3392,7 @@ LIBGRPC_UNSECURE_SRC = \ src/core/ext/census/base_resources.c \ src/core/ext/census/context.c \ src/core/ext/census/gen/census.pb.c \ + src/core/ext/census/gen/trace_context.pb.c \ src/core/ext/census/grpc_context.c \ src/core/ext/census/grpc_filter.c \ src/core/ext/census/grpc_plugin.c \ diff --git a/binding.gyp b/binding.gyp index a29cfda6fc5..666547c0975 100644 --- a/binding.gyp +++ b/binding.gyp @@ -742,6 +742,7 @@ 'src/core/ext/census/base_resources.c', 'src/core/ext/census/context.c', 'src/core/ext/census/gen/census.pb.c', + 'src/core/ext/census/gen/trace_context.pb.c', 'src/core/ext/census/grpc_context.c', 'src/core/ext/census/grpc_filter.c', 'src/core/ext/census/grpc_plugin.c', diff --git a/build.yaml b/build.yaml index 9b182d2ffcf..8599d6b0824 100644 --- a/build.yaml +++ b/build.yaml @@ -24,6 +24,7 @@ filegroups: - src/core/ext/census/census_interface.h - src/core/ext/census/census_rpc_stats.h - src/core/ext/census/gen/census.pb.h + - src/core/ext/census/gen/trace_context.pb.h - src/core/ext/census/grpc_filter.h - src/core/ext/census/mlog.h - src/core/ext/census/resource.h @@ -32,6 +33,7 @@ filegroups: - src/core/ext/census/base_resources.c - src/core/ext/census/context.c - src/core/ext/census/gen/census.pb.c + - src/core/ext/census/gen/trace_context.pb.c - src/core/ext/census/grpc_context.c - src/core/ext/census/grpc_filter.c - src/core/ext/census/grpc_plugin.c diff --git a/config.m4 b/config.m4 index 5b47074bcd7..1f6abbb6e2c 100644 --- a/config.m4 +++ b/config.m4 @@ -261,6 +261,7 @@ if test "$PHP_GRPC" != "no"; then src/core/ext/census/base_resources.c \ src/core/ext/census/context.c \ src/core/ext/census/gen/census.pb.c \ + src/core/ext/census/gen/trace_context.pb.c \ src/core/ext/census/grpc_context.c \ src/core/ext/census/grpc_filter.c \ src/core/ext/census/grpc_plugin.c \ diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index 41ba1b1058b..900ea414706 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -407,6 +407,7 @@ Pod::Spec.new do |s| 'src/core/ext/census/census_interface.h', 'src/core/ext/census/census_rpc_stats.h', 'src/core/ext/census/gen/census.pb.h', + 'src/core/ext/census/gen/trace_context.pb.h', 'src/core/ext/census/grpc_filter.h', 'src/core/ext/census/mlog.h', 'src/core/ext/census/resource.h', @@ -591,6 +592,7 @@ Pod::Spec.new do |s| 'src/core/ext/census/base_resources.c', 'src/core/ext/census/context.c', 'src/core/ext/census/gen/census.pb.c', + 'src/core/ext/census/gen/trace_context.pb.c', 'src/core/ext/census/grpc_context.c', 'src/core/ext/census/grpc_filter.c', 'src/core/ext/census/grpc_plugin.c', @@ -768,6 +770,7 @@ Pod::Spec.new do |s| 'src/core/ext/census/census_interface.h', 'src/core/ext/census/census_rpc_stats.h', 'src/core/ext/census/gen/census.pb.h', + 'src/core/ext/census/gen/trace_context.pb.h', 'src/core/ext/census/grpc_filter.h', 'src/core/ext/census/mlog.h', 'src/core/ext/census/resource.h', diff --git a/grpc.gemspec b/grpc.gemspec index 95eb28bd1d7..2acde4076c3 100755 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -326,6 +326,7 @@ Gem::Specification.new do |s| s.files += %w( src/core/ext/census/census_interface.h ) s.files += %w( src/core/ext/census/census_rpc_stats.h ) s.files += %w( src/core/ext/census/gen/census.pb.h ) + s.files += %w( src/core/ext/census/gen/trace_context.pb.h ) s.files += %w( src/core/ext/census/grpc_filter.h ) s.files += %w( src/core/ext/census/mlog.h ) s.files += %w( src/core/ext/census/resource.h ) @@ -510,6 +511,7 @@ Gem::Specification.new do |s| s.files += %w( src/core/ext/census/base_resources.c ) s.files += %w( src/core/ext/census/context.c ) s.files += %w( src/core/ext/census/gen/census.pb.c ) + s.files += %w( src/core/ext/census/gen/trace_context.pb.c ) s.files += %w( src/core/ext/census/grpc_context.c ) s.files += %w( src/core/ext/census/grpc_filter.c ) s.files += %w( src/core/ext/census/grpc_plugin.c ) diff --git a/package.xml b/package.xml index 1df4940530d..15707352f03 100644 --- a/package.xml +++ b/package.xml @@ -334,6 +334,7 @@ + @@ -518,6 +519,7 @@ + diff --git a/src/core/ext/census/gen/README.md b/src/core/ext/census/gen/README.md index 72bef6542d0..fdbac1084cd 100644 --- a/src/core/ext/census/gen/README.md +++ b/src/core/ext/census/gen/README.md @@ -4,3 +4,7 @@ Files generated for use by Census stats and trace recording subsystem. * census.pb.{h,c} - Generated from src/core/ext/census/census.proto, using the script `tools/codegen/core/gen_nano_proto.sh src/proto/census/census.proto $PWD/src/core/ext/census/gen src/core/ext/census/gen` +* trace_context.pb.{h,c} - Generated from + src/core/ext/census/trace_context.proto, using the script + `tools/codegen/core/gen_nano_proto.sh src/proto/census/trace_context.proto + $PWD/src/core/ext/census/gen src/core/ext/census/gen` diff --git a/src/core/ext/census/gen/trace_context.pb.c b/src/core/ext/census/gen/trace_context.pb.c new file mode 100644 index 00000000000..c8aea324cee --- /dev/null +++ b/src/core/ext/census/gen/trace_context.pb.c @@ -0,0 +1,81 @@ +/* + * + * Copyright 2016, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ +/* Automatically generated nanopb constant definitions */ +/* Generated by nanopb-0.3.5-dev */ + +#include "src/core/ext/census/gen/trace_context.pb.h" + +#if PB_PROTO_HEADER_VERSION != 30 +#error Regenerate this file with the current version of nanopb generator. +#endif + + + +const pb_field_t google_trace_TraceId_fields[3] = { + PB_FIELD( 1, FIXED64 , OPTIONAL, STATIC , FIRST, google_trace_TraceId, hi, hi, 0), + PB_FIELD( 2, FIXED64 , OPTIONAL, STATIC , OTHER, google_trace_TraceId, lo, hi, 0), + PB_LAST_FIELD +}; + +const pb_field_t google_trace_TraceContext_fields[4] = { + PB_FIELD( 1, MESSAGE , OPTIONAL, STATIC , FIRST, google_trace_TraceContext, trace_id, trace_id, &google_trace_TraceId_fields), + PB_FIELD( 2, FIXED64 , OPTIONAL, STATIC , OTHER, google_trace_TraceContext, span_id, trace_id, 0), + PB_FIELD( 3, BOOL , OPTIONAL, STATIC , OTHER, google_trace_TraceContext, is_sampled, span_id, 0), + PB_LAST_FIELD +}; + + +/* Check that field information fits in pb_field_t */ +#if !defined(PB_FIELD_32BIT) +/* If you get an error here, it means that you need to define PB_FIELD_32BIT + * compile-time option. You can do that in pb.h or on compiler command line. + * + * The reason you need to do this is that some of your messages contain tag + * numbers or field sizes that are larger than what can fit in 8 or 16 bit + * field descriptors. + */ +PB_STATIC_ASSERT((pb_membersize(google_trace_TraceContext, trace_id) < 65536), YOU_MUST_DEFINE_PB_FIELD_32BIT_FOR_MESSAGES_google_trace_TraceId_google_trace_TraceContext) +#endif + +#if !defined(PB_FIELD_16BIT) && !defined(PB_FIELD_32BIT) +/* If you get an error here, it means that you need to define PB_FIELD_16BIT + * compile-time option. You can do that in pb.h or on compiler command line. + * + * The reason you need to do this is that some of your messages contain tag + * numbers or field sizes that are larger than what can fit in the default + * 8 bit descriptors. + */ +PB_STATIC_ASSERT((pb_membersize(google_trace_TraceContext, trace_id) < 256), YOU_MUST_DEFINE_PB_FIELD_16BIT_FOR_MESSAGES_google_trace_TraceId_google_trace_TraceContext) +#endif + + diff --git a/src/core/ext/census/gen/trace_context.pb.h b/src/core/ext/census/gen/trace_context.pb.h new file mode 100644 index 00000000000..263c4c58cbf --- /dev/null +++ b/src/core/ext/census/gen/trace_context.pb.h @@ -0,0 +1,99 @@ +/* + * + * Copyright 2016, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ +/* Automatically generated nanopb header */ +/* Generated by nanopb-0.3.5-dev */ + +#ifndef GRPC_CORE_EXT_CENSUS_GEN_TRACE_CONTEXT_PB_H +#define GRPC_CORE_EXT_CENSUS_GEN_TRACE_CONTEXT_PB_H +#include "third_party/nanopb/pb.h" +#if PB_PROTO_HEADER_VERSION != 30 +#error Regenerate this file with the current version of nanopb generator. +#endif + +#ifdef __cplusplus +extern "C" { +#endif + +/* Struct definitions */ +typedef struct _google_trace_TraceId { + bool has_hi; + uint64_t hi; + bool has_lo; + uint64_t lo; +} google_trace_TraceId; + +typedef struct _google_trace_TraceContext { + bool has_trace_id; + google_trace_TraceId trace_id; + bool has_span_id; + uint64_t span_id; + bool has_is_sampled; + bool is_sampled; +} google_trace_TraceContext; + +/* Default values for struct fields */ + +/* Initializer values for message structs */ +#define google_trace_TraceId_init_default {false, 0, false, 0} +#define google_trace_TraceContext_init_default {false, google_trace_TraceId_init_default, false, 0, false, 0} +#define google_trace_TraceId_init_zero {false, 0, false, 0} +#define google_trace_TraceContext_init_zero {false, google_trace_TraceId_init_zero, false, 0, false, 0} + +/* Field tags (for use in manual encoding/decoding) */ +#define google_trace_TraceId_hi_tag 1 +#define google_trace_TraceId_lo_tag 2 +#define google_trace_TraceContext_trace_id_tag 1 +#define google_trace_TraceContext_span_id_tag 2 +#define google_trace_TraceContext_is_sampled_tag 3 + +/* Struct field encoding specification for nanopb */ +extern const pb_field_t google_trace_TraceId_fields[3]; +extern const pb_field_t google_trace_TraceContext_fields[4]; + +/* Maximum encoded size of messages (where known) */ +#define google_trace_TraceId_size 18 +#define google_trace_TraceContext_size 31 + +/* Message IDs (where set with "msgid" option) */ +#ifdef PB_MSGID + +#define TRACE_CONTEXT_MESSAGES \ + + +#endif + +#ifdef __cplusplus +} /* extern "C" */ +#endif + +#endif diff --git a/src/proto/census/trace_context.options b/src/proto/census/trace_context.options new file mode 100644 index 00000000000..e69de29bb2d diff --git a/src/proto/census/trace_context.proto b/src/proto/census/trace_context.proto new file mode 100644 index 00000000000..a5d5a9595db --- /dev/null +++ b/src/proto/census/trace_context.proto @@ -0,0 +1,48 @@ +// Copyright 2016, Google Inc. +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +syntax = "proto3"; + +package google.trace; + +// A TraceId uniquely represents a single Trace. It is a 128-bit nonce. +message TraceId { + fixed64 hi = 1; + fixed64 lo = 2; +} + +// Tracing information that is propagated with RPC's. +message TraceContext { + // Trace identifer. Must be present. + TraceId trace_id = 1; + // ID of parent (client) span. Must be present. + fixed64 span_id = 2; + // true if this trace is sampled. + bool is_sampled = 3; +} diff --git a/src/python/grpcio/grpc_core_dependencies.py b/src/python/grpcio/grpc_core_dependencies.py index 660e34d7420..c3db2f98aeb 100644 --- a/src/python/grpcio/grpc_core_dependencies.py +++ b/src/python/grpcio/grpc_core_dependencies.py @@ -255,6 +255,7 @@ CORE_SOURCE_FILES = [ 'src/core/ext/census/base_resources.c', 'src/core/ext/census/context.c', 'src/core/ext/census/gen/census.pb.c', + 'src/core/ext/census/gen/trace_context.pb.c', 'src/core/ext/census/grpc_context.c', 'src/core/ext/census/grpc_filter.c', 'src/core/ext/census/grpc_plugin.c', diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index 77f951af50e..cc07b930834 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -945,6 +945,7 @@ src/core/ext/census/base_resources.h \ src/core/ext/census/census_interface.h \ src/core/ext/census/census_rpc_stats.h \ src/core/ext/census/gen/census.pb.h \ +src/core/ext/census/gen/trace_context.pb.h \ src/core/ext/census/grpc_filter.h \ src/core/ext/census/mlog.h \ src/core/ext/census/resource.h \ @@ -1129,6 +1130,7 @@ src/core/ext/load_reporting/load_reporting_filter.c \ src/core/ext/census/base_resources.c \ src/core/ext/census/context.c \ src/core/ext/census/gen/census.pb.c \ +src/core/ext/census/gen/trace_context.pb.c \ src/core/ext/census/grpc_context.c \ src/core/ext/census/grpc_filter.c \ src/core/ext/census/grpc_plugin.c \ diff --git a/tools/run_tests/sources_and_headers.json b/tools/run_tests/sources_and_headers.json index ce94c5d5e89..004ff4164ed 100644 --- a/tools/run_tests/sources_and_headers.json +++ b/tools/run_tests/sources_and_headers.json @@ -5595,6 +5595,7 @@ "src/core/ext/census/census_interface.h", "src/core/ext/census/census_rpc_stats.h", "src/core/ext/census/gen/census.pb.h", + "src/core/ext/census/gen/trace_context.pb.h", "src/core/ext/census/grpc_filter.h", "src/core/ext/census/mlog.h", "src/core/ext/census/resource.h", @@ -5612,6 +5613,8 @@ "src/core/ext/census/context.c", "src/core/ext/census/gen/census.pb.c", "src/core/ext/census/gen/census.pb.h", + "src/core/ext/census/gen/trace_context.pb.c", + "src/core/ext/census/gen/trace_context.pb.h", "src/core/ext/census/grpc_context.c", "src/core/ext/census/grpc_filter.c", "src/core/ext/census/grpc_filter.h", diff --git a/vsprojects/vcxproj/grpc/grpc.vcxproj b/vsprojects/vcxproj/grpc/grpc.vcxproj index d805fdfdee5..6cceb17eb83 100644 --- a/vsprojects/vcxproj/grpc/grpc.vcxproj +++ b/vsprojects/vcxproj/grpc/grpc.vcxproj @@ -454,6 +454,7 @@ + @@ -820,6 +821,8 @@ + + diff --git a/vsprojects/vcxproj/grpc/grpc.vcxproj.filters b/vsprojects/vcxproj/grpc/grpc.vcxproj.filters index 991bbec9e25..16b2ed860fa 100644 --- a/vsprojects/vcxproj/grpc/grpc.vcxproj.filters +++ b/vsprojects/vcxproj/grpc/grpc.vcxproj.filters @@ -541,6 +541,9 @@ src\core\ext\census\gen + + src\core\ext\census\gen + src\core\ext\census @@ -1130,6 +1133,9 @@ src\core\ext\census\gen + + src\core\ext\census\gen + src\core\ext\census diff --git a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj index 66ec41d2c6a..0af04979804 100644 --- a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj +++ b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj @@ -420,6 +420,7 @@ + @@ -728,6 +729,8 @@ + + diff --git a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters index 6f2dc3571e8..9f8727f5b4d 100644 --- a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters +++ b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters @@ -454,6 +454,9 @@ src\core\ext\census\gen + + src\core\ext\census\gen + src\core\ext\census @@ -968,6 +971,9 @@ src\core\ext\census\gen + + src\core\ext\census\gen + src\core\ext\census From 4c0fe49f76e0e3d7e7ceed5722cc43010af61ab3 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Wed, 31 Aug 2016 13:51:55 -0700 Subject: [PATCH 10/13] Move subchannel_call_holder code into client_channel module. --- BUILD | 8 - CMakeLists.txt | 3 - Makefile | 3 - binding.gyp | 1 - build.yaml | 2 - config.m4 | 1 - gRPC-Core.podspec | 3 - grpc.gemspec | 2 - package.xml | 2 - src/core/ext/client_config/client_channel.c | 322 ++++++++++++++++-- .../client_config/subchannel_call_holder.c | 292 ---------------- .../client_config/subchannel_call_holder.h | 99 ------ src/python/grpcio/grpc_core_dependencies.py | 1 - tools/doxygen/Doxyfile.core.internal | 2 - tools/run_tests/sources_and_headers.json | 3 - vsprojects/vcxproj/grpc/grpc.vcxproj | 3 - vsprojects/vcxproj/grpc/grpc.vcxproj.filters | 6 - .../grpc_unsecure/grpc_unsecure.vcxproj | 3 - .../grpc_unsecure.vcxproj.filters | 6 - 19 files changed, 290 insertions(+), 472 deletions(-) delete mode 100644 src/core/ext/client_config/subchannel_call_holder.c delete mode 100644 src/core/ext/client_config/subchannel_call_holder.h diff --git a/BUILD b/BUILD index 7dc5b6ca398..927b6e532a5 100644 --- a/BUILD +++ b/BUILD @@ -299,7 +299,6 @@ cc_library( "src/core/ext/client_config/resolver_registry.h", "src/core/ext/client_config/resolver_result.h", "src/core/ext/client_config/subchannel.h", - "src/core/ext/client_config/subchannel_call_holder.h", "src/core/ext/client_config/subchannel_index.h", "src/core/ext/client_config/uri_parser.h", "src/core/ext/lb_policy/grpclb/grpclb.h", @@ -474,7 +473,6 @@ cc_library( "src/core/ext/client_config/resolver_registry.c", "src/core/ext/client_config/resolver_result.c", "src/core/ext/client_config/subchannel.c", - "src/core/ext/client_config/subchannel_call_holder.c", "src/core/ext/client_config/subchannel_index.c", "src/core/ext/client_config/uri_parser.c", "src/core/ext/transport/chttp2/server/insecure/server_chttp2.c", @@ -671,7 +669,6 @@ cc_library( "src/core/ext/client_config/resolver_registry.h", "src/core/ext/client_config/resolver_result.h", "src/core/ext/client_config/subchannel.h", - "src/core/ext/client_config/subchannel_call_holder.h", "src/core/ext/client_config/subchannel_index.h", "src/core/ext/client_config/uri_parser.h", "src/core/lib/security/context/security_context.h", @@ -830,7 +827,6 @@ cc_library( "src/core/ext/client_config/resolver_registry.c", "src/core/ext/client_config/resolver_result.c", "src/core/ext/client_config/subchannel.c", - "src/core/ext/client_config/subchannel_call_holder.c", "src/core/ext/client_config/subchannel_index.c", "src/core/ext/client_config/uri_parser.c", "src/core/lib/http/httpcli_security_connector.c", @@ -1024,7 +1020,6 @@ cc_library( "src/core/ext/client_config/resolver_registry.h", "src/core/ext/client_config/resolver_result.h", "src/core/ext/client_config/subchannel.h", - "src/core/ext/client_config/subchannel_call_holder.h", "src/core/ext/client_config/subchannel_index.h", "src/core/ext/client_config/uri_parser.h", "src/core/ext/load_reporting/load_reporting.h", @@ -1174,7 +1169,6 @@ cc_library( "src/core/ext/client_config/resolver_registry.c", "src/core/ext/client_config/resolver_result.c", "src/core/ext/client_config/subchannel.c", - "src/core/ext/client_config/subchannel_call_holder.c", "src/core/ext/client_config/subchannel_index.c", "src/core/ext/client_config/uri_parser.c", "src/core/ext/resolver/dns/native/dns_resolver.c", @@ -2320,7 +2314,6 @@ objc_library( "src/core/ext/client_config/resolver_registry.c", "src/core/ext/client_config/resolver_result.c", "src/core/ext/client_config/subchannel.c", - "src/core/ext/client_config/subchannel_call_holder.c", "src/core/ext/client_config/subchannel_index.c", "src/core/ext/client_config/uri_parser.c", "src/core/ext/transport/chttp2/server/insecure/server_chttp2.c", @@ -2519,7 +2512,6 @@ objc_library( "src/core/ext/client_config/resolver_registry.h", "src/core/ext/client_config/resolver_result.h", "src/core/ext/client_config/subchannel.h", - "src/core/ext/client_config/subchannel_call_holder.h", "src/core/ext/client_config/subchannel_index.h", "src/core/ext/client_config/uri_parser.h", "src/core/ext/lb_policy/grpclb/grpclb.h", diff --git a/CMakeLists.txt b/CMakeLists.txt index b8194652519..7b4ec92c40d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -448,7 +448,6 @@ add_library(grpc src/core/ext/client_config/resolver_registry.c src/core/ext/client_config/resolver_result.c src/core/ext/client_config/subchannel.c - src/core/ext/client_config/subchannel_call_holder.c src/core/ext/client_config/subchannel_index.c src/core/ext/client_config/uri_parser.c src/core/ext/transport/chttp2/server/insecure/server_chttp2.c @@ -680,7 +679,6 @@ add_library(grpc_cronet src/core/ext/client_config/resolver_registry.c src/core/ext/client_config/resolver_result.c src/core/ext/client_config/subchannel.c - src/core/ext/client_config/subchannel_call_holder.c src/core/ext/client_config/subchannel_index.c src/core/ext/client_config/uri_parser.c src/core/lib/http/httpcli_security_connector.c @@ -912,7 +910,6 @@ add_library(grpc_unsecure src/core/ext/client_config/resolver_registry.c src/core/ext/client_config/resolver_result.c src/core/ext/client_config/subchannel.c - src/core/ext/client_config/subchannel_call_holder.c src/core/ext/client_config/subchannel_index.c src/core/ext/client_config/uri_parser.c src/core/ext/resolver/dns/native/dns_resolver.c diff --git a/Makefile b/Makefile index 4bd4c306c17..0f8cfdf294d 100644 --- a/Makefile +++ b/Makefile @@ -2664,7 +2664,6 @@ LIBGRPC_SRC = \ src/core/ext/client_config/resolver_registry.c \ src/core/ext/client_config/resolver_result.c \ src/core/ext/client_config/subchannel.c \ - src/core/ext/client_config/subchannel_call_holder.c \ src/core/ext/client_config/subchannel_index.c \ src/core/ext/client_config/uri_parser.c \ src/core/ext/transport/chttp2/server/insecure/server_chttp2.c \ @@ -2914,7 +2913,6 @@ LIBGRPC_CRONET_SRC = \ src/core/ext/client_config/resolver_registry.c \ src/core/ext/client_config/resolver_result.c \ src/core/ext/client_config/subchannel.c \ - src/core/ext/client_config/subchannel_call_holder.c \ src/core/ext/client_config/subchannel_index.c \ src/core/ext/client_config/uri_parser.c \ src/core/lib/http/httpcli_security_connector.c \ @@ -3373,7 +3371,6 @@ LIBGRPC_UNSECURE_SRC = \ src/core/ext/client_config/resolver_registry.c \ src/core/ext/client_config/resolver_result.c \ src/core/ext/client_config/subchannel.c \ - src/core/ext/client_config/subchannel_call_holder.c \ src/core/ext/client_config/subchannel_index.c \ src/core/ext/client_config/uri_parser.c \ src/core/ext/resolver/dns/native/dns_resolver.c \ diff --git a/binding.gyp b/binding.gyp index a29cfda6fc5..76276475df8 100644 --- a/binding.gyp +++ b/binding.gyp @@ -720,7 +720,6 @@ 'src/core/ext/client_config/resolver_registry.c', 'src/core/ext/client_config/resolver_result.c', 'src/core/ext/client_config/subchannel.c', - 'src/core/ext/client_config/subchannel_call_holder.c', 'src/core/ext/client_config/subchannel_index.c', 'src/core/ext/client_config/uri_parser.c', 'src/core/ext/transport/chttp2/server/insecure/server_chttp2.c', diff --git a/build.yaml b/build.yaml index 9b182d2ffcf..1f47a41858a 100644 --- a/build.yaml +++ b/build.yaml @@ -350,7 +350,6 @@ filegroups: - src/core/ext/client_config/resolver_registry.h - src/core/ext/client_config/resolver_result.h - src/core/ext/client_config/subchannel.h - - src/core/ext/client_config/subchannel_call_holder.h - src/core/ext/client_config/subchannel_index.h - src/core/ext/client_config/uri_parser.h src: @@ -370,7 +369,6 @@ filegroups: - src/core/ext/client_config/resolver_registry.c - src/core/ext/client_config/resolver_result.c - src/core/ext/client_config/subchannel.c - - src/core/ext/client_config/subchannel_call_holder.c - src/core/ext/client_config/subchannel_index.c - src/core/ext/client_config/uri_parser.c plugin: grpc_client_config diff --git a/config.m4 b/config.m4 index 5b47074bcd7..038af4569a8 100644 --- a/config.m4 +++ b/config.m4 @@ -239,7 +239,6 @@ if test "$PHP_GRPC" != "no"; then src/core/ext/client_config/resolver_registry.c \ src/core/ext/client_config/resolver_result.c \ src/core/ext/client_config/subchannel.c \ - src/core/ext/client_config/subchannel_call_holder.c \ src/core/ext/client_config/subchannel_index.c \ src/core/ext/client_config/uri_parser.c \ src/core/ext/transport/chttp2/server/insecure/server_chttp2.c \ diff --git a/gRPC-Core.podspec b/gRPC-Core.podspec index 41ba1b1058b..1e58d328018 100644 --- a/gRPC-Core.podspec +++ b/gRPC-Core.podspec @@ -390,7 +390,6 @@ Pod::Spec.new do |s| 'src/core/ext/client_config/resolver_registry.h', 'src/core/ext/client_config/resolver_result.h', 'src/core/ext/client_config/subchannel.h', - 'src/core/ext/client_config/subchannel_call_holder.h', 'src/core/ext/client_config/subchannel_index.h', 'src/core/ext/client_config/uri_parser.h', 'src/core/ext/lb_policy/grpclb/grpclb.h', @@ -569,7 +568,6 @@ Pod::Spec.new do |s| 'src/core/ext/client_config/resolver_registry.c', 'src/core/ext/client_config/resolver_result.c', 'src/core/ext/client_config/subchannel.c', - 'src/core/ext/client_config/subchannel_call_holder.c', 'src/core/ext/client_config/subchannel_index.c', 'src/core/ext/client_config/uri_parser.c', 'src/core/ext/transport/chttp2/server/insecure/server_chttp2.c', @@ -751,7 +749,6 @@ Pod::Spec.new do |s| 'src/core/ext/client_config/resolver_registry.h', 'src/core/ext/client_config/resolver_result.h', 'src/core/ext/client_config/subchannel.h', - 'src/core/ext/client_config/subchannel_call_holder.h', 'src/core/ext/client_config/subchannel_index.h', 'src/core/ext/client_config/uri_parser.h', 'src/core/ext/lb_policy/grpclb/grpclb.h', diff --git a/grpc.gemspec b/grpc.gemspec index 95eb28bd1d7..31a99e9c126 100755 --- a/grpc.gemspec +++ b/grpc.gemspec @@ -309,7 +309,6 @@ Gem::Specification.new do |s| s.files += %w( src/core/ext/client_config/resolver_registry.h ) s.files += %w( src/core/ext/client_config/resolver_result.h ) s.files += %w( src/core/ext/client_config/subchannel.h ) - s.files += %w( src/core/ext/client_config/subchannel_call_holder.h ) s.files += %w( src/core/ext/client_config/subchannel_index.h ) s.files += %w( src/core/ext/client_config/uri_parser.h ) s.files += %w( src/core/ext/lb_policy/grpclb/grpclb.h ) @@ -488,7 +487,6 @@ Gem::Specification.new do |s| s.files += %w( src/core/ext/client_config/resolver_registry.c ) s.files += %w( src/core/ext/client_config/resolver_result.c ) s.files += %w( src/core/ext/client_config/subchannel.c ) - s.files += %w( src/core/ext/client_config/subchannel_call_holder.c ) s.files += %w( src/core/ext/client_config/subchannel_index.c ) s.files += %w( src/core/ext/client_config/uri_parser.c ) s.files += %w( src/core/ext/transport/chttp2/server/insecure/server_chttp2.c ) diff --git a/package.xml b/package.xml index 1df4940530d..39284b7a5ad 100644 --- a/package.xml +++ b/package.xml @@ -317,7 +317,6 @@ - @@ -496,7 +495,6 @@ - diff --git a/src/core/ext/client_config/client_channel.c b/src/core/ext/client_config/client_channel.c index 566d3d5ce49..e4d7527ce8d 100644 --- a/src/core/ext/client_config/client_channel.c +++ b/src/core/ext/client_config/client_channel.c @@ -33,6 +33,7 @@ #include "src/core/ext/client_config/client_channel.h" +#include #include #include @@ -41,10 +42,11 @@ #include #include -#include "src/core/ext/client_config/subchannel_call_holder.h" +#include "src/core/ext/client_config/subchannel.h" #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/channel/connected_channel.h" #include "src/core/lib/iomgr/iomgr.h" +#include "src/core/lib/iomgr/polling_entity.h" #include "src/core/lib/profiling/timers.h" #include "src/core/lib/support/string.h" #include "src/core/lib/surface/channel.h" @@ -52,13 +54,60 @@ /* Client channel implementation */ -typedef grpc_subchannel_call_holder call_data; +#define GET_CALL(call_data) \ + ((grpc_subchannel_call *)(gpr_atm_acq_load(&(call_data)->subchannel_call))) + +#define CANCELLED_CALL ((grpc_subchannel_call *)1) + +/** Picks a subchannel. + Returns true if subchannel is available immediately (in which case on_ready + should not be called), or false otherwise (in which case on_ready should be + called when the subchannel is available) */ +typedef bool (*pick_subchannel_cb)( + grpc_exec_ctx *exec_ctx, void *arg, grpc_metadata_batch *initial_metadata, + uint32_t initial_metadata_flags, + grpc_connected_subchannel **connected_subchannel, grpc_closure *on_ready); + +typedef enum { + GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING, + GRPC_SUBCHANNEL_CALL_HOLDER_PICKING_SUBCHANNEL +} subchannel_creation_phase; + +/** Call data. Holds a pointer to grpc_subchannel_call and the + associated machinery to create such a pointer. + Handles queueing of stream ops until a call object is ready, waiting + for initial metadata before trying to create a call object, + and handling cancellation gracefully. */ +typedef struct client_channel_call_data { + /** either 0 for no call, 1 for cancelled, or a pointer to a + grpc_subchannel_call */ + gpr_atm subchannel_call; + /** Helper function to choose the subchannel on which to create + the call object. Channel filter delegates to the load + balancing policy (once it's ready). */ + pick_subchannel_cb pick_subchannel; + void *pick_subchannel_arg; + + gpr_mu mu; + + subchannel_creation_phase creation_phase; + grpc_connected_subchannel *connected_subchannel; + grpc_polling_entity *pollent; + + grpc_transport_stream_op *waiting_ops; + size_t waiting_ops_count; + size_t waiting_ops_capacity; + + grpc_closure next_step; + + grpc_call_stack *owning_call; +} call_data; typedef struct client_channel_channel_data { /** resolver for this channel */ grpc_resolver *resolver; /** have we started resolving this channel */ - int started_resolving; + bool started_resolving; /** mutex protecting client configuration, including all variables below in this data structure */ @@ -74,7 +123,7 @@ typedef struct client_channel_channel_data { /** connectivity state being tracked */ grpc_connectivity_state_tracker state_tracker; /** when an lb_policy arrives, should we try to exit idle */ - int exit_idle_when_lb_policy_arrives; + bool exit_idle_when_lb_policy_arrives; /** owning stack */ grpc_channel_stack *owning_stack; /** interested parties (owned) */ @@ -82,10 +131,8 @@ typedef struct client_channel_channel_data { } channel_data; /** We create one watcher for each new lb_policy that is returned from a - resolver, - to watch for state changes from the lb_policy. When a state change is seen, - we - update the channel, and create a new watcher */ + resolver, to watch for state changes from the lb_policy. When a state + change is seen, we update the channel, and create a new watcher. */ typedef struct { channel_data *chand; grpc_closure on_changed; @@ -98,15 +145,208 @@ typedef struct { grpc_call_element *elem; } waiting_call; +static void add_waiting_locked(call_data *calld, grpc_transport_stream_op *op) { + GPR_TIMER_BEGIN("add_waiting_locked", 0); + if (calld->waiting_ops_count == calld->waiting_ops_capacity) { + calld->waiting_ops_capacity = GPR_MAX(3, 2 * calld->waiting_ops_capacity); + calld->waiting_ops = + gpr_realloc(calld->waiting_ops, + calld->waiting_ops_capacity * sizeof(*calld->waiting_ops)); + } + calld->waiting_ops[calld->waiting_ops_count++] = *op; + GPR_TIMER_END("add_waiting_locked", 0); +} + +static void fail_locked(grpc_exec_ctx *exec_ctx, call_data *calld, + grpc_error *error) { + size_t i; + for (i = 0; i < calld->waiting_ops_count; i++) { + grpc_transport_stream_op_finish_with_failure( + exec_ctx, &calld->waiting_ops[i], GRPC_ERROR_REF(error)); + } + calld->waiting_ops_count = 0; + GRPC_ERROR_UNREF(error); +} + +typedef struct { + grpc_transport_stream_op *ops; + size_t nops; + grpc_subchannel_call *call; +} retry_ops_args; + +static void retry_ops(grpc_exec_ctx *exec_ctx, void *args, grpc_error *error) { + retry_ops_args *a = args; + size_t i; + for (i = 0; i < a->nops; i++) { + grpc_subchannel_call_process_op(exec_ctx, a->call, &a->ops[i]); + } + GRPC_SUBCHANNEL_CALL_UNREF(exec_ctx, a->call, "retry_ops"); + gpr_free(a->ops); + gpr_free(a); +} + +static void retry_waiting_locked(grpc_exec_ctx *exec_ctx, call_data *calld) { + retry_ops_args *a = gpr_malloc(sizeof(*a)); + a->ops = calld->waiting_ops; + a->nops = calld->waiting_ops_count; + a->call = GET_CALL(calld); + if (a->call == CANCELLED_CALL) { + gpr_free(a); + fail_locked(exec_ctx, calld, GRPC_ERROR_CANCELLED); + return; + } + calld->waiting_ops = NULL; + calld->waiting_ops_count = 0; + calld->waiting_ops_capacity = 0; + GRPC_SUBCHANNEL_CALL_REF(a->call, "retry_ops"); + grpc_exec_ctx_sched(exec_ctx, grpc_closure_create(retry_ops, a), + GRPC_ERROR_NONE, NULL); +} + +static void subchannel_ready(grpc_exec_ctx *exec_ctx, void *arg, + grpc_error *error) { + call_data *calld = arg; + gpr_mu_lock(&calld->mu); + GPR_ASSERT(calld->creation_phase == + GRPC_SUBCHANNEL_CALL_HOLDER_PICKING_SUBCHANNEL); + calld->creation_phase = GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING; + if (calld->connected_subchannel == NULL) { + gpr_atm_no_barrier_store(&calld->subchannel_call, 1); + fail_locked(exec_ctx, calld, GRPC_ERROR_CREATE_REFERENCING( + "Failed to create subchannel", &error, 1)); + } else if (1 == gpr_atm_acq_load(&calld->subchannel_call)) { + /* already cancelled before subchannel became ready */ + fail_locked(exec_ctx, calld, + GRPC_ERROR_CREATE_REFERENCING( + "Cancelled before creating subchannel", &error, 1)); + } else { + grpc_subchannel_call *subchannel_call = NULL; + grpc_error *new_error = grpc_connected_subchannel_create_call( + exec_ctx, calld->connected_subchannel, calld->pollent, + &subchannel_call); + if (new_error != GRPC_ERROR_NONE) { + new_error = grpc_error_add_child(new_error, error); + subchannel_call = CANCELLED_CALL; + fail_locked(exec_ctx, calld, new_error); + } + gpr_atm_rel_store(&calld->subchannel_call, + (gpr_atm)(uintptr_t)subchannel_call); + retry_waiting_locked(exec_ctx, calld); + } + gpr_mu_unlock(&calld->mu); + GRPC_CALL_STACK_UNREF(exec_ctx, calld->owning_call, "pick_subchannel"); +} + static char *cc_get_peer(grpc_exec_ctx *exec_ctx, grpc_call_element *elem) { - return grpc_subchannel_call_holder_get_peer(exec_ctx, elem->call_data); + call_data *calld = elem->call_data; + grpc_subchannel_call *subchannel_call = GET_CALL(calld); + if (subchannel_call == NULL || subchannel_call == CANCELLED_CALL) { + return NULL; + } else { + return grpc_subchannel_call_get_peer(exec_ctx, subchannel_call); + } } +// The logic here is fairly complicated, due to (a) the fact that we +// need to handle the case where we receive the send op before the +// initial metadata op, and (b) the need for efficiency, especially in +// the streaming case. +// TODO(ctiller): Explain this more thoroughly. static void cc_start_transport_stream_op(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, grpc_transport_stream_op *op) { + call_data *calld = elem->call_data; GRPC_CALL_LOG_OP(GPR_INFO, elem, op); - grpc_subchannel_call_holder_perform_op(exec_ctx, elem->call_data, op); + /* try to (atomically) get the call */ + grpc_subchannel_call *call = GET_CALL(calld); + GPR_TIMER_BEGIN("cc_start_transport_stream_op", 0); + if (call == CANCELLED_CALL) { + grpc_transport_stream_op_finish_with_failure(exec_ctx, op, + GRPC_ERROR_CANCELLED); + GPR_TIMER_END("cc_start_transport_stream_op", 0); + return; + } + if (call != NULL) { + grpc_subchannel_call_process_op(exec_ctx, call, op); + GPR_TIMER_END("cc_start_transport_stream_op", 0); + return; + } + /* we failed; lock and figure out what to do */ + gpr_mu_lock(&calld->mu); +retry: + /* need to recheck that another thread hasn't set the call */ + call = GET_CALL(calld); + if (call == CANCELLED_CALL) { + gpr_mu_unlock(&calld->mu); + grpc_transport_stream_op_finish_with_failure(exec_ctx, op, + GRPC_ERROR_CANCELLED); + GPR_TIMER_END("cc_start_transport_stream_op", 0); + return; + } + if (call != NULL) { + gpr_mu_unlock(&calld->mu); + grpc_subchannel_call_process_op(exec_ctx, call, op); + GPR_TIMER_END("cc_start_transport_stream_op", 0); + return; + } + /* if this is a cancellation, then we can raise our cancelled flag */ + if (op->cancel_error != GRPC_ERROR_NONE) { + if (!gpr_atm_rel_cas(&calld->subchannel_call, 0, + (gpr_atm)(uintptr_t)CANCELLED_CALL)) { + goto retry; + } else { + switch (calld->creation_phase) { + case GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING: + fail_locked(exec_ctx, calld, GRPC_ERROR_REF(op->cancel_error)); + break; + case GRPC_SUBCHANNEL_CALL_HOLDER_PICKING_SUBCHANNEL: + calld->pick_subchannel(exec_ctx, calld->pick_subchannel_arg, NULL, 0, + &calld->connected_subchannel, NULL); + break; + } + gpr_mu_unlock(&calld->mu); + grpc_transport_stream_op_finish_with_failure(exec_ctx, op, + GRPC_ERROR_CANCELLED); + GPR_TIMER_END("cc_start_transport_stream_op", 0); + return; + } + } + /* if we don't have a subchannel, try to get one */ + if (calld->creation_phase == GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING && + calld->connected_subchannel == NULL && + op->send_initial_metadata != NULL) { + calld->creation_phase = GRPC_SUBCHANNEL_CALL_HOLDER_PICKING_SUBCHANNEL; + grpc_closure_init(&calld->next_step, subchannel_ready, calld); + GRPC_CALL_STACK_REF(calld->owning_call, "pick_subchannel"); + if (calld->pick_subchannel( + exec_ctx, calld->pick_subchannel_arg, op->send_initial_metadata, + op->send_initial_metadata_flags, &calld->connected_subchannel, + &calld->next_step)) { + calld->creation_phase = GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING; + GRPC_CALL_STACK_UNREF(exec_ctx, calld->owning_call, "pick_subchannel"); + } + } + /* if we've got a subchannel, then let's ask it to create a call */ + if (calld->creation_phase == GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING && + calld->connected_subchannel != NULL) { + grpc_subchannel_call *subchannel_call = NULL; + grpc_error *error = grpc_connected_subchannel_create_call( + exec_ctx, calld->connected_subchannel, calld->pollent, + &subchannel_call); + if (error != GRPC_ERROR_NONE) { + subchannel_call = CANCELLED_CALL; + fail_locked(exec_ctx, calld, GRPC_ERROR_REF(error)); + grpc_transport_stream_op_finish_with_failure(exec_ctx, op, error); + } + gpr_atm_rel_store(&calld->subchannel_call, + (gpr_atm)(uintptr_t)subchannel_call); + retry_waiting_locked(exec_ctx, calld); + goto retry; + } + /* nothing to be done but wait */ + add_waiting_locked(calld, op); + gpr_mu_unlock(&calld->mu); + GPR_TIMER_END("cc_start_transport_stream_op", 0); } static void watch_lb_policy(grpc_exec_ctx *exec_ctx, channel_data *chand, @@ -183,7 +423,7 @@ static void cc_on_resolver_result_changed(grpc_exec_ctx *exec_ctx, void *arg, grpc_lb_policy *lb_policy = NULL; grpc_lb_policy *old_lb_policy; grpc_connectivity_state state = GRPC_CHANNEL_TRANSIENT_FAILURE; - int exit_idle = 0; + bool exit_idle = false; grpc_error *state_error = GRPC_ERROR_CREATE("No load balancing policy"); if (chand->resolver_result != NULL) { @@ -221,8 +461,8 @@ static void cc_on_resolver_result_changed(grpc_exec_ctx *exec_ctx, void *arg, } if (lb_policy != NULL && chand->exit_idle_when_lb_policy_arrives) { GRPC_LB_POLICY_REF(lb_policy, "exit_idle"); - exit_idle = 1; - chand->exit_idle_when_lb_policy_arrives = 0; + exit_idle = true; + chand->exit_idle_when_lb_policy_arrives = false; } if (error == GRPC_ERROR_NONE && chand->resolver) { @@ -339,11 +579,11 @@ typedef struct { grpc_closure closure; } continue_picking_args; -static int cc_pick_subchannel(grpc_exec_ctx *exec_ctx, void *arg, - grpc_metadata_batch *initial_metadata, - uint32_t initial_metadata_flags, - grpc_connected_subchannel **connected_subchannel, - grpc_closure *on_ready); +static bool cc_pick_subchannel(grpc_exec_ctx *exec_ctx, void *arg, + grpc_metadata_batch *initial_metadata, + uint32_t initial_metadata_flags, + grpc_connected_subchannel **connected_subchannel, + grpc_closure *on_ready); static void continue_picking(grpc_exec_ctx *exec_ctx, void *arg, grpc_error *error) { @@ -360,11 +600,11 @@ static void continue_picking(grpc_exec_ctx *exec_ctx, void *arg, gpr_free(cpa); } -static int cc_pick_subchannel(grpc_exec_ctx *exec_ctx, void *elemp, - grpc_metadata_batch *initial_metadata, - uint32_t initial_metadata_flags, - grpc_connected_subchannel **connected_subchannel, - grpc_closure *on_ready) { +static bool cc_pick_subchannel(grpc_exec_ctx *exec_ctx, void *elemp, + grpc_metadata_batch *initial_metadata, + uint32_t initial_metadata_flags, + grpc_connected_subchannel **connected_subchannel, + grpc_closure *on_ready) { GPR_TIMER_BEGIN("cc_pick_subchannel", 0); grpc_call_element *elem = elemp; @@ -392,7 +632,7 @@ static int cc_pick_subchannel(grpc_exec_ctx *exec_ctx, void *elemp, } gpr_mu_unlock(&chand->mu); GPR_TIMER_END("cc_pick_subchannel", 0); - return 1; + return true; } if (chand->lb_policy != NULL) { grpc_lb_policy *lb_policy = chand->lb_policy; @@ -407,7 +647,7 @@ static int cc_pick_subchannel(grpc_exec_ctx *exec_ctx, void *elemp, return r; } if (chand->resolver != NULL && !chand->started_resolving) { - chand->started_resolving = 1; + chand->started_resolving = true; GRPC_CHANNEL_STACK_REF(chand->owning_stack, "resolver"); grpc_resolver_next(exec_ctx, chand->resolver, &chand->resolver_result, &chand->on_resolver_result_changed); @@ -429,15 +669,25 @@ static int cc_pick_subchannel(grpc_exec_ctx *exec_ctx, void *elemp, gpr_mu_unlock(&chand->mu); GPR_TIMER_END("cc_pick_subchannel", 0); - return 0; + return false; } /* Constructor for call_data */ static grpc_error *init_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, grpc_call_element_args *args) { - grpc_subchannel_call_holder_init(elem->call_data, cc_pick_subchannel, elem, - args->call_stack); + call_data *calld = elem->call_data; + gpr_atm_rel_store(&calld->subchannel_call, 0); + calld->pick_subchannel = cc_pick_subchannel; + calld->pick_subchannel_arg = elem; + gpr_mu_init(&calld->mu); + calld->connected_subchannel = NULL; + calld->waiting_ops = NULL; + calld->waiting_ops_count = 0; + calld->waiting_ops_capacity = 0; + calld->creation_phase = GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING; + calld->owning_call = args->call_stack; + calld->pollent = NULL; return GRPC_ERROR_NONE; } @@ -445,7 +695,15 @@ static grpc_error *init_call_elem(grpc_exec_ctx *exec_ctx, static void destroy_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, const grpc_call_final_info *final_info, void *and_free_memory) { - grpc_subchannel_call_holder_destroy(exec_ctx, elem->call_data); + call_data *calld = elem->call_data; + grpc_subchannel_call *call = GET_CALL(calld); + if (call != NULL && call != CANCELLED_CALL) { + GRPC_SUBCHANNEL_CALL_UNREF(exec_ctx, call, "client_channel_destroy_call"); + } + GPR_ASSERT(calld->creation_phase == GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING); + gpr_mu_destroy(&calld->mu); + GPR_ASSERT(calld->waiting_ops_count == 0); + gpr_free(calld->waiting_ops); gpr_free(and_free_memory); } @@ -523,7 +781,7 @@ void grpc_client_channel_set_resolver(grpc_exec_ctx *exec_ctx, GRPC_RESOLVER_REF(resolver, "channel"); if (!grpc_closure_list_empty(chand->waiting_for_config_closures) || chand->exit_idle_when_lb_policy_arrives) { - chand->started_resolving = 1; + chand->started_resolving = true; GRPC_CHANNEL_STACK_REF(chand->owning_stack, "resolver"); grpc_resolver_next(exec_ctx, resolver, &chand->resolver_result, &chand->on_resolver_result_changed); @@ -541,10 +799,10 @@ grpc_connectivity_state grpc_client_channel_check_connectivity_state( if (chand->lb_policy != NULL) { grpc_lb_policy_exit_idle(exec_ctx, chand->lb_policy); } else { - chand->exit_idle_when_lb_policy_arrives = 1; + chand->exit_idle_when_lb_policy_arrives = true; if (!chand->started_resolving && chand->resolver != NULL) { GRPC_CHANNEL_STACK_REF(chand->owning_stack, "resolver"); - chand->started_resolving = 1; + chand->started_resolving = true; grpc_resolver_next(exec_ctx, chand->resolver, &chand->resolver_result, &chand->on_resolver_result_changed); } diff --git a/src/core/ext/client_config/subchannel_call_holder.c b/src/core/ext/client_config/subchannel_call_holder.c deleted file mode 100644 index be6d054af4e..00000000000 --- a/src/core/ext/client_config/subchannel_call_holder.c +++ /dev/null @@ -1,292 +0,0 @@ -/* - * - * Copyright 2015, Google Inc. - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are - * met: - * - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above - * copyright notice, this list of conditions and the following disclaimer - * in the documentation and/or other materials provided with the - * distribution. - * * Neither the name of Google Inc. nor the names of its - * contributors may be used to endorse or promote products derived from - * this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR - * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT - * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, - * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT - * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, - * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY - * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE - * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - * - */ - -#include "src/core/ext/client_config/subchannel_call_holder.h" - -#include - -#include "src/core/lib/profiling/timers.h" - -#define GET_CALL(holder) \ - ((grpc_subchannel_call *)(gpr_atm_acq_load(&(holder)->subchannel_call))) - -#define CANCELLED_CALL ((grpc_subchannel_call *)1) - -static void subchannel_ready(grpc_exec_ctx *exec_ctx, void *holder, - grpc_error *error); -static void retry_ops(grpc_exec_ctx *exec_ctx, void *retry_ops_args, - grpc_error *error); - -static void add_waiting_locked(grpc_subchannel_call_holder *holder, - grpc_transport_stream_op *op); -static void fail_locked(grpc_exec_ctx *exec_ctx, - grpc_subchannel_call_holder *holder, grpc_error *error); -static void retry_waiting_locked(grpc_exec_ctx *exec_ctx, - grpc_subchannel_call_holder *holder); - -void grpc_subchannel_call_holder_init( - grpc_subchannel_call_holder *holder, - grpc_subchannel_call_holder_pick_subchannel pick_subchannel, - void *pick_subchannel_arg, grpc_call_stack *owning_call) { - gpr_atm_rel_store(&holder->subchannel_call, 0); - holder->pick_subchannel = pick_subchannel; - holder->pick_subchannel_arg = pick_subchannel_arg; - gpr_mu_init(&holder->mu); - holder->connected_subchannel = NULL; - holder->waiting_ops = NULL; - holder->waiting_ops_count = 0; - holder->waiting_ops_capacity = 0; - holder->creation_phase = GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING; - holder->owning_call = owning_call; - holder->pollent = NULL; -} - -void grpc_subchannel_call_holder_destroy(grpc_exec_ctx *exec_ctx, - grpc_subchannel_call_holder *holder) { - grpc_subchannel_call *call = GET_CALL(holder); - if (call != NULL && call != CANCELLED_CALL) { - GRPC_SUBCHANNEL_CALL_UNREF(exec_ctx, call, "holder"); - } - GPR_ASSERT(holder->creation_phase == - GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING); - gpr_mu_destroy(&holder->mu); - GPR_ASSERT(holder->waiting_ops_count == 0); - gpr_free(holder->waiting_ops); -} - -// The logic here is fairly complicated, due to (a) the fact that we -// need to handle the case where we receive the send op before the -// initial metadata op, and (b) the need for efficiency, especially in -// the streaming case. -// TODO(ctiller): Explain this more thoroughly. -void grpc_subchannel_call_holder_perform_op(grpc_exec_ctx *exec_ctx, - grpc_subchannel_call_holder *holder, - grpc_transport_stream_op *op) { - /* try to (atomically) get the call */ - grpc_subchannel_call *call = GET_CALL(holder); - GPR_TIMER_BEGIN("grpc_subchannel_call_holder_perform_op", 0); - if (call == CANCELLED_CALL) { - grpc_transport_stream_op_finish_with_failure(exec_ctx, op, - GRPC_ERROR_CANCELLED); - GPR_TIMER_END("grpc_subchannel_call_holder_perform_op", 0); - return; - } - if (call != NULL) { - grpc_subchannel_call_process_op(exec_ctx, call, op); - GPR_TIMER_END("grpc_subchannel_call_holder_perform_op", 0); - return; - } - /* we failed; lock and figure out what to do */ - gpr_mu_lock(&holder->mu); -retry: - /* need to recheck that another thread hasn't set the call */ - call = GET_CALL(holder); - if (call == CANCELLED_CALL) { - gpr_mu_unlock(&holder->mu); - grpc_transport_stream_op_finish_with_failure(exec_ctx, op, - GRPC_ERROR_CANCELLED); - GPR_TIMER_END("grpc_subchannel_call_holder_perform_op", 0); - return; - } - if (call != NULL) { - gpr_mu_unlock(&holder->mu); - grpc_subchannel_call_process_op(exec_ctx, call, op); - GPR_TIMER_END("grpc_subchannel_call_holder_perform_op", 0); - return; - } - /* if this is a cancellation, then we can raise our cancelled flag */ - if (op->cancel_error != GRPC_ERROR_NONE) { - if (!gpr_atm_rel_cas(&holder->subchannel_call, 0, - (gpr_atm)(uintptr_t)CANCELLED_CALL)) { - goto retry; - } else { - switch (holder->creation_phase) { - case GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING: - fail_locked(exec_ctx, holder, GRPC_ERROR_REF(op->cancel_error)); - break; - case GRPC_SUBCHANNEL_CALL_HOLDER_PICKING_SUBCHANNEL: - holder->pick_subchannel(exec_ctx, holder->pick_subchannel_arg, NULL, - 0, &holder->connected_subchannel, NULL); - break; - } - gpr_mu_unlock(&holder->mu); - grpc_transport_stream_op_finish_with_failure(exec_ctx, op, - GRPC_ERROR_CANCELLED); - GPR_TIMER_END("grpc_subchannel_call_holder_perform_op", 0); - return; - } - } - /* if we don't have a subchannel, try to get one */ - if (holder->creation_phase == GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING && - holder->connected_subchannel == NULL && - op->send_initial_metadata != NULL) { - holder->creation_phase = GRPC_SUBCHANNEL_CALL_HOLDER_PICKING_SUBCHANNEL; - grpc_closure_init(&holder->next_step, subchannel_ready, holder); - GRPC_CALL_STACK_REF(holder->owning_call, "pick_subchannel"); - if (holder->pick_subchannel( - exec_ctx, holder->pick_subchannel_arg, op->send_initial_metadata, - op->send_initial_metadata_flags, &holder->connected_subchannel, - &holder->next_step)) { - holder->creation_phase = GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING; - GRPC_CALL_STACK_UNREF(exec_ctx, holder->owning_call, "pick_subchannel"); - } - } - /* if we've got a subchannel, then let's ask it to create a call */ - if (holder->creation_phase == GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING && - holder->connected_subchannel != NULL) { - grpc_subchannel_call *subchannel_call = NULL; - grpc_error *error = grpc_connected_subchannel_create_call( - exec_ctx, holder->connected_subchannel, holder->pollent, - &subchannel_call); - if (error != GRPC_ERROR_NONE) { - subchannel_call = CANCELLED_CALL; - fail_locked(exec_ctx, holder, GRPC_ERROR_REF(error)); - grpc_transport_stream_op_finish_with_failure(exec_ctx, op, error); - } - gpr_atm_rel_store(&holder->subchannel_call, - (gpr_atm)(uintptr_t)subchannel_call); - retry_waiting_locked(exec_ctx, holder); - goto retry; - } - /* nothing to be done but wait */ - add_waiting_locked(holder, op); - gpr_mu_unlock(&holder->mu); - GPR_TIMER_END("grpc_subchannel_call_holder_perform_op", 0); -} - -static void subchannel_ready(grpc_exec_ctx *exec_ctx, void *arg, - grpc_error *error) { - grpc_subchannel_call_holder *holder = arg; - gpr_mu_lock(&holder->mu); - GPR_ASSERT(holder->creation_phase == - GRPC_SUBCHANNEL_CALL_HOLDER_PICKING_SUBCHANNEL); - holder->creation_phase = GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING; - if (holder->connected_subchannel == NULL) { - gpr_atm_no_barrier_store(&holder->subchannel_call, 1); - fail_locked(exec_ctx, holder, - GRPC_ERROR_CREATE_REFERENCING("Failed to create subchannel", - &error, 1)); - } else if (1 == gpr_atm_acq_load(&holder->subchannel_call)) { - /* already cancelled before subchannel became ready */ - fail_locked(exec_ctx, holder, - GRPC_ERROR_CREATE_REFERENCING( - "Cancelled before creating subchannel", &error, 1)); - } else { - grpc_subchannel_call *subchannel_call = NULL; - grpc_error *new_error = grpc_connected_subchannel_create_call( - exec_ctx, holder->connected_subchannel, holder->pollent, - &subchannel_call); - if (new_error != GRPC_ERROR_NONE) { - new_error = grpc_error_add_child(new_error, error); - subchannel_call = CANCELLED_CALL; - fail_locked(exec_ctx, holder, new_error); - } - gpr_atm_rel_store(&holder->subchannel_call, - (gpr_atm)(uintptr_t)subchannel_call); - retry_waiting_locked(exec_ctx, holder); - } - gpr_mu_unlock(&holder->mu); - GRPC_CALL_STACK_UNREF(exec_ctx, holder->owning_call, "pick_subchannel"); -} - -typedef struct { - grpc_transport_stream_op *ops; - size_t nops; - grpc_subchannel_call *call; -} retry_ops_args; - -static void retry_waiting_locked(grpc_exec_ctx *exec_ctx, - grpc_subchannel_call_holder *holder) { - retry_ops_args *a = gpr_malloc(sizeof(*a)); - a->ops = holder->waiting_ops; - a->nops = holder->waiting_ops_count; - a->call = GET_CALL(holder); - if (a->call == CANCELLED_CALL) { - gpr_free(a); - fail_locked(exec_ctx, holder, GRPC_ERROR_CANCELLED); - return; - } - holder->waiting_ops = NULL; - holder->waiting_ops_count = 0; - holder->waiting_ops_capacity = 0; - GRPC_SUBCHANNEL_CALL_REF(a->call, "retry_ops"); - grpc_exec_ctx_sched(exec_ctx, grpc_closure_create(retry_ops, a), - GRPC_ERROR_NONE, NULL); -} - -static void retry_ops(grpc_exec_ctx *exec_ctx, void *args, grpc_error *error) { - retry_ops_args *a = args; - size_t i; - for (i = 0; i < a->nops; i++) { - grpc_subchannel_call_process_op(exec_ctx, a->call, &a->ops[i]); - } - GRPC_SUBCHANNEL_CALL_UNREF(exec_ctx, a->call, "retry_ops"); - gpr_free(a->ops); - gpr_free(a); -} - -static void add_waiting_locked(grpc_subchannel_call_holder *holder, - grpc_transport_stream_op *op) { - GPR_TIMER_BEGIN("add_waiting_locked", 0); - if (holder->waiting_ops_count == holder->waiting_ops_capacity) { - holder->waiting_ops_capacity = GPR_MAX(3, 2 * holder->waiting_ops_capacity); - holder->waiting_ops = - gpr_realloc(holder->waiting_ops, holder->waiting_ops_capacity * - sizeof(*holder->waiting_ops)); - } - holder->waiting_ops[holder->waiting_ops_count++] = *op; - GPR_TIMER_END("add_waiting_locked", 0); -} - -static void fail_locked(grpc_exec_ctx *exec_ctx, - grpc_subchannel_call_holder *holder, - grpc_error *error) { - size_t i; - for (i = 0; i < holder->waiting_ops_count; i++) { - grpc_transport_stream_op_finish_with_failure( - exec_ctx, &holder->waiting_ops[i], GRPC_ERROR_REF(error)); - } - holder->waiting_ops_count = 0; - GRPC_ERROR_UNREF(error); -} - -char *grpc_subchannel_call_holder_get_peer( - grpc_exec_ctx *exec_ctx, grpc_subchannel_call_holder *holder) { - grpc_subchannel_call *subchannel_call = GET_CALL(holder); - - if (subchannel_call == NULL || subchannel_call == CANCELLED_CALL) { - return NULL; - } else { - return grpc_subchannel_call_get_peer(exec_ctx, subchannel_call); - } -} diff --git a/src/core/ext/client_config/subchannel_call_holder.h b/src/core/ext/client_config/subchannel_call_holder.h deleted file mode 100644 index 8d2deb02f38..00000000000 --- a/src/core/ext/client_config/subchannel_call_holder.h +++ /dev/null @@ -1,99 +0,0 @@ -/* - * - * Copyright 2015, Google Inc. - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are - * met: - * - * * Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * * Redistributions in binary form must reproduce the above - * copyright notice, this list of conditions and the following disclaimer - * in the documentation and/or other materials provided with the - * distribution. - * * Neither the name of Google Inc. nor the names of its - * contributors may be used to endorse or promote products derived from - * this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS - * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT - * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR - * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT - * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, - * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT - * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, - * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY - * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE - * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - * - */ - -#ifndef GRPC_CORE_EXT_CLIENT_CONFIG_SUBCHANNEL_CALL_HOLDER_H -#define GRPC_CORE_EXT_CLIENT_CONFIG_SUBCHANNEL_CALL_HOLDER_H - -#include "src/core/ext/client_config/subchannel.h" -#include "src/core/lib/iomgr/polling_entity.h" - -/** Pick a subchannel for grpc_subchannel_call_holder; - Return 1 if subchannel is available immediately (in which case on_ready - should not be called), or 0 otherwise (in which case on_ready should be - called when the subchannel is available) */ -typedef int (*grpc_subchannel_call_holder_pick_subchannel)( - grpc_exec_ctx *exec_ctx, void *arg, grpc_metadata_batch *initial_metadata, - uint32_t initial_metadata_flags, - grpc_connected_subchannel **connected_subchannel, grpc_closure *on_ready); - -typedef enum { - GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING, - GRPC_SUBCHANNEL_CALL_HOLDER_PICKING_SUBCHANNEL -} grpc_subchannel_call_holder_creation_phase; - -/** Wrapper for holding a pointer to grpc_subchannel_call, and the - associated machinery to create such a pointer. - Handles queueing of stream ops until a call object is ready, waiting - for initial metadata before trying to create a call object, - and handling cancellation gracefully. - - The channel filter uses this as their call_data. */ -typedef struct grpc_subchannel_call_holder { - /** either 0 for no call, 1 for cancelled, or a pointer to a - grpc_subchannel_call */ - gpr_atm subchannel_call; - /** Helper function to choose the subchannel on which to create - the call object. Channel filter delegates to the load - balancing policy (once it's ready). */ - grpc_subchannel_call_holder_pick_subchannel pick_subchannel; - void *pick_subchannel_arg; - - gpr_mu mu; - - grpc_subchannel_call_holder_creation_phase creation_phase; - grpc_connected_subchannel *connected_subchannel; - grpc_polling_entity *pollent; - - grpc_transport_stream_op *waiting_ops; - size_t waiting_ops_count; - size_t waiting_ops_capacity; - - grpc_closure next_step; - - grpc_call_stack *owning_call; -} grpc_subchannel_call_holder; - -void grpc_subchannel_call_holder_init( - grpc_subchannel_call_holder *holder, - grpc_subchannel_call_holder_pick_subchannel pick_subchannel, - void *pick_subchannel_arg, grpc_call_stack *owning_call); -void grpc_subchannel_call_holder_destroy(grpc_exec_ctx *exec_ctx, - grpc_subchannel_call_holder *holder); - -void grpc_subchannel_call_holder_perform_op(grpc_exec_ctx *exec_ctx, - grpc_subchannel_call_holder *holder, - grpc_transport_stream_op *op); -char *grpc_subchannel_call_holder_get_peer(grpc_exec_ctx *exec_ctx, - grpc_subchannel_call_holder *holder); - -#endif /* GRPC_CORE_EXT_CLIENT_CONFIG_SUBCHANNEL_CALL_HOLDER_H */ diff --git a/src/python/grpcio/grpc_core_dependencies.py b/src/python/grpcio/grpc_core_dependencies.py index 660e34d7420..a3ff66224a2 100644 --- a/src/python/grpcio/grpc_core_dependencies.py +++ b/src/python/grpcio/grpc_core_dependencies.py @@ -233,7 +233,6 @@ CORE_SOURCE_FILES = [ 'src/core/ext/client_config/resolver_registry.c', 'src/core/ext/client_config/resolver_result.c', 'src/core/ext/client_config/subchannel.c', - 'src/core/ext/client_config/subchannel_call_holder.c', 'src/core/ext/client_config/subchannel_index.c', 'src/core/ext/client_config/uri_parser.c', 'src/core/ext/transport/chttp2/server/insecure/server_chttp2.c', diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index 77f951af50e..94ade7787e9 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -928,7 +928,6 @@ src/core/ext/client_config/resolver_factory.h \ src/core/ext/client_config/resolver_registry.h \ src/core/ext/client_config/resolver_result.h \ src/core/ext/client_config/subchannel.h \ -src/core/ext/client_config/subchannel_call_holder.h \ src/core/ext/client_config/subchannel_index.h \ src/core/ext/client_config/uri_parser.h \ src/core/ext/lb_policy/grpclb/grpclb.h \ @@ -1107,7 +1106,6 @@ src/core/ext/client_config/resolver_factory.c \ src/core/ext/client_config/resolver_registry.c \ src/core/ext/client_config/resolver_result.c \ src/core/ext/client_config/subchannel.c \ -src/core/ext/client_config/subchannel_call_holder.c \ src/core/ext/client_config/subchannel_index.c \ src/core/ext/client_config/uri_parser.c \ src/core/ext/transport/chttp2/server/insecure/server_chttp2.c \ diff --git a/tools/run_tests/sources_and_headers.json b/tools/run_tests/sources_and_headers.json index ce94c5d5e89..b6b220f449f 100644 --- a/tools/run_tests/sources_and_headers.json +++ b/tools/run_tests/sources_and_headers.json @@ -6100,7 +6100,6 @@ "src/core/ext/client_config/resolver_registry.h", "src/core/ext/client_config/resolver_result.h", "src/core/ext/client_config/subchannel.h", - "src/core/ext/client_config/subchannel_call_holder.h", "src/core/ext/client_config/subchannel_index.h", "src/core/ext/client_config/uri_parser.h" ], @@ -6136,8 +6135,6 @@ "src/core/ext/client_config/resolver_result.h", "src/core/ext/client_config/subchannel.c", "src/core/ext/client_config/subchannel.h", - "src/core/ext/client_config/subchannel_call_holder.c", - "src/core/ext/client_config/subchannel_call_holder.h", "src/core/ext/client_config/subchannel_index.c", "src/core/ext/client_config/subchannel_index.h", "src/core/ext/client_config/uri_parser.c", diff --git a/vsprojects/vcxproj/grpc/grpc.vcxproj b/vsprojects/vcxproj/grpc/grpc.vcxproj index d805fdfdee5..9dcb76c48bc 100644 --- a/vsprojects/vcxproj/grpc/grpc.vcxproj +++ b/vsprojects/vcxproj/grpc/grpc.vcxproj @@ -437,7 +437,6 @@ - @@ -776,8 +775,6 @@ - - diff --git a/vsprojects/vcxproj/grpc/grpc.vcxproj.filters b/vsprojects/vcxproj/grpc/grpc.vcxproj.filters index 991bbec9e25..637dade8e1f 100644 --- a/vsprojects/vcxproj/grpc/grpc.vcxproj.filters +++ b/vsprojects/vcxproj/grpc/grpc.vcxproj.filters @@ -475,9 +475,6 @@ src\core\ext\client_config - - src\core\ext\client_config - src\core\ext\client_config @@ -1079,9 +1076,6 @@ src\core\ext\client_config - - src\core\ext\client_config - src\core\ext\client_config diff --git a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj index 66ec41d2c6a..e77ea8dd720 100644 --- a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj +++ b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj @@ -403,7 +403,6 @@ - @@ -692,8 +691,6 @@ - - diff --git a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters index 6f2dc3571e8..d97f8248c91 100644 --- a/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters +++ b/vsprojects/vcxproj/grpc_unsecure/grpc_unsecure.vcxproj.filters @@ -400,9 +400,6 @@ src\core\ext\client_config - - src\core\ext\client_config - src\core\ext\client_config @@ -917,9 +914,6 @@ src\core\ext\client_config - - src\core\ext\client_config - src\core\ext\client_config From 5c51aadf5728bf61caabe6d88dd46d815252526f Mon Sep 17 00:00:00 2001 From: Yuchen Zeng Date: Wed, 31 Aug 2016 18:18:14 -0700 Subject: [PATCH 11/13] Update CQ_EXPECT_COMPLETION --- test/core/end2end/tests/no_logging.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/core/end2end/tests/no_logging.c b/test/core/end2end/tests/no_logging.c index aa96118fc31..3c40e5dbacd 100644 --- a/test/core/end2end/tests/no_logging.c +++ b/test/core/end2end/tests/no_logging.c @@ -181,7 +181,7 @@ static void simple_request_body(grpc_end2end_test_fixture f) { 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_EXPECT_COMPLETION(cqv, tag(101), 1); cq_verify(cqv); peer = grpc_call_get_peer(s); @@ -213,8 +213,8 @@ static void simple_request_body(grpc_end2end_test_fixture f) { error = grpc_call_start_batch(s, ops, (size_t)(op - ops), tag(102), NULL); GPR_ASSERT(GRPC_CALL_OK == error); - cq_expect_completion(cqv, tag(102), 1); - cq_expect_completion(cqv, tag(1), 1); + CQ_EXPECT_COMPLETION(cqv, tag(102), 1); + CQ_EXPECT_COMPLETION(cqv, tag(1), 1); cq_verify(cqv); GPR_ASSERT(status == GRPC_STATUS_UNIMPLEMENTED); From 2a5959f2aeca2e86cc752288d890bc10b24c55d0 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Thu, 1 Sep 2016 08:20:27 -0700 Subject: [PATCH 12/13] A bit more cleanup. --- src/core/ext/client_config/client_channel.c | 679 ++++++++++---------- 1 file changed, 335 insertions(+), 344 deletions(-) diff --git a/src/core/ext/client_config/client_channel.c b/src/core/ext/client_config/client_channel.c index e4d7527ce8d..fff80abb802 100644 --- a/src/core/ext/client_config/client_channel.c +++ b/src/core/ext/client_config/client_channel.c @@ -54,54 +54,9 @@ /* Client channel implementation */ -#define GET_CALL(call_data) \ - ((grpc_subchannel_call *)(gpr_atm_acq_load(&(call_data)->subchannel_call))) - -#define CANCELLED_CALL ((grpc_subchannel_call *)1) - -/** Picks a subchannel. - Returns true if subchannel is available immediately (in which case on_ready - should not be called), or false otherwise (in which case on_ready should be - called when the subchannel is available) */ -typedef bool (*pick_subchannel_cb)( - grpc_exec_ctx *exec_ctx, void *arg, grpc_metadata_batch *initial_metadata, - uint32_t initial_metadata_flags, - grpc_connected_subchannel **connected_subchannel, grpc_closure *on_ready); - -typedef enum { - GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING, - GRPC_SUBCHANNEL_CALL_HOLDER_PICKING_SUBCHANNEL -} subchannel_creation_phase; - -/** Call data. Holds a pointer to grpc_subchannel_call and the - associated machinery to create such a pointer. - Handles queueing of stream ops until a call object is ready, waiting - for initial metadata before trying to create a call object, - and handling cancellation gracefully. */ -typedef struct client_channel_call_data { - /** either 0 for no call, 1 for cancelled, or a pointer to a - grpc_subchannel_call */ - gpr_atm subchannel_call; - /** Helper function to choose the subchannel on which to create - the call object. Channel filter delegates to the load - balancing policy (once it's ready). */ - pick_subchannel_cb pick_subchannel; - void *pick_subchannel_arg; - - gpr_mu mu; - - subchannel_creation_phase creation_phase; - grpc_connected_subchannel *connected_subchannel; - grpc_polling_entity *pollent; - - grpc_transport_stream_op *waiting_ops; - size_t waiting_ops_count; - size_t waiting_ops_capacity; - - grpc_closure next_step; - - grpc_call_stack *owning_call; -} call_data; +/************************************************************************* + * CHANNEL-WIDE FUNCTIONS + */ typedef struct client_channel_channel_data { /** resolver for this channel */ @@ -140,215 +95,6 @@ typedef struct { grpc_lb_policy *lb_policy; } lb_policy_connectivity_watcher; -typedef struct { - grpc_closure closure; - grpc_call_element *elem; -} waiting_call; - -static void add_waiting_locked(call_data *calld, grpc_transport_stream_op *op) { - GPR_TIMER_BEGIN("add_waiting_locked", 0); - if (calld->waiting_ops_count == calld->waiting_ops_capacity) { - calld->waiting_ops_capacity = GPR_MAX(3, 2 * calld->waiting_ops_capacity); - calld->waiting_ops = - gpr_realloc(calld->waiting_ops, - calld->waiting_ops_capacity * sizeof(*calld->waiting_ops)); - } - calld->waiting_ops[calld->waiting_ops_count++] = *op; - GPR_TIMER_END("add_waiting_locked", 0); -} - -static void fail_locked(grpc_exec_ctx *exec_ctx, call_data *calld, - grpc_error *error) { - size_t i; - for (i = 0; i < calld->waiting_ops_count; i++) { - grpc_transport_stream_op_finish_with_failure( - exec_ctx, &calld->waiting_ops[i], GRPC_ERROR_REF(error)); - } - calld->waiting_ops_count = 0; - GRPC_ERROR_UNREF(error); -} - -typedef struct { - grpc_transport_stream_op *ops; - size_t nops; - grpc_subchannel_call *call; -} retry_ops_args; - -static void retry_ops(grpc_exec_ctx *exec_ctx, void *args, grpc_error *error) { - retry_ops_args *a = args; - size_t i; - for (i = 0; i < a->nops; i++) { - grpc_subchannel_call_process_op(exec_ctx, a->call, &a->ops[i]); - } - GRPC_SUBCHANNEL_CALL_UNREF(exec_ctx, a->call, "retry_ops"); - gpr_free(a->ops); - gpr_free(a); -} - -static void retry_waiting_locked(grpc_exec_ctx *exec_ctx, call_data *calld) { - retry_ops_args *a = gpr_malloc(sizeof(*a)); - a->ops = calld->waiting_ops; - a->nops = calld->waiting_ops_count; - a->call = GET_CALL(calld); - if (a->call == CANCELLED_CALL) { - gpr_free(a); - fail_locked(exec_ctx, calld, GRPC_ERROR_CANCELLED); - return; - } - calld->waiting_ops = NULL; - calld->waiting_ops_count = 0; - calld->waiting_ops_capacity = 0; - GRPC_SUBCHANNEL_CALL_REF(a->call, "retry_ops"); - grpc_exec_ctx_sched(exec_ctx, grpc_closure_create(retry_ops, a), - GRPC_ERROR_NONE, NULL); -} - -static void subchannel_ready(grpc_exec_ctx *exec_ctx, void *arg, - grpc_error *error) { - call_data *calld = arg; - gpr_mu_lock(&calld->mu); - GPR_ASSERT(calld->creation_phase == - GRPC_SUBCHANNEL_CALL_HOLDER_PICKING_SUBCHANNEL); - calld->creation_phase = GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING; - if (calld->connected_subchannel == NULL) { - gpr_atm_no_barrier_store(&calld->subchannel_call, 1); - fail_locked(exec_ctx, calld, GRPC_ERROR_CREATE_REFERENCING( - "Failed to create subchannel", &error, 1)); - } else if (1 == gpr_atm_acq_load(&calld->subchannel_call)) { - /* already cancelled before subchannel became ready */ - fail_locked(exec_ctx, calld, - GRPC_ERROR_CREATE_REFERENCING( - "Cancelled before creating subchannel", &error, 1)); - } else { - grpc_subchannel_call *subchannel_call = NULL; - grpc_error *new_error = grpc_connected_subchannel_create_call( - exec_ctx, calld->connected_subchannel, calld->pollent, - &subchannel_call); - if (new_error != GRPC_ERROR_NONE) { - new_error = grpc_error_add_child(new_error, error); - subchannel_call = CANCELLED_CALL; - fail_locked(exec_ctx, calld, new_error); - } - gpr_atm_rel_store(&calld->subchannel_call, - (gpr_atm)(uintptr_t)subchannel_call); - retry_waiting_locked(exec_ctx, calld); - } - gpr_mu_unlock(&calld->mu); - GRPC_CALL_STACK_UNREF(exec_ctx, calld->owning_call, "pick_subchannel"); -} - -static char *cc_get_peer(grpc_exec_ctx *exec_ctx, grpc_call_element *elem) { - call_data *calld = elem->call_data; - grpc_subchannel_call *subchannel_call = GET_CALL(calld); - if (subchannel_call == NULL || subchannel_call == CANCELLED_CALL) { - return NULL; - } else { - return grpc_subchannel_call_get_peer(exec_ctx, subchannel_call); - } -} - -// The logic here is fairly complicated, due to (a) the fact that we -// need to handle the case where we receive the send op before the -// initial metadata op, and (b) the need for efficiency, especially in -// the streaming case. -// TODO(ctiller): Explain this more thoroughly. -static void cc_start_transport_stream_op(grpc_exec_ctx *exec_ctx, - grpc_call_element *elem, - grpc_transport_stream_op *op) { - call_data *calld = elem->call_data; - GRPC_CALL_LOG_OP(GPR_INFO, elem, op); - /* try to (atomically) get the call */ - grpc_subchannel_call *call = GET_CALL(calld); - GPR_TIMER_BEGIN("cc_start_transport_stream_op", 0); - if (call == CANCELLED_CALL) { - grpc_transport_stream_op_finish_with_failure(exec_ctx, op, - GRPC_ERROR_CANCELLED); - GPR_TIMER_END("cc_start_transport_stream_op", 0); - return; - } - if (call != NULL) { - grpc_subchannel_call_process_op(exec_ctx, call, op); - GPR_TIMER_END("cc_start_transport_stream_op", 0); - return; - } - /* we failed; lock and figure out what to do */ - gpr_mu_lock(&calld->mu); -retry: - /* need to recheck that another thread hasn't set the call */ - call = GET_CALL(calld); - if (call == CANCELLED_CALL) { - gpr_mu_unlock(&calld->mu); - grpc_transport_stream_op_finish_with_failure(exec_ctx, op, - GRPC_ERROR_CANCELLED); - GPR_TIMER_END("cc_start_transport_stream_op", 0); - return; - } - if (call != NULL) { - gpr_mu_unlock(&calld->mu); - grpc_subchannel_call_process_op(exec_ctx, call, op); - GPR_TIMER_END("cc_start_transport_stream_op", 0); - return; - } - /* if this is a cancellation, then we can raise our cancelled flag */ - if (op->cancel_error != GRPC_ERROR_NONE) { - if (!gpr_atm_rel_cas(&calld->subchannel_call, 0, - (gpr_atm)(uintptr_t)CANCELLED_CALL)) { - goto retry; - } else { - switch (calld->creation_phase) { - case GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING: - fail_locked(exec_ctx, calld, GRPC_ERROR_REF(op->cancel_error)); - break; - case GRPC_SUBCHANNEL_CALL_HOLDER_PICKING_SUBCHANNEL: - calld->pick_subchannel(exec_ctx, calld->pick_subchannel_arg, NULL, 0, - &calld->connected_subchannel, NULL); - break; - } - gpr_mu_unlock(&calld->mu); - grpc_transport_stream_op_finish_with_failure(exec_ctx, op, - GRPC_ERROR_CANCELLED); - GPR_TIMER_END("cc_start_transport_stream_op", 0); - return; - } - } - /* if we don't have a subchannel, try to get one */ - if (calld->creation_phase == GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING && - calld->connected_subchannel == NULL && - op->send_initial_metadata != NULL) { - calld->creation_phase = GRPC_SUBCHANNEL_CALL_HOLDER_PICKING_SUBCHANNEL; - grpc_closure_init(&calld->next_step, subchannel_ready, calld); - GRPC_CALL_STACK_REF(calld->owning_call, "pick_subchannel"); - if (calld->pick_subchannel( - exec_ctx, calld->pick_subchannel_arg, op->send_initial_metadata, - op->send_initial_metadata_flags, &calld->connected_subchannel, - &calld->next_step)) { - calld->creation_phase = GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING; - GRPC_CALL_STACK_UNREF(exec_ctx, calld->owning_call, "pick_subchannel"); - } - } - /* if we've got a subchannel, then let's ask it to create a call */ - if (calld->creation_phase == GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING && - calld->connected_subchannel != NULL) { - grpc_subchannel_call *subchannel_call = NULL; - grpc_error *error = grpc_connected_subchannel_create_call( - exec_ctx, calld->connected_subchannel, calld->pollent, - &subchannel_call); - if (error != GRPC_ERROR_NONE) { - subchannel_call = CANCELLED_CALL; - fail_locked(exec_ctx, calld, GRPC_ERROR_REF(error)); - grpc_transport_stream_op_finish_with_failure(exec_ctx, op, error); - } - gpr_atm_rel_store(&calld->subchannel_call, - (gpr_atm)(uintptr_t)subchannel_call); - retry_waiting_locked(exec_ctx, calld); - goto retry; - } - /* nothing to be done but wait */ - add_waiting_locked(calld, op); - gpr_mu_unlock(&calld->mu); - GPR_TIMER_END("cc_start_transport_stream_op", 0); -} - static void watch_lb_policy(grpc_exec_ctx *exec_ctx, channel_data *chand, grpc_lb_policy *lb_policy, grpc_connectivity_state current_state); @@ -417,8 +163,8 @@ static void watch_lb_policy(grpc_exec_ctx *exec_ctx, channel_data *chand, &w->on_changed); } -static void cc_on_resolver_result_changed(grpc_exec_ctx *exec_ctx, void *arg, - grpc_error *error) { +static void on_resolver_result_changed(grpc_exec_ctx *exec_ctx, void *arg, + grpc_error *error) { channel_data *chand = arg; grpc_lb_policy *lb_policy = NULL; grpc_lb_policy *old_lb_policy; @@ -570,44 +316,225 @@ static void cc_start_transport_op(grpc_exec_ctx *exec_ctx, gpr_mu_unlock(&chand->mu); } -typedef struct { - grpc_metadata_batch *initial_metadata; - uint32_t initial_metadata_flags; - grpc_connected_subchannel **connected_subchannel; - grpc_closure *on_ready; - grpc_call_element *elem; - grpc_closure closure; -} continue_picking_args; +/* Constructor for channel_data */ +static void cc_init_channel_elem(grpc_exec_ctx *exec_ctx, + grpc_channel_element *elem, + grpc_channel_element_args *args) { + channel_data *chand = elem->channel_data; -static bool cc_pick_subchannel(grpc_exec_ctx *exec_ctx, void *arg, - grpc_metadata_batch *initial_metadata, - uint32_t initial_metadata_flags, - grpc_connected_subchannel **connected_subchannel, - grpc_closure *on_ready); + memset(chand, 0, sizeof(*chand)); -static void continue_picking(grpc_exec_ctx *exec_ctx, void *arg, - grpc_error *error) { - continue_picking_args *cpa = arg; - if (cpa->connected_subchannel == NULL) { + GPR_ASSERT(args->is_last); + GPR_ASSERT(elem->filter == &grpc_client_channel_filter); + + gpr_mu_init(&chand->mu); + grpc_closure_init(&chand->on_resolver_result_changed, + on_resolver_result_changed, chand); + chand->owning_stack = args->channel_stack; + + grpc_connectivity_state_init(&chand->state_tracker, GRPC_CHANNEL_IDLE, + "client_channel"); + chand->interested_parties = grpc_pollset_set_create(); +} + +/* Destructor for channel_data */ +static void cc_destroy_channel_elem(grpc_exec_ctx *exec_ctx, + grpc_channel_element *elem) { + channel_data *chand = elem->channel_data; + + if (chand->resolver != NULL) { + grpc_resolver_shutdown(exec_ctx, chand->resolver); + GRPC_RESOLVER_UNREF(exec_ctx, chand->resolver, "channel"); + } + if (chand->lb_policy != NULL) { + grpc_pollset_set_del_pollset_set(exec_ctx, + chand->lb_policy->interested_parties, + chand->interested_parties); + GRPC_LB_POLICY_UNREF(exec_ctx, chand->lb_policy, "channel"); + } + grpc_connectivity_state_destroy(exec_ctx, &chand->state_tracker); + grpc_pollset_set_destroy(chand->interested_parties); + gpr_mu_destroy(&chand->mu); +} + +/************************************************************************* + * PER-CALL FUNCTIONS + */ + +#define GET_CALL(call_data) \ + ((grpc_subchannel_call *)(gpr_atm_acq_load(&(call_data)->subchannel_call))) + +#define CANCELLED_CALL ((grpc_subchannel_call *)1) + +typedef enum { + GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING, + GRPC_SUBCHANNEL_CALL_HOLDER_PICKING_SUBCHANNEL +} subchannel_creation_phase; + +/** Call data. Holds a pointer to grpc_subchannel_call and the + associated machinery to create such a pointer. + Handles queueing of stream ops until a call object is ready, waiting + for initial metadata before trying to create a call object, + and handling cancellation gracefully. */ +typedef struct client_channel_call_data { + /** either 0 for no call, 1 for cancelled, or a pointer to a + grpc_subchannel_call */ + gpr_atm subchannel_call; + + gpr_mu mu; + + subchannel_creation_phase creation_phase; + grpc_connected_subchannel *connected_subchannel; + grpc_polling_entity *pollent; + + grpc_transport_stream_op *waiting_ops; + size_t waiting_ops_count; + size_t waiting_ops_capacity; + + grpc_closure next_step; + + grpc_call_stack *owning_call; +} call_data; + +static void add_waiting_locked(call_data *calld, grpc_transport_stream_op *op) { + GPR_TIMER_BEGIN("add_waiting_locked", 0); + if (calld->waiting_ops_count == calld->waiting_ops_capacity) { + calld->waiting_ops_capacity = GPR_MAX(3, 2 * calld->waiting_ops_capacity); + calld->waiting_ops = + gpr_realloc(calld->waiting_ops, + calld->waiting_ops_capacity * sizeof(*calld->waiting_ops)); + } + calld->waiting_ops[calld->waiting_ops_count++] = *op; + GPR_TIMER_END("add_waiting_locked", 0); +} + +static void fail_locked(grpc_exec_ctx *exec_ctx, call_data *calld, + grpc_error *error) { + size_t i; + for (i = 0; i < calld->waiting_ops_count; i++) { + grpc_transport_stream_op_finish_with_failure( + exec_ctx, &calld->waiting_ops[i], GRPC_ERROR_REF(error)); + } + calld->waiting_ops_count = 0; + GRPC_ERROR_UNREF(error); +} + +typedef struct { + grpc_transport_stream_op *ops; + size_t nops; + grpc_subchannel_call *call; +} retry_ops_args; + +static void retry_ops(grpc_exec_ctx *exec_ctx, void *args, grpc_error *error) { + retry_ops_args *a = args; + size_t i; + for (i = 0; i < a->nops; i++) { + grpc_subchannel_call_process_op(exec_ctx, a->call, &a->ops[i]); + } + GRPC_SUBCHANNEL_CALL_UNREF(exec_ctx, a->call, "retry_ops"); + gpr_free(a->ops); + gpr_free(a); +} + +static void retry_waiting_locked(grpc_exec_ctx *exec_ctx, call_data *calld) { + retry_ops_args *a = gpr_malloc(sizeof(*a)); + a->ops = calld->waiting_ops; + a->nops = calld->waiting_ops_count; + a->call = GET_CALL(calld); + if (a->call == CANCELLED_CALL) { + gpr_free(a); + fail_locked(exec_ctx, calld, GRPC_ERROR_CANCELLED); + return; + } + calld->waiting_ops = NULL; + calld->waiting_ops_count = 0; + calld->waiting_ops_capacity = 0; + GRPC_SUBCHANNEL_CALL_REF(a->call, "retry_ops"); + grpc_exec_ctx_sched(exec_ctx, grpc_closure_create(retry_ops, a), + GRPC_ERROR_NONE, NULL); +} + +static void subchannel_ready(grpc_exec_ctx *exec_ctx, void *arg, + grpc_error *error) { + call_data *calld = arg; + gpr_mu_lock(&calld->mu); + GPR_ASSERT(calld->creation_phase == + GRPC_SUBCHANNEL_CALL_HOLDER_PICKING_SUBCHANNEL); + calld->creation_phase = GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING; + if (calld->connected_subchannel == NULL) { + gpr_atm_no_barrier_store(&calld->subchannel_call, 1); + fail_locked(exec_ctx, calld, GRPC_ERROR_CREATE_REFERENCING( + "Failed to create subchannel", &error, 1)); + } else if (1 == gpr_atm_acq_load(&calld->subchannel_call)) { + /* already cancelled before subchannel became ready */ + fail_locked(exec_ctx, calld, + GRPC_ERROR_CREATE_REFERENCING( + "Cancelled before creating subchannel", &error, 1)); + } else { + grpc_subchannel_call *subchannel_call = NULL; + grpc_error *new_error = grpc_connected_subchannel_create_call( + exec_ctx, calld->connected_subchannel, calld->pollent, + &subchannel_call); + if (new_error != GRPC_ERROR_NONE) { + new_error = grpc_error_add_child(new_error, error); + subchannel_call = CANCELLED_CALL; + fail_locked(exec_ctx, calld, new_error); + } + gpr_atm_rel_store(&calld->subchannel_call, + (gpr_atm)(uintptr_t)subchannel_call); + retry_waiting_locked(exec_ctx, calld); + } + gpr_mu_unlock(&calld->mu); + GRPC_CALL_STACK_UNREF(exec_ctx, calld->owning_call, "pick_subchannel"); +} + +static char *cc_get_peer(grpc_exec_ctx *exec_ctx, grpc_call_element *elem) { + call_data *calld = elem->call_data; + grpc_subchannel_call *subchannel_call = GET_CALL(calld); + if (subchannel_call == NULL || subchannel_call == CANCELLED_CALL) { + return NULL; + } else { + return grpc_subchannel_call_get_peer(exec_ctx, subchannel_call); + } +} + +typedef struct { + grpc_metadata_batch *initial_metadata; + uint32_t initial_metadata_flags; + grpc_connected_subchannel **connected_subchannel; + grpc_closure *on_ready; + grpc_call_element *elem; + grpc_closure closure; +} continue_picking_args; + +static bool pick_subchannel(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, + grpc_metadata_batch *initial_metadata, + uint32_t initial_metadata_flags, + grpc_connected_subchannel **connected_subchannel, + grpc_closure *on_ready); + +static void continue_picking(grpc_exec_ctx *exec_ctx, void *arg, + grpc_error *error) { + continue_picking_args *cpa = arg; + if (cpa->connected_subchannel == NULL) { /* cancelled, do nothing */ } else if (error != GRPC_ERROR_NONE) { grpc_exec_ctx_sched(exec_ctx, cpa->on_ready, GRPC_ERROR_REF(error), NULL); - } else if (cc_pick_subchannel(exec_ctx, cpa->elem, cpa->initial_metadata, - cpa->initial_metadata_flags, - cpa->connected_subchannel, cpa->on_ready)) { + } else if (pick_subchannel(exec_ctx, cpa->elem, cpa->initial_metadata, + cpa->initial_metadata_flags, + cpa->connected_subchannel, cpa->on_ready)) { grpc_exec_ctx_sched(exec_ctx, cpa->on_ready, GRPC_ERROR_NONE, NULL); } gpr_free(cpa); } -static bool cc_pick_subchannel(grpc_exec_ctx *exec_ctx, void *elemp, - grpc_metadata_batch *initial_metadata, - uint32_t initial_metadata_flags, - grpc_connected_subchannel **connected_subchannel, - grpc_closure *on_ready) { - GPR_TIMER_BEGIN("cc_pick_subchannel", 0); +static bool pick_subchannel(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, + grpc_metadata_batch *initial_metadata, + uint32_t initial_metadata_flags, + grpc_connected_subchannel **connected_subchannel, + grpc_closure *on_ready) { + GPR_TIMER_BEGIN("pick_subchannel", 0); - grpc_call_element *elem = elemp; channel_data *chand = elem->channel_data; call_data *calld = elem->call_data; continue_picking_args *cpa; @@ -631,19 +558,19 @@ static bool cc_pick_subchannel(grpc_exec_ctx *exec_ctx, void *elemp, } } gpr_mu_unlock(&chand->mu); - GPR_TIMER_END("cc_pick_subchannel", 0); + GPR_TIMER_END("pick_subchannel", 0); return true; } if (chand->lb_policy != NULL) { grpc_lb_policy *lb_policy = chand->lb_policy; int r; - GRPC_LB_POLICY_REF(lb_policy, "cc_pick_subchannel"); + GRPC_LB_POLICY_REF(lb_policy, "pick_subchannel"); gpr_mu_unlock(&chand->mu); r = grpc_lb_policy_pick(exec_ctx, lb_policy, calld->pollent, initial_metadata, initial_metadata_flags, connected_subchannel, on_ready); - GRPC_LB_POLICY_UNREF(exec_ctx, lb_policy, "cc_pick_subchannel"); - GPR_TIMER_END("cc_pick_subchannel", 0); + GRPC_LB_POLICY_UNREF(exec_ctx, lb_policy, "pick_subchannel"); + GPR_TIMER_END("pick_subchannel", 0); return r; } if (chand->resolver != NULL && !chand->started_resolving) { @@ -668,18 +595,118 @@ static bool cc_pick_subchannel(grpc_exec_ctx *exec_ctx, void *elemp, } gpr_mu_unlock(&chand->mu); - GPR_TIMER_END("cc_pick_subchannel", 0); + GPR_TIMER_END("pick_subchannel", 0); return false; } +// The logic here is fairly complicated, due to (a) the fact that we +// need to handle the case where we receive the send op before the +// initial metadata op, and (b) the need for efficiency, especially in +// the streaming case. +// TODO(ctiller): Explain this more thoroughly. +static void cc_start_transport_stream_op(grpc_exec_ctx *exec_ctx, + grpc_call_element *elem, + grpc_transport_stream_op *op) { + call_data *calld = elem->call_data; + GRPC_CALL_LOG_OP(GPR_INFO, elem, op); + /* try to (atomically) get the call */ + grpc_subchannel_call *call = GET_CALL(calld); + GPR_TIMER_BEGIN("cc_start_transport_stream_op", 0); + if (call == CANCELLED_CALL) { + grpc_transport_stream_op_finish_with_failure(exec_ctx, op, + GRPC_ERROR_CANCELLED); + GPR_TIMER_END("cc_start_transport_stream_op", 0); + return; + } + if (call != NULL) { + grpc_subchannel_call_process_op(exec_ctx, call, op); + GPR_TIMER_END("cc_start_transport_stream_op", 0); + return; + } + /* we failed; lock and figure out what to do */ + gpr_mu_lock(&calld->mu); +retry: + /* need to recheck that another thread hasn't set the call */ + call = GET_CALL(calld); + if (call == CANCELLED_CALL) { + gpr_mu_unlock(&calld->mu); + grpc_transport_stream_op_finish_with_failure(exec_ctx, op, + GRPC_ERROR_CANCELLED); + GPR_TIMER_END("cc_start_transport_stream_op", 0); + return; + } + if (call != NULL) { + gpr_mu_unlock(&calld->mu); + grpc_subchannel_call_process_op(exec_ctx, call, op); + GPR_TIMER_END("cc_start_transport_stream_op", 0); + return; + } + /* if this is a cancellation, then we can raise our cancelled flag */ + if (op->cancel_error != GRPC_ERROR_NONE) { + if (!gpr_atm_rel_cas(&calld->subchannel_call, 0, + (gpr_atm)(uintptr_t)CANCELLED_CALL)) { + goto retry; + } else { + switch (calld->creation_phase) { + case GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING: + fail_locked(exec_ctx, calld, GRPC_ERROR_REF(op->cancel_error)); + break; + case GRPC_SUBCHANNEL_CALL_HOLDER_PICKING_SUBCHANNEL: + pick_subchannel(exec_ctx, elem, NULL, 0, + &calld->connected_subchannel, NULL); + break; + } + gpr_mu_unlock(&calld->mu); + grpc_transport_stream_op_finish_with_failure(exec_ctx, op, + GRPC_ERROR_CANCELLED); + GPR_TIMER_END("cc_start_transport_stream_op", 0); + return; + } + } + /* if we don't have a subchannel, try to get one */ + if (calld->creation_phase == GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING && + calld->connected_subchannel == NULL && + op->send_initial_metadata != NULL) { + calld->creation_phase = GRPC_SUBCHANNEL_CALL_HOLDER_PICKING_SUBCHANNEL; + grpc_closure_init(&calld->next_step, subchannel_ready, calld); + GRPC_CALL_STACK_REF(calld->owning_call, "pick_subchannel"); + if (pick_subchannel( + exec_ctx, elem, op->send_initial_metadata, + op->send_initial_metadata_flags, &calld->connected_subchannel, + &calld->next_step)) { + calld->creation_phase = GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING; + GRPC_CALL_STACK_UNREF(exec_ctx, calld->owning_call, "pick_subchannel"); + } + } + /* if we've got a subchannel, then let's ask it to create a call */ + if (calld->creation_phase == GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING && + calld->connected_subchannel != NULL) { + grpc_subchannel_call *subchannel_call = NULL; + grpc_error *error = grpc_connected_subchannel_create_call( + exec_ctx, calld->connected_subchannel, calld->pollent, + &subchannel_call); + if (error != GRPC_ERROR_NONE) { + subchannel_call = CANCELLED_CALL; + fail_locked(exec_ctx, calld, GRPC_ERROR_REF(error)); + grpc_transport_stream_op_finish_with_failure(exec_ctx, op, error); + } + gpr_atm_rel_store(&calld->subchannel_call, + (gpr_atm)(uintptr_t)subchannel_call); + retry_waiting_locked(exec_ctx, calld); + goto retry; + } + /* nothing to be done but wait */ + add_waiting_locked(calld, op); + gpr_mu_unlock(&calld->mu); + GPR_TIMER_END("cc_start_transport_stream_op", 0); +} + /* Constructor for call_data */ -static grpc_error *init_call_elem(grpc_exec_ctx *exec_ctx, - grpc_call_element *elem, - grpc_call_element_args *args) { +static grpc_error *cc_init_call_elem(grpc_exec_ctx *exec_ctx, + grpc_call_element *elem, + grpc_call_element_args *args) { call_data *calld = elem->call_data; gpr_atm_rel_store(&calld->subchannel_call, 0); - calld->pick_subchannel = cc_pick_subchannel; - calld->pick_subchannel_arg = elem; gpr_mu_init(&calld->mu); calld->connected_subchannel = NULL; calld->waiting_ops = NULL; @@ -692,9 +719,10 @@ static grpc_error *init_call_elem(grpc_exec_ctx *exec_ctx, } /* Destructor for call_data */ -static void destroy_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, - const grpc_call_final_info *final_info, - void *and_free_memory) { +static void cc_destroy_call_elem(grpc_exec_ctx *exec_ctx, + grpc_call_element *elem, + const grpc_call_final_info *final_info, + void *and_free_memory) { call_data *calld = elem->call_data; grpc_subchannel_call *call = GET_CALL(calld); if (call != NULL && call != CANCELLED_CALL) { @@ -707,47 +735,6 @@ static void destroy_call_elem(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, gpr_free(and_free_memory); } -/* Constructor for channel_data */ -static void init_channel_elem(grpc_exec_ctx *exec_ctx, - grpc_channel_element *elem, - grpc_channel_element_args *args) { - channel_data *chand = elem->channel_data; - - memset(chand, 0, sizeof(*chand)); - - GPR_ASSERT(args->is_last); - GPR_ASSERT(elem->filter == &grpc_client_channel_filter); - - gpr_mu_init(&chand->mu); - grpc_closure_init(&chand->on_resolver_result_changed, - cc_on_resolver_result_changed, chand); - chand->owning_stack = args->channel_stack; - - grpc_connectivity_state_init(&chand->state_tracker, GRPC_CHANNEL_IDLE, - "client_channel"); - chand->interested_parties = grpc_pollset_set_create(); -} - -/* Destructor for channel_data */ -static void destroy_channel_elem(grpc_exec_ctx *exec_ctx, - grpc_channel_element *elem) { - channel_data *chand = elem->channel_data; - - if (chand->resolver != NULL) { - grpc_resolver_shutdown(exec_ctx, chand->resolver); - GRPC_RESOLVER_UNREF(exec_ctx, chand->resolver, "channel"); - } - if (chand->lb_policy != NULL) { - grpc_pollset_set_del_pollset_set(exec_ctx, - chand->lb_policy->interested_parties, - chand->interested_parties); - GRPC_LB_POLICY_UNREF(exec_ctx, chand->lb_policy, "channel"); - } - grpc_connectivity_state_destroy(exec_ctx, &chand->state_tracker); - grpc_pollset_set_destroy(chand->interested_parties); - gpr_mu_destroy(&chand->mu); -} - static void cc_set_pollset_or_pollset_set(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, grpc_polling_entity *pollent) { @@ -755,16 +742,20 @@ static void cc_set_pollset_or_pollset_set(grpc_exec_ctx *exec_ctx, calld->pollent = pollent; } +/************************************************************************* + * EXPORTED SYMBOLS + */ + const grpc_channel_filter grpc_client_channel_filter = { cc_start_transport_stream_op, cc_start_transport_op, sizeof(call_data), - init_call_elem, + cc_init_call_elem, cc_set_pollset_or_pollset_set, - destroy_call_elem, + cc_destroy_call_elem, sizeof(channel_data), - init_channel_elem, - destroy_channel_elem, + cc_init_channel_elem, + cc_destroy_channel_elem, cc_get_peer, "client-channel", }; From d4c0f550b0662d2ec2bc0deb7985aa1357c195a7 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Thu, 1 Sep 2016 09:25:32 -0700 Subject: [PATCH 13/13] clang-format --- src/core/ext/client_config/client_channel.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/core/ext/client_config/client_channel.c b/src/core/ext/client_config/client_channel.c index fff80abb802..61e012578e0 100644 --- a/src/core/ext/client_config/client_channel.c +++ b/src/core/ext/client_config/client_channel.c @@ -652,8 +652,8 @@ retry: fail_locked(exec_ctx, calld, GRPC_ERROR_REF(op->cancel_error)); break; case GRPC_SUBCHANNEL_CALL_HOLDER_PICKING_SUBCHANNEL: - pick_subchannel(exec_ctx, elem, NULL, 0, - &calld->connected_subchannel, NULL); + pick_subchannel(exec_ctx, elem, NULL, 0, &calld->connected_subchannel, + NULL); break; } gpr_mu_unlock(&calld->mu); @@ -670,10 +670,9 @@ retry: calld->creation_phase = GRPC_SUBCHANNEL_CALL_HOLDER_PICKING_SUBCHANNEL; grpc_closure_init(&calld->next_step, subchannel_ready, calld); GRPC_CALL_STACK_REF(calld->owning_call, "pick_subchannel"); - if (pick_subchannel( - exec_ctx, elem, op->send_initial_metadata, - op->send_initial_metadata_flags, &calld->connected_subchannel, - &calld->next_step)) { + if (pick_subchannel(exec_ctx, elem, op->send_initial_metadata, + op->send_initial_metadata_flags, + &calld->connected_subchannel, &calld->next_step)) { calld->creation_phase = GRPC_SUBCHANNEL_CALL_HOLDER_NOT_CREATING; GRPC_CALL_STACK_UNREF(exec_ctx, calld->owning_call, "pick_subchannel"); }