From 2457a04bff9180e60570d80152af931281903826 Mon Sep 17 00:00:00 2001 From: Matt Kwong Date: Mon, 14 Aug 2017 14:59:36 -0700 Subject: [PATCH 01/21] Add Performance profiling job to Kokoro --- .../prepare_build_linux_perf_rc | 22 +++++++++++ .../linux/grpc_performance_profile_daily.cfg | 26 +++++++++++++ .../linux/grpc_performance_profile_daily.sh | 37 +++++++++++++++++++ .../linux/grpc_performance_profile_master.cfg | 26 +++++++++++++ .../linux/grpc_performance_profile_master.sh | 32 ++++++++++++++++ 5 files changed, 143 insertions(+) create mode 100644 tools/internal_ci/helper_scripts/prepare_build_linux_perf_rc create mode 100644 tools/internal_ci/linux/grpc_performance_profile_daily.cfg create mode 100755 tools/internal_ci/linux/grpc_performance_profile_daily.sh create mode 100644 tools/internal_ci/linux/grpc_performance_profile_master.cfg create mode 100755 tools/internal_ci/linux/grpc_performance_profile_master.sh diff --git a/tools/internal_ci/helper_scripts/prepare_build_linux_perf_rc b/tools/internal_ci/helper_scripts/prepare_build_linux_perf_rc new file mode 100644 index 00000000000..1b7779caa82 --- /dev/null +++ b/tools/internal_ci/helper_scripts/prepare_build_linux_perf_rc @@ -0,0 +1,22 @@ +#!/bin/bash +# Copyright 2017 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Source this rc script to prepare the environment for linux perf builds + +# Need to increase open files limit and size for perf test +ulimit -n 32768 +ulimit -c unlimited + +git submodule update --init diff --git a/tools/internal_ci/linux/grpc_performance_profile_daily.cfg b/tools/internal_ci/linux/grpc_performance_profile_daily.cfg new file mode 100644 index 00000000000..9831869edb2 --- /dev/null +++ b/tools/internal_ci/linux/grpc_performance_profile_daily.cfg @@ -0,0 +1,26 @@ +# Copyright 2017 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Config file for the internal CI (in protobuf text format) + +# Location of the continuous shell script in repository. +build_file: "grpc/tools/internal_ci/linux/grpc_performance_profile_daily.sh" +timeout_mins: 1440 +action { + define_artifacts { + regex: "**" + regex: "github/grpc/reports/**" + } +} + diff --git a/tools/internal_ci/linux/grpc_performance_profile_daily.sh b/tools/internal_ci/linux/grpc_performance_profile_daily.sh new file mode 100755 index 00000000000..25523e21b80 --- /dev/null +++ b/tools/internal_ci/linux/grpc_performance_profile_daily.sh @@ -0,0 +1,37 @@ +#!/usr/bin/env bash +# Copyright 2017 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +set -ex + +# Enter the gRPC repo root +cd $(dirname $0)/../../.. + +source tools/internal_ci/helper_scripts/prepare_build_linux_perf_rc + +CPUS=`python -c 'import multiprocessing; print multiprocessing.cpu_count()'` + +make CONFIG=opt memory_profile_test memory_profile_client memory_profile_server -j $CPUS +bins/opt/memory_profile_test +bq load microbenchmarks.memory memory_usage.csv + +tools/run_tests/run_microbenchmark.py --collect summary --bigquery_upload || FAILED="true" + +# kill port_server.py to prevent the build from hanging +ps aux | grep port_server\\.py | awk '{print $2}' | xargs kill -9 + +if [ "$FAILED" != "" ] +then + exit 1 +fi diff --git a/tools/internal_ci/linux/grpc_performance_profile_master.cfg b/tools/internal_ci/linux/grpc_performance_profile_master.cfg new file mode 100644 index 00000000000..e4cefbc3f4e --- /dev/null +++ b/tools/internal_ci/linux/grpc_performance_profile_master.cfg @@ -0,0 +1,26 @@ +# Copyright 2017 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Config file for the internal CI (in protobuf text format) + +# Location of the continuous shell script in repository. +build_file: "grpc/tools/internal_ci/linux/grpc_performance_profile_master.sh" +timeout_mins: 600 +action { + define_artifacts { + regex: "**" + regex: "github/grpc/reports/**" + } +} + diff --git a/tools/internal_ci/linux/grpc_performance_profile_master.sh b/tools/internal_ci/linux/grpc_performance_profile_master.sh new file mode 100755 index 00000000000..40bbfe89dc2 --- /dev/null +++ b/tools/internal_ci/linux/grpc_performance_profile_master.sh @@ -0,0 +1,32 @@ +#!/usr/bin/env bash +# Copyright 2017 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +set -ex + +# Enter the gRPC repo root +cd $(dirname $0)/../../.. + +source tools/internal_ci/helper_scripts/prepare_build_linux_perf_rc + +tools/jenkins/run_performance_profile_hourly.sh || FAILED="true" + +# kill port_server.py to prevent the build from hanging +ps aux | grep port_server\\.py | awk '{print $2}' | xargs kill -9 + +if [ "$FAILED" != "" ] +then + exit 1 +fi + From 5920abc5358e854a0f4a2217bf8be230977d1853 Mon Sep 17 00:00:00 2001 From: Matt Kwong Date: Thu, 24 Aug 2017 12:26:05 -0700 Subject: [PATCH 02/21] Revert "Don't clear alarm in jobset when running performance tests" This reverts commit c15d32bbe89a2bf950992ded06d1b3da7f1f39a6. --- tools/run_tests/python_utils/jobset.py | 13 ++++--------- tools/run_tests/run_performance_tests.py | 10 +++++----- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/tools/run_tests/python_utils/jobset.py b/tools/run_tests/python_utils/jobset.py index 08d652ae3f3..044c6f3aa4f 100755 --- a/tools/run_tests/python_utils/jobset.py +++ b/tools/run_tests/python_utils/jobset.py @@ -367,10 +367,9 @@ class Jobset(object): """Manages one run of jobs.""" def __init__(self, check_cancelled, maxjobs, newline_on_success, travis, - stop_on_failure, add_env, quiet_success, max_time, clear_alarms): + stop_on_failure, add_env, quiet_success, max_time): self._running = set() self._check_cancelled = check_cancelled - self._clear_alarms = clear_alarms self._cancelled = False self._failures = 0 self._completed = 0 @@ -474,10 +473,7 @@ class Jobset(object): while self._running: if self.cancelled(): pass # poll cancellation self.reap() - # Clear the alarms when finished to avoid a race condition causing job - # failures. Don't do this when running multi-VM tests because clearing - # the alarms causes the test to stall - if platform_string() != 'windows' and self._clear_alarms: + if platform_string() != 'windows': signal.alarm(0) return not self.cancelled() and self._failures == 0 @@ -507,8 +503,7 @@ def run(cmdlines, add_env={}, skip_jobs=False, quiet_success=False, - max_time=-1, - clear_alarms=True): + max_time=-1): if skip_jobs: resultset = {} skipped_job_result = JobResult() @@ -520,7 +515,7 @@ def run(cmdlines, js = Jobset(check_cancelled, maxjobs if maxjobs is not None else _DEFAULT_MAX_JOBS, newline_on_success, travis, stop_on_failure, add_env, - quiet_success, max_time, clear_alarms) + quiet_success, max_time) for cmdline, remaining in tag_remaining(cmdlines): if not js.start(cmdline): break diff --git a/tools/run_tests/run_performance_tests.py b/tools/run_tests/run_performance_tests.py index 0db5e4ef830..b9277c919b5 100755 --- a/tools/run_tests/run_performance_tests.py +++ b/tools/run_tests/run_performance_tests.py @@ -183,7 +183,7 @@ def archive_repo(languages): jobset.message('START', 'Archiving local repository.', do_newline=True) num_failures, _ = jobset.run( - [archive_job], newline_on_success=True, maxjobs=1, clear_alarms=False) + [archive_job], newline_on_success=True, maxjobs=1) if num_failures == 0: jobset.message('SUCCESS', 'Archive with local repository created successfully.', @@ -215,7 +215,7 @@ def prepare_remote_hosts(hosts, prepare_local=False): timeout_seconds=prepare_timeout)) jobset.message('START', 'Preparing hosts.', do_newline=True) num_failures, _ = jobset.run( - prepare_jobs, newline_on_success=True, maxjobs=10, clear_alarms=False) + prepare_jobs, newline_on_success=True, maxjobs=10) if num_failures == 0: jobset.message('SUCCESS', 'Prepare step completed successfully.', @@ -248,7 +248,7 @@ def build_on_remote_hosts(hosts, languages=scenario_config.LANGUAGES.keys(), bui timeout_seconds=build_timeout)) jobset.message('START', 'Building.', do_newline=True) num_failures, _ = jobset.run( - build_jobs, newline_on_success=True, maxjobs=10, clear_alarms=False) + build_jobs, newline_on_success=True, maxjobs=10) if num_failures == 0: jobset.message('SUCCESS', 'Built successfully.', @@ -414,7 +414,7 @@ def run_collect_perf_profile_jobs(hosts_and_base_names, scenario_name): perf_report_jobs.append(perf_report_processor_job(host, perf_base_name, output_filename)) jobset.message('START', 'Collecting perf reports from qps workers', do_newline=True) - failures, _ = jobset.run(perf_report_jobs, newline_on_success=True, maxjobs=1, clear_alarms=False) + failures, _ = jobset.run(perf_report_jobs, newline_on_success=True, maxjobs=1) jobset.message('END', 'Collecting perf reports from qps workers', do_newline=True) return failures @@ -556,7 +556,7 @@ for scenario in scenarios: jobs = [scenario.jobspec] if scenario.workers: jobs.append(create_quit_jobspec(scenario.workers, remote_host=args.remote_driver_host)) - scenario_failures, resultset = jobset.run(jobs, newline_on_success=True, maxjobs=1, clear_alarms=False) + scenario_failures, resultset = jobset.run(jobs, newline_on_success=True, maxjobs=1) total_scenario_failures += scenario_failures merged_resultset = dict(itertools.chain(six.iteritems(merged_resultset), six.iteritems(resultset))) From 3da8c5defbc9a83d3db0bf84fb0dce012802d9b3 Mon Sep 17 00:00:00 2001 From: Matt Kwong Date: Thu, 24 Aug 2017 12:35:04 -0700 Subject: [PATCH 03/21] Let alarms at end of jobset.py trigger isntead of clearing --- tools/run_tests/python_utils/jobset.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/run_tests/python_utils/jobset.py b/tools/run_tests/python_utils/jobset.py index 044c6f3aa4f..50fe7d7df86 100755 --- a/tools/run_tests/python_utils/jobset.py +++ b/tools/run_tests/python_utils/jobset.py @@ -473,8 +473,10 @@ class Jobset(object): while self._running: if self.cancelled(): pass # poll cancellation self.reap() - if platform_string() != 'windows': - signal.alarm(0) + global have_alarm + if platform_string() != 'windows' and have_alarm: + signal.alarm(1) + signal.pause() return not self.cancelled() and self._failures == 0 From 738b1bb424292b4138e82a3c2085ab4a3a95507e Mon Sep 17 00:00:00 2001 From: Matt Kwong Date: Tue, 29 Aug 2017 12:45:53 -0700 Subject: [PATCH 04/21] Get rid of have_alarm var in jobset.py --- tools/run_tests/python_utils/jobset.py | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/tools/run_tests/python_utils/jobset.py b/tools/run_tests/python_utils/jobset.py index 50fe7d7df86..6151a7276a9 100755 --- a/tools/run_tests/python_utils/jobset.py +++ b/tools/run_tests/python_utils/jobset.py @@ -71,10 +71,8 @@ def platform_string(): if platform_string() == 'windows': pass else: - have_alarm = False def alarm_handler(unused_signum, unused_frame): - global have_alarm - have_alarm = False + pass signal.signal(signal.SIGCHLD, lambda unused_signum, unused_frame: None) signal.signal(signal.SIGALRM, alarm_handler) @@ -454,10 +452,7 @@ class Jobset(object): if platform_string() == 'windows': time.sleep(0.1) else: - global have_alarm - if not have_alarm: - have_alarm = True - signal.alarm(10) + signal.alarm(10) signal.pause() def cancelled(self): @@ -473,10 +468,8 @@ class Jobset(object): while self._running: if self.cancelled(): pass # poll cancellation self.reap() - global have_alarm - if platform_string() != 'windows' and have_alarm: - signal.alarm(1) - signal.pause() + if platform_string() != 'windows': + signal.alarm(0) return not self.cancelled() and self._failures == 0 From 561dc32247534d9204d9f68e92259759875c6d93 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 5 Sep 2017 13:00:46 -0700 Subject: [PATCH 05/21] Add stats for server request serving path --- src/core/lib/debug/stats_data.c | 48 +++++++++++++++++++++++++----- src/core/lib/debug/stats_data.h | 23 ++++++++++---- src/core/lib/debug/stats_data.yaml | 6 ++++ src/core/lib/surface/server.c | 5 ++++ 4 files changed, 69 insertions(+), 13 deletions(-) diff --git a/src/core/lib/debug/stats_data.c b/src/core/lib/debug/stats_data.c index 57cbafc8176..73703208da1 100644 --- a/src/core/lib/debug/stats_data.c +++ b/src/core/lib/debug/stats_data.c @@ -48,10 +48,13 @@ const char *grpc_stats_counter_name[GRPC_STATS_COUNTER_COUNT] = { "executor_scheduled_to_self", "executor_wakeup_initiated", "executor_queue_drained", + "server_requested_calls", + "server_slowpath_requests_queued", }; const char *grpc_stats_histogram_name[GRPC_STATS_HISTOGRAM_COUNT] = { - "tcp_write_size", "tcp_write_iov_size", "tcp_read_size", - "tcp_read_offer", "tcp_read_iov_size", "http2_send_message_size", + "tcp_write_size", "tcp_write_iov_size", "tcp_read_size", + "tcp_read_offer", "tcp_read_iov_size", "http2_send_message_size", + "server_cqs_checked", }; const int grpc_stats_table_0[64] = { 0, 1, 2, 3, 4, 6, 8, 11, @@ -81,6 +84,8 @@ const uint8_t grpc_stats_table_3[104] = { 24, 24, 25, 25, 26, 27, 28, 28, 29, 29, 30, 30, 31, 31, 32, 32, 33, 33, 34, 34, 35, 36, 36, 37, 38, 38, 39, 39, 40, 40, 41, 41, 42, 42, 42, 43, 44, 45, 45, 46, 47, 47, 48, 48, 49, 49, 50, 50, 51, 51}; +const int grpc_stats_table_4[8] = {0, 1, 2, 4, 8, 16, 32, 64}; +const uint8_t grpc_stats_table_5[4] = {0, 1, 2, 3}; void grpc_stats_inc_tcp_write_size(grpc_exec_ctx *exec_ctx, int value) { value = GPR_CLAMP(value, 0, 16777216); if (value < 5) { @@ -233,12 +238,39 @@ void grpc_stats_inc_http2_send_message_size(grpc_exec_ctx *exec_ctx, grpc_stats_histo_find_bucket_slow( (exec_ctx), value, grpc_stats_table_0, 64)); } -const int grpc_stats_histo_buckets[6] = {64, 64, 64, 64, 64, 64}; -const int grpc_stats_histo_start[6] = {0, 64, 128, 192, 256, 320}; -const int *const grpc_stats_histo_bucket_boundaries[6] = { +void grpc_stats_inc_server_cqs_checked(grpc_exec_ctx *exec_ctx, int value) { + value = GPR_CLAMP(value, 0, 64); + if (value < 3) { + GRPC_STATS_INC_HISTOGRAM((exec_ctx), + GRPC_STATS_HISTOGRAM_SERVER_CQS_CHECKED, value); + return; + } + union { + double dbl; + uint64_t uint; + } _val, _bkt; + _val.dbl = value; + if (_val.uint < 4627448617123184640ull) { + int bucket = + grpc_stats_table_5[((_val.uint - 4613937818241073152ull) >> 52)] + 3; + _bkt.dbl = grpc_stats_table_4[bucket]; + bucket -= (_val.uint < _bkt.uint); + GRPC_STATS_INC_HISTOGRAM((exec_ctx), + GRPC_STATS_HISTOGRAM_SERVER_CQS_CHECKED, bucket); + return; + } + GRPC_STATS_INC_HISTOGRAM((exec_ctx), GRPC_STATS_HISTOGRAM_SERVER_CQS_CHECKED, + grpc_stats_histo_find_bucket_slow( + (exec_ctx), value, grpc_stats_table_4, 8)); +} +const int grpc_stats_histo_buckets[7] = {64, 64, 64, 64, 64, 64, 8}; +const int grpc_stats_histo_start[7] = {0, 64, 128, 192, 256, 320, 384}; +const int *const grpc_stats_histo_bucket_boundaries[7] = { + grpc_stats_table_0, grpc_stats_table_2, grpc_stats_table_0, grpc_stats_table_0, grpc_stats_table_2, grpc_stats_table_0, - grpc_stats_table_0, grpc_stats_table_2, grpc_stats_table_0}; -void (*const grpc_stats_inc_histogram[6])(grpc_exec_ctx *exec_ctx, int x) = { + grpc_stats_table_4}; +void (*const grpc_stats_inc_histogram[7])(grpc_exec_ctx *exec_ctx, int x) = { grpc_stats_inc_tcp_write_size, grpc_stats_inc_tcp_write_iov_size, grpc_stats_inc_tcp_read_size, grpc_stats_inc_tcp_read_offer, - grpc_stats_inc_tcp_read_iov_size, grpc_stats_inc_http2_send_message_size}; + grpc_stats_inc_tcp_read_iov_size, grpc_stats_inc_http2_send_message_size, + grpc_stats_inc_server_cqs_checked}; diff --git a/src/core/lib/debug/stats_data.h b/src/core/lib/debug/stats_data.h index 9b2d43a03c9..f5dd4868738 100644 --- a/src/core/lib/debug/stats_data.h +++ b/src/core/lib/debug/stats_data.h @@ -50,6 +50,8 @@ typedef enum { GRPC_STATS_COUNTER_EXECUTOR_SCHEDULED_TO_SELF, GRPC_STATS_COUNTER_EXECUTOR_WAKEUP_INITIATED, GRPC_STATS_COUNTER_EXECUTOR_QUEUE_DRAINED, + GRPC_STATS_COUNTER_SERVER_REQUESTED_CALLS, + GRPC_STATS_COUNTER_SERVER_SLOWPATH_REQUESTS_QUEUED, GRPC_STATS_COUNTER_COUNT } grpc_stats_counters; extern const char *grpc_stats_counter_name[GRPC_STATS_COUNTER_COUNT]; @@ -60,6 +62,7 @@ typedef enum { GRPC_STATS_HISTOGRAM_TCP_READ_OFFER, GRPC_STATS_HISTOGRAM_TCP_READ_IOV_SIZE, GRPC_STATS_HISTOGRAM_HTTP2_SEND_MESSAGE_SIZE, + GRPC_STATS_HISTOGRAM_SERVER_CQS_CHECKED, GRPC_STATS_HISTOGRAM_COUNT } grpc_stats_histograms; extern const char *grpc_stats_histogram_name[GRPC_STATS_HISTOGRAM_COUNT]; @@ -76,7 +79,9 @@ typedef enum { GRPC_STATS_HISTOGRAM_TCP_READ_IOV_SIZE_BUCKETS = 64, GRPC_STATS_HISTOGRAM_HTTP2_SEND_MESSAGE_SIZE_FIRST_SLOT = 320, GRPC_STATS_HISTOGRAM_HTTP2_SEND_MESSAGE_SIZE_BUCKETS = 64, - GRPC_STATS_HISTOGRAM_BUCKETS = 384 + GRPC_STATS_HISTOGRAM_SERVER_CQS_CHECKED_FIRST_SLOT = 384, + GRPC_STATS_HISTOGRAM_SERVER_CQS_CHECKED_BUCKETS = 8, + GRPC_STATS_HISTOGRAM_BUCKETS = 392 } grpc_stats_histogram_constants; #define GRPC_STATS_INC_CLIENT_CALLS_CREATED(exec_ctx) \ GRPC_STATS_INC_COUNTER((exec_ctx), GRPC_STATS_COUNTER_CLIENT_CALLS_CREATED) @@ -139,6 +144,11 @@ typedef enum { GRPC_STATS_COUNTER_EXECUTOR_WAKEUP_INITIATED) #define GRPC_STATS_INC_EXECUTOR_QUEUE_DRAINED(exec_ctx) \ GRPC_STATS_INC_COUNTER((exec_ctx), GRPC_STATS_COUNTER_EXECUTOR_QUEUE_DRAINED) +#define GRPC_STATS_INC_SERVER_REQUESTED_CALLS(exec_ctx) \ + GRPC_STATS_INC_COUNTER((exec_ctx), GRPC_STATS_COUNTER_SERVER_REQUESTED_CALLS) +#define GRPC_STATS_INC_SERVER_SLOWPATH_REQUESTS_QUEUED(exec_ctx) \ + GRPC_STATS_INC_COUNTER((exec_ctx), \ + GRPC_STATS_COUNTER_SERVER_SLOWPATH_REQUESTS_QUEUED) #define GRPC_STATS_INC_TCP_WRITE_SIZE(exec_ctx, value) \ grpc_stats_inc_tcp_write_size((exec_ctx), (int)(value)) void grpc_stats_inc_tcp_write_size(grpc_exec_ctx *exec_ctx, int x); @@ -157,10 +167,13 @@ void grpc_stats_inc_tcp_read_iov_size(grpc_exec_ctx *exec_ctx, int x); #define GRPC_STATS_INC_HTTP2_SEND_MESSAGE_SIZE(exec_ctx, value) \ grpc_stats_inc_http2_send_message_size((exec_ctx), (int)(value)) void grpc_stats_inc_http2_send_message_size(grpc_exec_ctx *exec_ctx, int x); -extern const int grpc_stats_histo_buckets[6]; -extern const int grpc_stats_histo_start[6]; -extern const int *const grpc_stats_histo_bucket_boundaries[6]; -extern void (*const grpc_stats_inc_histogram[6])(grpc_exec_ctx *exec_ctx, +#define GRPC_STATS_INC_SERVER_CQS_CHECKED(exec_ctx, value) \ + grpc_stats_inc_server_cqs_checked((exec_ctx), (int)(value)) +void grpc_stats_inc_server_cqs_checked(grpc_exec_ctx *exec_ctx, int x); +extern const int grpc_stats_histo_buckets[7]; +extern const int grpc_stats_histo_start[7]; +extern const int *const grpc_stats_histo_bucket_boundaries[7]; +extern void (*const grpc_stats_inc_histogram[7])(grpc_exec_ctx *exec_ctx, int x); #endif /* GRPC_CORE_LIB_DEBUG_STATS_DATA_H */ diff --git a/src/core/lib/debug/stats_data.yaml b/src/core/lib/debug/stats_data.yaml index a0d042a688b..71dc32a8ea5 100644 --- a/src/core/lib/debug/stats_data.yaml +++ b/src/core/lib/debug/stats_data.yaml @@ -65,3 +65,9 @@ - counter: executor_scheduled_to_self - counter: executor_wakeup_initiated - counter: executor_queue_drained +# server +- counter: server_requested_calls +- histogram: server_cqs_checked + buckets: 8 + max: 64 +- counter: server_slowpath_requests_queued diff --git a/src/core/lib/surface/server.c b/src/core/lib/surface/server.c index 66dcc299aab..786d754b496 100644 --- a/src/core/lib/surface/server.c +++ b/src/core/lib/surface/server.c @@ -29,6 +29,7 @@ #include "src/core/lib/channel/channel_args.h" #include "src/core/lib/channel/connected_channel.h" +#include "src/core/lib/debug/stats.h" #include "src/core/lib/iomgr/executor.h" #include "src/core/lib/iomgr/iomgr.h" #include "src/core/lib/slice/slice_internal.h" @@ -538,6 +539,7 @@ static void publish_new_rpc(grpc_exec_ctx *exec_ctx, void *arg, if (request_id == -1) { continue; } else { + GRPC_STATS_INC_SERVER_CQS_CHECKED(exec_ctx, i); gpr_mu_lock(&calld->mu_state); calld->state = ACTIVATED; gpr_mu_unlock(&calld->mu_state); @@ -548,6 +550,7 @@ static void publish_new_rpc(grpc_exec_ctx *exec_ctx, void *arg, } /* no cq to take the request found: queue it on the slow list */ + GRPC_STATS_INC_SERVER_SLOWPATH_REQUESTS_QUEUED(exec_ctx); gpr_mu_lock(&server->mu_call); gpr_mu_lock(&calld->mu_state); calld->state = PENDING; @@ -1429,6 +1432,7 @@ grpc_call_error grpc_server_request_call( grpc_call_error error; grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; requested_call *rc = gpr_malloc(sizeof(*rc)); + GRPC_STATS_INC_SERVER_REQUESTED_CALLS(&exec_ctx); GRPC_API_TRACE( "grpc_server_request_call(" "server=%p, call=%p, details=%p, initial_metadata=%p, " @@ -1475,6 +1479,7 @@ grpc_call_error grpc_server_request_registered_call( grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT; requested_call *rc = gpr_malloc(sizeof(*rc)); registered_method *rm = rmp; + GRPC_STATS_INC_SERVER_REQUESTED_CALLS(&exec_ctx); GRPC_API_TRACE( "grpc_server_request_registered_call(" "server=%p, rmp=%p, call=%p, deadline=%p, initial_metadata=%p, " From 4657a4818b56e11d6b11654933dcd80a434f9f93 Mon Sep 17 00:00:00 2001 From: Ken Payson Date: Fri, 8 Sep 2017 14:49:38 -0700 Subject: [PATCH 06/21] Warm pip cache with libraries used by virtualenv --- tools/internal_ci/helper_scripts/prepare_build_linux_rc | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/internal_ci/helper_scripts/prepare_build_linux_rc b/tools/internal_ci/helper_scripts/prepare_build_linux_rc index 91627d60cb5..2ade8dac51f 100644 --- a/tools/internal_ci/helper_scripts/prepare_build_linux_rc +++ b/tools/internal_ci/helper_scripts/prepare_build_linux_rc @@ -26,6 +26,7 @@ sudo service docker restart # Populate xdg-cache-home to workaround https://github.com/grpc/grpc/issues/11968 sudo mkdir -p /tmp/xdg-cache-home +PYTHONWARNINGS=ignore XDG_CACHE_HOME=/tmp/xdg-cache-home sudo -E pip install setuptools wheel PYTHONWARNINGS=ignore XDG_CACHE_HOME=/tmp/xdg-cache-home sudo -E pip install coverage==4.4 pylint==1.6.5 # Download Docker images from DockerHub From 5a2a792509af457322b8fc63eff3ec1ee360b0a0 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Mon, 24 Apr 2017 13:31:20 +0200 Subject: [PATCH 07/21] add cmake distribtest --- test/distrib/cpp/run_distrib_test_cmake.sh | 66 +++++++++++++++++++ ...test.sh => run_distrib_test_routeguide.sh} | 0 .../distribtest/cpp_jessie_x64/Dockerfile | 2 + .../artifacts/distribtest_targets.py | 12 ++-- 4 files changed, 75 insertions(+), 5 deletions(-) create mode 100755 test/distrib/cpp/run_distrib_test_cmake.sh rename test/distrib/cpp/{run_distrib_test.sh => run_distrib_test_routeguide.sh} (100%) diff --git a/test/distrib/cpp/run_distrib_test_cmake.sh b/test/distrib/cpp/run_distrib_test_cmake.sh new file mode 100755 index 00000000000..a81de57810b --- /dev/null +++ b/test/distrib/cpp/run_distrib_test_cmake.sh @@ -0,0 +1,66 @@ +#!/bin/bash +# Copyright 2017 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +set -ex + +cd $(dirname $0)/../../.. + +echo "deb http://ftp.debian.org/debian jessie-backports main" | tee /etc/apt/sources.list.d/jessie-backports.list +apt-get update +#apt-get install -t jessie-backports -y libc-ares-dev # we need specifically version 1.12 +apt-get install -t jessie-backports -y libssl-dev + +# Install c-ares +cd third_party/cares/cares +git fetch origin +git checkout cares-1_13_0 +mkdir -p cmake/build +cd cmake/build +cmake -DCMAKE_BUILD_TYPE=Release ../.. +make -j4 install +cd ../../../../.. + +# Install zlib +cd third_party/zlib +mkdir -p cmake/build +cd cmake/build +cmake -DCMAKE_BUILD_TYPE=Release ../.. +make -j4 install +cd ../../../.. + +# Install protobuf +cd third_party/protobuf +mkdir -p cmake/build +cd cmake/build +cmake -Dprotobuf_BUILD_TESTS=OFF -DCMAKE_BUILD_TYPE=Release .. +make -j4 install +cd ../../../.. + +# TODO: Install boringssl + +# Install gRPC +mkdir -p cmake/build +cd cmake/build +cmake -DgRPC_INSTALL=ON -DgRPC_BUILD_TESTS=OFF -DgRPC_PROTOBUF_PROVIDER=package -DgRPC_ZLIB_PROVIDER=package -DgRPC_CARES_PROVIDER=package -DgRPC_SSL_PROVIDER=package -DCMAKE_BUILD_TYPE=Release ../.. +make -j4 install +cd ../.. + +# Build helloworld example using cmake +cd examples/cpp/helloworld +mkdir -p cmake/build +cd cmake/build +cmake ../.. +make + diff --git a/test/distrib/cpp/run_distrib_test.sh b/test/distrib/cpp/run_distrib_test_routeguide.sh similarity index 100% rename from test/distrib/cpp/run_distrib_test.sh rename to test/distrib/cpp/run_distrib_test_routeguide.sh diff --git a/tools/dockerfile/distribtest/cpp_jessie_x64/Dockerfile b/tools/dockerfile/distribtest/cpp_jessie_x64/Dockerfile index ff66bca0aec..0e8186d40ce 100644 --- a/tools/dockerfile/distribtest/cpp_jessie_x64/Dockerfile +++ b/tools/dockerfile/distribtest/cpp_jessie_x64/Dockerfile @@ -27,4 +27,6 @@ RUN apt-get update && apt-get install -y \ pkg-config \ unzip && apt-get clean +RUN apt-get update && apt-get install -y cmake golang && apt-get clean + CMD ["bash"] diff --git a/tools/run_tests/artifacts/distribtest_targets.py b/tools/run_tests/artifacts/distribtest_targets.py index fa461efa85e..fb1be383cd9 100644 --- a/tools/run_tests/artifacts/distribtest_targets.py +++ b/tools/run_tests/artifacts/distribtest_targets.py @@ -255,12 +255,13 @@ class PHPDistribTest(object): class CppDistribTest(object): """Tests Cpp make intall by building examples.""" - def __init__(self, platform, arch, docker_suffix=None): - self.name = 'cpp_%s_%s_%s' % (platform, arch, docker_suffix) + def __init__(self, platform, arch, docker_suffix=None, testcase=None): + self.name = 'cpp_%s_%s_%s_%s' % (platform, arch, docker_suffix, testcase) self.platform = platform self.arch = arch self.docker_suffix = docker_suffix - self.labels = ['distribtest', 'cpp', platform, arch, docker_suffix] + self.testcase = testcase + self.labels = ['distribtest', 'cpp', platform, arch, docker_suffix, testcase] def pre_build_jobspecs(self): return [] @@ -271,7 +272,7 @@ class CppDistribTest(object): 'tools/dockerfile/distribtest/cpp_%s_%s' % ( self.docker_suffix, self.arch), - 'test/distrib/cpp/run_distrib_test.sh') + 'test/distrib/cpp/run_distrib_test_%s.sh' % self.testcase) else: raise Exception("Not supported yet.") @@ -281,7 +282,8 @@ class CppDistribTest(object): def targets(): """Gets list of supported targets""" - return [CppDistribTest('linux', 'x64', 'jessie'), + return [CppDistribTest('linux', 'x64', 'jessie', 'routeguide'), + CppDistribTest('linux', 'x64', 'jessie', 'cmake'), CSharpDistribTest('linux', 'x64', 'wheezy'), CSharpDistribTest('linux', 'x64', 'jessie'), CSharpDistribTest('linux', 'x86', 'jessie'), From 4caadb9205bcb9f745cda2e3c310b27837cfd636 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 25 Aug 2017 12:59:11 +0200 Subject: [PATCH 08/21] fix helloworld cmake build --- examples/cpp/helloworld/CMakeLists.txt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/examples/cpp/helloworld/CMakeLists.txt b/examples/cpp/helloworld/CMakeLists.txt index 8f098c91a6b..71a8db4f24f 100644 --- a/examples/cpp/helloworld/CMakeLists.txt +++ b/examples/cpp/helloworld/CMakeLists.txt @@ -2,7 +2,11 @@ cmake_minimum_required(VERSION 2.8) # Project -project(HelloWorld CXX) +project(HelloWorld C CXX) + +if(NOT MSVC) + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11") +endif() # Protobuf set(protobuf_MODULE_COMPATIBLE TRUE) From f567ab0d2b3b761a3647126c4c5a2ec2549f4c73 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 25 Aug 2017 16:13:30 +0200 Subject: [PATCH 09/21] fix public header path for installed grpc --- CMakeLists.txt | 70 +++++++++++++++---------------- templates/CMakeLists.txt.template | 2 +- 2 files changed, 36 insertions(+), 36 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5131bf39b47..a82e9ef6d03 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -832,7 +832,7 @@ endif() target_include_directories(gpr - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -924,7 +924,7 @@ endif() target_include_directories(gpr_test_util - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -1219,7 +1219,7 @@ endif() target_include_directories(grpc - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -1525,7 +1525,7 @@ endif() target_include_directories(grpc_cronet - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -1803,7 +1803,7 @@ endif() target_include_directories(grpc_test_util - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -2063,7 +2063,7 @@ endif() target_include_directories(grpc_test_util_unsecure - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -2357,7 +2357,7 @@ endif() target_include_directories(grpc_unsecure - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -2446,7 +2446,7 @@ endif() target_include_directories(reconnect_server - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -2488,7 +2488,7 @@ endif() target_include_directories(test_tcp_server - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -2569,7 +2569,7 @@ endif() target_include_directories(grpc++ - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -2769,7 +2769,7 @@ protobuf_generate_grpc_cpp( ) target_include_directories(grpc++_core_stats - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -3060,7 +3060,7 @@ endif() target_include_directories(grpc++_cronet - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -3259,7 +3259,7 @@ protobuf_generate_grpc_cpp( ) target_include_directories(grpc++_error_details - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -3324,7 +3324,7 @@ protobuf_generate_grpc_cpp( ) target_include_directories(grpc++_proto_reflection_desc_db - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -3385,7 +3385,7 @@ protobuf_generate_grpc_cpp( ) target_include_directories(grpc++_reflection - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -3443,7 +3443,7 @@ endif() target_include_directories(grpc++_test_config - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -3521,7 +3521,7 @@ protobuf_generate_grpc_cpp( ) target_include_directories(grpc++_test_util - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -3659,7 +3659,7 @@ protobuf_generate_grpc_cpp( ) target_include_directories(grpc++_test_util_unsecure - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -3799,7 +3799,7 @@ endif() target_include_directories(grpc++_unsecure - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -3989,7 +3989,7 @@ endif() target_include_directories(grpc_benchmark - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -4048,7 +4048,7 @@ protobuf_generate_grpc_cpp( ) target_include_directories(grpc_cli_libs - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -4108,7 +4108,7 @@ endif() target_include_directories(grpc_plugin_support - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -4186,7 +4186,7 @@ protobuf_generate_grpc_cpp( ) target_include_directories(http2_client_main - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -4241,7 +4241,7 @@ protobuf_generate_grpc_cpp( ) target_include_directories(interop_client_helper - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -4311,7 +4311,7 @@ protobuf_generate_grpc_cpp( ) target_include_directories(interop_client_main - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -4362,7 +4362,7 @@ endif() target_include_directories(interop_server_helper - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -4431,7 +4431,7 @@ protobuf_generate_grpc_cpp( ) target_include_directories(interop_server_lib - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -4482,7 +4482,7 @@ endif() target_include_directories(interop_server_main - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -4570,7 +4570,7 @@ protobuf_generate_grpc_cpp( ) target_include_directories(qps - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -4617,7 +4617,7 @@ endif() target_include_directories(grpc_csharp_ext - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -4712,7 +4712,7 @@ endif() target_include_directories(ares - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -4750,7 +4750,7 @@ endif() target_include_directories(bad_client_test - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -4791,7 +4791,7 @@ endif() target_include_directories(bad_ssl_test_server - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -4892,7 +4892,7 @@ endif() target_include_directories(end2end_tests - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src @@ -4993,7 +4993,7 @@ endif() target_include_directories(end2end_nosec_tests - PUBLIC $ $ + PUBLIC $ $ PRIVATE ${CMAKE_CURRENT_SOURCE_DIR} PRIVATE ${BORINGSSL_ROOT_DIR}/include PRIVATE ${PROTOBUF_ROOT_DIR}/src diff --git a/templates/CMakeLists.txt.template b/templates/CMakeLists.txt.template index 6adff889fef..ad79873d562 100644 --- a/templates/CMakeLists.txt.template +++ b/templates/CMakeLists.txt.template @@ -515,7 +515,7 @@ % endfor target_include_directories(${lib.name} - PUBLIC <%text>$ $ + PUBLIC <%text>$ $ PRIVATE <%text>${CMAKE_CURRENT_SOURCE_DIR} PRIVATE <%text>${BORINGSSL_ROOT_DIR}/include PRIVATE <%text>${PROTOBUF_ROOT_DIR}/src From b2cf73e1f81a9c9eeff6dab8f8980c1711784796 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Mon, 28 Aug 2017 12:18:45 +0200 Subject: [PATCH 10/21] prevent submodule headers from influencing the build --- test/distrib/cpp/run_distrib_test_cmake.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/distrib/cpp/run_distrib_test_cmake.sh b/test/distrib/cpp/run_distrib_test_cmake.sh index a81de57810b..ead8cc10bc7 100755 --- a/test/distrib/cpp/run_distrib_test_cmake.sh +++ b/test/distrib/cpp/run_distrib_test_cmake.sh @@ -31,6 +31,7 @@ cd cmake/build cmake -DCMAKE_BUILD_TYPE=Release ../.. make -j4 install cd ../../../../.. +rm -rf third_party/cares/cares # wipe out to prevent influencing the grpc build # Install zlib cd third_party/zlib @@ -39,6 +40,7 @@ cd cmake/build cmake -DCMAKE_BUILD_TYPE=Release ../.. make -j4 install cd ../../../.. +rm -rf third_party/zlib # wipe out to prevent influencing the grpc build # Install protobuf cd third_party/protobuf @@ -47,8 +49,7 @@ cd cmake/build cmake -Dprotobuf_BUILD_TESTS=OFF -DCMAKE_BUILD_TYPE=Release .. make -j4 install cd ../../../.. - -# TODO: Install boringssl +rm -rf third_party/protobuf # wipe out to prevent influencing the grpc build # Install gRPC mkdir -p cmake/build From ae6fd66cef94bf9d4d89bcdcf64a7463ef0e77f2 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Mon, 28 Aug 2017 12:21:46 +0200 Subject: [PATCH 11/21] fix zlib and openssl package mode --- CMakeLists.txt | 18 +++++++----------- templates/CMakeLists.txt.template | 18 +++++++----------- 2 files changed, 14 insertions(+), 22 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index a82e9ef6d03..dd68016be86 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -123,10 +123,8 @@ if("${gRPC_ZLIB_PROVIDER}" STREQUAL "module") set(gRPC_INSTALL FALSE) endif() elseif("${gRPC_ZLIB_PROVIDER}" STREQUAL "package") - find_package(ZLIB) - if(TARGET ZLIB::ZLIB) - set(_gRPC_ZLIB_LIBRARIES ZLIB::ZLIB) - endif() + find_package(ZLIB REQUIRED) + set(_gRPC_ZLIB_LIBRARIES ${ZLIB_LIBRARIES}) set(_gRPC_FIND_ZLIB "if(NOT ZLIB_FOUND)\n find_package(ZLIB)\nendif()") endif() @@ -145,7 +143,7 @@ if("${gRPC_CARES_PROVIDER}" STREQUAL "module") set(gRPC_INSTALL FALSE) endif() elseif("${gRPC_CARES_PROVIDER}" STREQUAL "package") - find_package(c-ares CONFIG) + find_package(c-ares REQUIRED CONFIG) if(TARGET c-ares::cares) set(_gRPC_CARES_LIBRARIES c-ares::cares) endif() @@ -189,7 +187,7 @@ if("${gRPC_PROTOBUF_PROVIDER}" STREQUAL "module") set(gRPC_INSTALL FALSE) endif() elseif("${gRPC_PROTOBUF_PROVIDER}" STREQUAL "package") - find_package(Protobuf ${gRPC_PROTOBUF_PACKAGE_TYPE}) + find_package(Protobuf REQUIRED ${gRPC_PROTOBUF_PACKAGE_TYPE}) if(Protobuf_FOUND OR PROTOBUF_FOUND) if(TARGET protobuf::${_gRPC_PROTOBUF_LIBRARY_NAME}) set(_gRPC_PROTOBUF_LIBRARIES protobuf::${_gRPC_PROTOBUF_LIBRARY_NAME}) @@ -234,11 +232,9 @@ if("${gRPC_SSL_PROVIDER}" STREQUAL "module") set(gRPC_INSTALL FALSE) endif() elseif("${gRPC_SSL_PROVIDER}" STREQUAL "package") - find_package(OpenSSL) - if(TARGET OpenSSL::SSL) - set(_gRPC_SSL_LIBRARIES OpenSSL::SSL) - endif() - set(_gRPC_FIND_SSL "if(NOT OpenSSL_FOUND)\n find_package(OpenSSL)\nendif()") + find_package(OpenSSL REQUIRED) + set(_gRPC_SSL_LIBRARIES ${OPENSSL_LIBRARIES}) + set(_gRPC_FIND_SSL "if(NOT OPENSSL_FOUND)\n find_package(OpenSSL)\nendif()") endif() if("${gRPC_GFLAGS_PROVIDER}" STREQUAL "module") diff --git a/templates/CMakeLists.txt.template b/templates/CMakeLists.txt.template index ad79873d562..f2ce9b5a90c 100644 --- a/templates/CMakeLists.txt.template +++ b/templates/CMakeLists.txt.template @@ -168,10 +168,8 @@ set(gRPC_INSTALL FALSE) endif() elseif("<%text>${gRPC_ZLIB_PROVIDER}" STREQUAL "package") - find_package(ZLIB) - if(TARGET ZLIB::ZLIB) - set(_gRPC_ZLIB_LIBRARIES ZLIB::ZLIB) - endif() + find_package(ZLIB REQUIRED) + set(_gRPC_ZLIB_LIBRARIES <%text>${ZLIB_LIBRARIES}) set(_gRPC_FIND_ZLIB "if(NOT ZLIB_FOUND)\n find_package(ZLIB)\nendif()") endif() @@ -190,7 +188,7 @@ set(gRPC_INSTALL FALSE) endif() elseif("<%text>${gRPC_CARES_PROVIDER}" STREQUAL "package") - find_package(c-ares CONFIG) + find_package(c-ares REQUIRED CONFIG) if(TARGET c-ares::cares) set(_gRPC_CARES_LIBRARIES c-ares::cares) endif() @@ -234,7 +232,7 @@ set(gRPC_INSTALL FALSE) endif() elseif("<%text>${gRPC_PROTOBUF_PROVIDER}" STREQUAL "package") - find_package(Protobuf <%text>${gRPC_PROTOBUF_PACKAGE_TYPE}) + find_package(Protobuf REQUIRED <%text>${gRPC_PROTOBUF_PACKAGE_TYPE}) if(Protobuf_FOUND OR PROTOBUF_FOUND) if(TARGET protobuf::<%text>${_gRPC_PROTOBUF_LIBRARY_NAME}) set(_gRPC_PROTOBUF_LIBRARIES protobuf::<%text>${_gRPC_PROTOBUF_LIBRARY_NAME}) @@ -279,11 +277,9 @@ set(gRPC_INSTALL FALSE) endif() elseif("<%text>${gRPC_SSL_PROVIDER}" STREQUAL "package") - find_package(OpenSSL) - if(TARGET OpenSSL::SSL) - set(_gRPC_SSL_LIBRARIES OpenSSL::SSL) - endif() - set(_gRPC_FIND_SSL "if(NOT OpenSSL_FOUND)\n find_package(OpenSSL)\nendif()") + find_package(OpenSSL REQUIRED) + set(_gRPC_SSL_LIBRARIES <%text>${OPENSSL_LIBRARIES}) + set(_gRPC_FIND_SSL "if(NOT OPENSSL_FOUND)\n find_package(OpenSSL)\nendif()") endif() if("<%text>${gRPC_GFLAGS_PROVIDER}" STREQUAL "module") From b29296042a308060574d5cd119d6b05c00660624 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Mon, 11 Sep 2017 09:31:11 -0700 Subject: [PATCH 12/21] More cleanup in client_channel code. --- .../filters/client_channel/client_channel.c | 314 +++++++++--------- 1 file changed, 153 insertions(+), 161 deletions(-) diff --git a/src/core/ext/filters/client_channel/client_channel.c b/src/core/ext/filters/client_channel/client_channel.c index dad5c4fce5b..e5f4a8a813c 100644 --- a/src/core/ext/filters/client_channel/client_channel.c +++ b/src/core/ext/filters/client_channel/client_channel.c @@ -1016,13 +1016,11 @@ static void create_subchannel_call_locked(grpc_exec_ctx *exec_ctx, GRPC_ERROR_UNREF(error); } -static void subchannel_ready_locked(grpc_exec_ctx *exec_ctx, - grpc_call_element *elem, - grpc_error *error) { +// Invoked when a pick is completed, on both success or failure. +static void pick_done_locked(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, + grpc_error *error) { call_data *calld = (call_data *)elem->call_data; channel_data *chand = (channel_data *)elem->channel_data; - grpc_polling_entity_del_from_pollset_set(exec_ctx, calld->pollent, - chand->interested_parties); if (calld->connected_subchannel == NULL) { // Failed to create subchannel. GRPC_ERROR_UNREF(calld->error); @@ -1044,12 +1042,116 @@ static void subchannel_ready_locked(grpc_exec_ctx *exec_ctx, GRPC_ERROR_UNREF(error); } -/** Return true if subchannel is available immediately (in which case - subchannel_ready_locked() should not be called), or false otherwise (in - which case subchannel_ready_locked() should be called when the subchannel - is available). */ -static bool pick_subchannel_locked(grpc_exec_ctx *exec_ctx, - grpc_call_element *elem); +// A wrapper around pick_done_locked() that is used in cases where +// either (a) the pick was deferred pending a resolver result or (b) the +// pick was done asynchronously. Removes the call's polling entity from +// chand->interested_parties before invoking pick_done_locked(). +static void async_pick_done_locked(grpc_exec_ctx *exec_ctx, + grpc_call_element *elem, grpc_error *error) { + channel_data *chand = (channel_data *)elem->channel_data; + call_data *calld = (call_data *)elem->call_data; + grpc_polling_entity_del_from_pollset_set(exec_ctx, calld->pollent, + chand->interested_parties); + pick_done_locked(exec_ctx, elem, error); +} + +// Note: This runs under the client_channel combiner, but will NOT be +// holding the call combiner. +static void pick_callback_cancel_locked(grpc_exec_ctx *exec_ctx, void *arg, + grpc_error *error) { + grpc_call_element *elem = (grpc_call_element *)arg; + channel_data *chand = (channel_data *)elem->channel_data; + call_data *calld = (call_data *)elem->call_data; + if (calld->lb_policy != NULL) { + if (GRPC_TRACER_ON(grpc_client_channel_trace)) { + gpr_log(GPR_DEBUG, "chand=%p calld=%p: cancelling pick from LB policy %p", + chand, calld, calld->lb_policy); + } + grpc_lb_policy_cancel_pick_locked(exec_ctx, calld->lb_policy, + &calld->connected_subchannel, + GRPC_ERROR_REF(error)); + } + GRPC_CALL_STACK_UNREF(exec_ctx, calld->owning_call, "pick_callback_cancel"); +} + +// Callback invoked by grpc_lb_policy_pick_locked() for async picks. +// Unrefs the LB policy and invokes async_pick_done_locked(). +static void pick_callback_done_locked(grpc_exec_ctx *exec_ctx, void *arg, + grpc_error *error) { + grpc_call_element *elem = (grpc_call_element *)arg; + channel_data *chand = (channel_data *)elem->channel_data; + call_data *calld = (call_data *)elem->call_data; + if (GRPC_TRACER_ON(grpc_client_channel_trace)) { + gpr_log(GPR_DEBUG, "chand=%p calld=%p: pick completed asynchronously", + chand, calld); + } + GPR_ASSERT(calld->lb_policy != NULL); + GRPC_LB_POLICY_UNREF(exec_ctx, calld->lb_policy, "pick_subchannel"); + calld->lb_policy = NULL; + async_pick_done_locked(exec_ctx, elem, GRPC_ERROR_REF(error)); +} + +// Takes a ref to chand->lb_policy and calls grpc_lb_policy_pick_locked(). +// If the pick was completed synchronously, unrefs the LB policy and +// returns true. +static bool pick_callback_start_locked(grpc_exec_ctx *exec_ctx, + grpc_call_element *elem) { + channel_data *chand = (channel_data *)elem->channel_data; + call_data *calld = (call_data *)elem->call_data; + if (GRPC_TRACER_ON(grpc_client_channel_trace)) { + gpr_log(GPR_DEBUG, "chand=%p calld=%p: starting pick on lb_policy=%p", + chand, calld, chand->lb_policy); + } + apply_service_config_to_call_locked(exec_ctx, elem); + // If the application explicitly set wait_for_ready, use that. + // Otherwise, if the service config specified a value for this + // method, use that. + uint32_t initial_metadata_flags = + calld->initial_metadata_batch->payload->send_initial_metadata + .send_initial_metadata_flags; + const bool wait_for_ready_set_from_api = + initial_metadata_flags & + GRPC_INITIAL_METADATA_WAIT_FOR_READY_EXPLICITLY_SET; + const bool wait_for_ready_set_from_service_config = + calld->method_params != NULL && + calld->method_params->wait_for_ready != WAIT_FOR_READY_UNSET; + if (!wait_for_ready_set_from_api && wait_for_ready_set_from_service_config) { + if (calld->method_params->wait_for_ready == WAIT_FOR_READY_TRUE) { + initial_metadata_flags |= GRPC_INITIAL_METADATA_WAIT_FOR_READY; + } else { + initial_metadata_flags &= ~GRPC_INITIAL_METADATA_WAIT_FOR_READY; + } + } + const grpc_lb_policy_pick_args inputs = { + calld->initial_metadata_batch->payload->send_initial_metadata + .send_initial_metadata, + initial_metadata_flags, &calld->lb_token_mdelem}; + // Keep a ref to the LB policy in calld while the pick is pending. + GRPC_LB_POLICY_REF(chand->lb_policy, "pick_subchannel"); + calld->lb_policy = chand->lb_policy; + GRPC_CLOSURE_INIT(&calld->lb_pick_closure, pick_callback_done_locked, elem, + grpc_combiner_scheduler(chand->combiner)); + const bool pick_done = grpc_lb_policy_pick_locked( + exec_ctx, chand->lb_policy, &inputs, &calld->connected_subchannel, + calld->subchannel_call_context, NULL, &calld->lb_pick_closure); + if (pick_done) { + /* synchronous grpc_lb_policy_pick call. Unref the LB policy. */ + if (GRPC_TRACER_ON(grpc_client_channel_trace)) { + gpr_log(GPR_DEBUG, "chand=%p calld=%p: pick completed synchronously", + chand, calld); + } + GRPC_LB_POLICY_UNREF(exec_ctx, calld->lb_policy, "pick_subchannel"); + calld->lb_policy = NULL; + } else { + GRPC_CALL_STACK_REF(calld->owning_call, "pick_callback_cancel"); + grpc_call_combiner_set_notify_on_cancel( + exec_ctx, calld->call_combiner, + GRPC_CLOSURE_INIT(&calld->lb_pick_cancel_closure, + pick_callback_cancel_locked, elem, + grpc_combiner_scheduler(chand->combiner))); + } + return pick_done; +} typedef struct { grpc_call_element *elem; @@ -1069,17 +1171,17 @@ static void pick_after_resolver_result_cancel_locked(grpc_exec_ctx *exec_ctx, gpr_free(args); return; } - args->finished = true; - grpc_call_element *elem = args->elem; - channel_data *chand = (channel_data *)elem->channel_data; - call_data *calld = (call_data *)elem->call_data; // If we don't yet have a resolver result, then a closure for // pick_after_resolver_result_done_locked() will have been added to // chand->waiting_for_resolver_result_closures, and it may not be invoked // until after this call has been destroyed. We mark the operation as // finished, so that when pick_after_resolver_result_done_locked() // is called, it will be a no-op. We also immediately invoke - // subchannel_ready_locked() to propagate the error back to the caller. + // async_pick_done_locked() to propagate the error back to the caller. + args->finished = true; + grpc_call_element *elem = args->elem; + channel_data *chand = (channel_data *)elem->channel_data; + call_data *calld = (call_data *)elem->call_data; if (GRPC_TRACER_ON(grpc_client_channel_trace)) { gpr_log(GPR_DEBUG, "chand=%p calld=%p: cancelling pick waiting for resolver result", @@ -1087,12 +1189,12 @@ static void pick_after_resolver_result_cancel_locked(grpc_exec_ctx *exec_ctx, } // Note: Although we are not in the call combiner here, we are // basically stealing the call combiner from the pending pick, so - // it's safe to call subchannel_ready_locked() here -- we are + // it's safe to call async_pick_done_locked() here -- we are // essentially calling it here instead of calling it in // pick_after_resolver_result_done_locked(). - subchannel_ready_locked(exec_ctx, elem, - GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING( - "Pick cancelled", &error, 1)); + async_pick_done_locked(exec_ctx, elem, + GRPC_ERROR_CREATE_REFERENCING_FROM_STATIC_STRING( + "Pick cancelled", &error, 1)); } static void pick_after_resolver_result_done_locked(grpc_exec_ctx *exec_ctx, @@ -1117,14 +1219,19 @@ static void pick_after_resolver_result_done_locked(grpc_exec_ctx *exec_ctx, gpr_log(GPR_DEBUG, "chand=%p calld=%p: resolver failed to return data", chand, calld); } - subchannel_ready_locked(exec_ctx, elem, GRPC_ERROR_REF(error)); + async_pick_done_locked(exec_ctx, elem, GRPC_ERROR_REF(error)); } else { if (GRPC_TRACER_ON(grpc_client_channel_trace)) { gpr_log(GPR_DEBUG, "chand=%p calld=%p: resolver returned, doing pick", chand, calld); } - if (pick_subchannel_locked(exec_ctx, elem)) { - subchannel_ready_locked(exec_ctx, elem, GRPC_ERROR_NONE); + if (pick_callback_start_locked(exec_ctx, elem)) { + // Even if the LB policy returns a result synchronously, we have + // already added our polling entity to chand->interested_parties + // in order to wait for the resolver result, so we need to + // remove it here. Therefore, we call async_pick_done_locked() + // instead of pick_done_locked(). + async_pick_done_locked(exec_ctx, elem, GRPC_ERROR_NONE); } } } @@ -1152,154 +1259,38 @@ static void pick_after_resolver_result_start_locked(grpc_exec_ctx *exec_ctx, grpc_combiner_scheduler(chand->combiner))); } -// Note: This runs under the client_channel combiner, but will NOT be -// holding the call combiner. -static void pick_callback_cancel_locked(grpc_exec_ctx *exec_ctx, void *arg, - grpc_error *error) { - grpc_call_element *elem = (grpc_call_element *)arg; - channel_data *chand = (channel_data *)elem->channel_data; - call_data *calld = (call_data *)elem->call_data; - if (error != GRPC_ERROR_NONE && calld->lb_policy != NULL) { - if (GRPC_TRACER_ON(grpc_client_channel_trace)) { - gpr_log(GPR_DEBUG, "chand=%p calld=%p: cancelling pick from LB policy %p", - chand, calld, calld->lb_policy); - } - grpc_lb_policy_cancel_pick_locked(exec_ctx, calld->lb_policy, - &calld->connected_subchannel, - GRPC_ERROR_REF(error)); - } - GRPC_CALL_STACK_UNREF(exec_ctx, calld->owning_call, "pick_callback_cancel"); -} - -// Callback invoked by grpc_lb_policy_pick_locked() for async picks. -// Unrefs the LB policy and invokes subchannel_ready_locked(). -static void pick_callback_done_locked(grpc_exec_ctx *exec_ctx, void *arg, - grpc_error *error) { +static void start_pick_locked(grpc_exec_ctx *exec_ctx, void *arg, + grpc_error *ignored) { grpc_call_element *elem = (grpc_call_element *)arg; - channel_data *chand = (channel_data *)elem->channel_data; call_data *calld = (call_data *)elem->call_data; - if (GRPC_TRACER_ON(grpc_client_channel_trace)) { - gpr_log(GPR_DEBUG, "chand=%p calld=%p: pick completed asynchronously", - chand, calld); - } - GPR_ASSERT(calld->lb_policy != NULL); - GRPC_LB_POLICY_UNREF(exec_ctx, calld->lb_policy, "pick_subchannel"); - calld->lb_policy = NULL; - subchannel_ready_locked(exec_ctx, elem, GRPC_ERROR_REF(error)); -} - -// Takes a ref to chand->lb_policy and calls grpc_lb_policy_pick_locked(). -// If the pick was completed synchronously, unrefs the LB policy and -// returns true. -static bool pick_callback_start_locked(grpc_exec_ctx *exec_ctx, - grpc_call_element *elem, - const grpc_lb_policy_pick_args *inputs) { channel_data *chand = (channel_data *)elem->channel_data; - call_data *calld = (call_data *)elem->call_data; - if (GRPC_TRACER_ON(grpc_client_channel_trace)) { - gpr_log(GPR_DEBUG, "chand=%p calld=%p: starting pick on lb_policy=%p", - chand, calld, chand->lb_policy); - } - // Keep a ref to the LB policy in calld while the pick is pending. - GRPC_LB_POLICY_REF(chand->lb_policy, "pick_subchannel"); - calld->lb_policy = chand->lb_policy; - GRPC_CLOSURE_INIT(&calld->lb_pick_closure, pick_callback_done_locked, elem, - grpc_combiner_scheduler(chand->combiner)); - const bool pick_done = grpc_lb_policy_pick_locked( - exec_ctx, chand->lb_policy, inputs, &calld->connected_subchannel, - calld->subchannel_call_context, NULL, &calld->lb_pick_closure); - if (pick_done) { - /* synchronous grpc_lb_policy_pick call. Unref the LB policy. */ - if (GRPC_TRACER_ON(grpc_client_channel_trace)) { - gpr_log(GPR_DEBUG, "chand=%p calld=%p: pick completed synchronously", - chand, calld); + GPR_ASSERT(calld->connected_subchannel == NULL); + if (chand->lb_policy != NULL) { + // We already have an LB policy, so ask it for a pick. + if (pick_callback_start_locked(exec_ctx, elem)) { + // Pick completed synchronously. + pick_done_locked(exec_ctx, elem, GRPC_ERROR_NONE); + return; } - GRPC_LB_POLICY_UNREF(exec_ctx, calld->lb_policy, "pick_subchannel"); - calld->lb_policy = NULL; } else { - GRPC_CALL_STACK_REF(calld->owning_call, "pick_callback_cancel"); - grpc_call_combiner_set_notify_on_cancel( - exec_ctx, calld->call_combiner, - GRPC_CLOSURE_INIT(&calld->lb_pick_cancel_closure, - pick_callback_cancel_locked, elem, - grpc_combiner_scheduler(chand->combiner))); - } - return pick_done; -} - -static bool pick_subchannel_locked(grpc_exec_ctx *exec_ctx, - grpc_call_element *elem) { - GPR_TIMER_BEGIN("pick_subchannel", 0); - channel_data *chand = (channel_data *)elem->channel_data; - call_data *calld = (call_data *)elem->call_data; - bool pick_done = false; - if (chand->lb_policy != NULL) { - apply_service_config_to_call_locked(exec_ctx, elem); - // If the application explicitly set wait_for_ready, use that. - // Otherwise, if the service config specified a value for this - // method, use that. - uint32_t initial_metadata_flags = - calld->initial_metadata_batch->payload->send_initial_metadata - .send_initial_metadata_flags; - const bool wait_for_ready_set_from_api = - initial_metadata_flags & - GRPC_INITIAL_METADATA_WAIT_FOR_READY_EXPLICITLY_SET; - const bool wait_for_ready_set_from_service_config = - calld->method_params != NULL && - calld->method_params->wait_for_ready != WAIT_FOR_READY_UNSET; - if (!wait_for_ready_set_from_api && - wait_for_ready_set_from_service_config) { - if (calld->method_params->wait_for_ready == WAIT_FOR_READY_TRUE) { - initial_metadata_flags |= GRPC_INITIAL_METADATA_WAIT_FOR_READY; - } else { - initial_metadata_flags &= ~GRPC_INITIAL_METADATA_WAIT_FOR_READY; - } + // We do not yet have an LB policy, so wait for a resolver result. + if (chand->resolver == NULL) { + pick_done_locked(exec_ctx, elem, + GRPC_ERROR_CREATE_FROM_STATIC_STRING("Disconnected")); + return; } - const grpc_lb_policy_pick_args inputs = { - calld->initial_metadata_batch->payload->send_initial_metadata - .send_initial_metadata, - initial_metadata_flags, &calld->lb_token_mdelem}; - pick_done = pick_callback_start_locked(exec_ctx, elem, &inputs); - } else if (chand->resolver != NULL) { if (!chand->started_resolving) { start_resolving_locked(exec_ctx, chand); } pick_after_resolver_result_start_locked(exec_ctx, elem); - } else { - subchannel_ready_locked( - exec_ctx, elem, GRPC_ERROR_CREATE_FROM_STATIC_STRING("Disconnected")); - } - GPR_TIMER_END("pick_subchannel", 0); - return pick_done; -} - -static void start_pick_locked(grpc_exec_ctx *exec_ctx, void *arg, - grpc_error *error_ignored) { - GPR_TIMER_BEGIN("start_pick_locked", 0); - grpc_call_element *elem = (grpc_call_element *)arg; - call_data *calld = (call_data *)elem->call_data; - channel_data *chand = (channel_data *)elem->channel_data; - GPR_ASSERT(calld->connected_subchannel == NULL); - if (pick_subchannel_locked(exec_ctx, elem)) { - // Pick was returned synchronously. - if (calld->connected_subchannel == NULL) { - GRPC_ERROR_UNREF(calld->error); - calld->error = GRPC_ERROR_CREATE_FROM_STATIC_STRING( - "Call dropped by load balancing policy"); - waiting_for_pick_batches_fail(exec_ctx, elem, - GRPC_ERROR_REF(calld->error)); - } else { - // Create subchannel call. - create_subchannel_call_locked(exec_ctx, elem, GRPC_ERROR_NONE); - } - } else { - // Pick will be done asynchronously. Add the call's polling entity to - // the channel's interested_parties, so that I/O for the resolver - // and LB policy can be done under it. - grpc_polling_entity_add_to_pollset_set(exec_ctx, calld->pollent, - chand->interested_parties); } - GPR_TIMER_END("start_pick_locked", 0); + // We need to wait for either a resolver result or for an async result + // from the LB policy. Add the polling entity from call_data to the + // channel_data's interested_parties, so that the I/O of the LB policy + // and resolver can be done under it. The polling entity will be + // removed in async_pick_done_locked(). + grpc_polling_entity_add_to_pollset_set(exec_ctx, calld->pollent, + chand->interested_parties); } static void on_complete(grpc_exec_ctx *exec_ctx, void *arg, grpc_error *error) { @@ -1394,7 +1385,8 @@ static void cc_start_transport_stream_op_batch( // combiner to start a pick. if (batch->send_initial_metadata) { if (GRPC_TRACER_ON(grpc_client_channel_trace)) { - gpr_log(GPR_DEBUG, "chand=%p calld=%p: entering combiner", chand, calld); + gpr_log(GPR_DEBUG, "chand=%p calld=%p: entering client_channel combiner", + chand, calld); } GRPC_CLOSURE_SCHED( exec_ctx, From 0ebc3ad22b50ec759b5949f682bdf4b679f305b4 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Mon, 11 Sep 2017 11:29:59 -0700 Subject: [PATCH 13/21] Temporarily take ownership of cpp generator to prevent changes --- .github/CODEOWNERS | 1 + src/compiler/OWNERS | 1 + 2 files changed, 2 insertions(+) create mode 100644 src/compiler/OWNERS diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 094e43e4705..2a4eacd9981 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -3,4 +3,5 @@ # repository as the source of truth for module ownership. /**/OWNERS @markdroth @nicolasnoble @ctiller /bazel/** @nicolasnoble @dgquintas @ctiller +/src/compiler/cpp_generator.cc @vjpai /src/core/ext/filters/client_channel/** @markdroth @dgquintas @ctiller diff --git a/src/compiler/OWNERS b/src/compiler/OWNERS new file mode 100644 index 00000000000..96b89fc60ff --- /dev/null +++ b/src/compiler/OWNERS @@ -0,0 +1 @@ +@vjpai cpp_generator.cc From c75ae78b0806f8ecea3ddfedbf4c2497331bbca5 Mon Sep 17 00:00:00 2001 From: Nathaniel Manista Date: Mon, 11 Sep 2017 19:21:34 +0000 Subject: [PATCH 14/21] Add more Cython-layer tests I wrote these in the course of a bug hunt. I haven't yet caught and fixed the bug, but that's no reason not to check in perfectly good tests. :-) --- src/python/grpcio_tests/tests/tests.json | 2 + .../tests/unit/_cython/_common.py | 118 ++++++++++++++++ ...s_server_completion_queue_per_call_test.py | 131 ++++++++++++++++++ ...ges_single_server_completion_queue_test.py | 126 +++++++++++++++++ 4 files changed, 377 insertions(+) create mode 100644 src/python/grpcio_tests/tests/unit/_cython/_common.py create mode 100644 src/python/grpcio_tests/tests/unit/_cython/_no_messages_server_completion_queue_per_call_test.py create mode 100644 src/python/grpcio_tests/tests/unit/_cython/_no_messages_single_server_completion_queue_test.py diff --git a/src/python/grpcio_tests/tests/tests.json b/src/python/grpcio_tests/tests/tests.json index d61297b9187..4c078e6c22b 100644 --- a/src/python/grpcio_tests/tests/tests.json +++ b/src/python/grpcio_tests/tests/tests.json @@ -26,6 +26,8 @@ "unit._credentials_test.CredentialsTest", "unit._cython._cancel_many_calls_test.CancelManyCallsTest", "unit._cython._channel_test.ChannelTest", + "unit._cython._no_messages_server_completion_queue_per_call_test.Test", + "unit._cython._no_messages_single_server_completion_queue_test.Test", "unit._cython._read_some_but_not_all_responses_test.ReadSomeButNotAllResponsesTest", "unit._cython.cygrpc_test.InsecureServerInsecureClient", "unit._cython.cygrpc_test.SecureServerSecureClient", diff --git a/src/python/grpcio_tests/tests/unit/_cython/_common.py b/src/python/grpcio_tests/tests/unit/_cython/_common.py new file mode 100644 index 00000000000..ac66d1db3d8 --- /dev/null +++ b/src/python/grpcio_tests/tests/unit/_cython/_common.py @@ -0,0 +1,118 @@ +# Copyright 2017 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Common utilities for tests of the Cython layer of gRPC Python.""" + +import collections +import threading + +from grpc._cython import cygrpc + +RPC_COUNT = 4000 + +INFINITE_FUTURE = cygrpc.Timespec(float('+inf')) +EMPTY_FLAGS = 0 + +INVOCATION_METADATA = cygrpc.Metadata( + (cygrpc.Metadatum(b'client-md-key', b'client-md-key'), + cygrpc.Metadatum(b'client-md-key-bin', b'\x00\x01' * 3000),)) + +INITIAL_METADATA = cygrpc.Metadata( + (cygrpc.Metadatum(b'server-initial-md-key', b'server-initial-md-value'), + cygrpc.Metadatum(b'server-initial-md-key-bin', b'\x00\x02' * 3000),)) + +TRAILING_METADATA = cygrpc.Metadata( + (cygrpc.Metadatum(b'server-trailing-md-key', b'server-trailing-md-value'), + cygrpc.Metadatum(b'server-trailing-md-key-bin', b'\x00\x03' * 3000),)) + + +class QueueDriver(object): + + def __init__(self, condition, completion_queue): + self._condition = condition + self._completion_queue = completion_queue + self._due = collections.defaultdict(int) + self._events = collections.defaultdict(list) + + def add_due(self, tags): + if not self._due: + + def in_thread(): + while True: + event = self._completion_queue.poll() + with self._condition: + self._events[event.tag].append(event) + self._due[event.tag] -= 1 + self._condition.notify_all() + if self._due[event.tag] <= 0: + self._due.pop(event.tag) + if not self._due: + return + + thread = threading.Thread(target=in_thread) + thread.start() + for tag in tags: + self._due[tag] += 1 + + def event_with_tag(self, tag): + with self._condition: + while True: + if self._events[tag]: + return self._events[tag].pop(0) + else: + self._condition.wait() + + +def execute_many_times(behavior): + return tuple(behavior() for _ in range(RPC_COUNT)) + + +class OperationResult( + collections.namedtuple('OperationResult', ( + 'start_batch_result', 'completion_type', 'success',))): + pass + + +SUCCESSFUL_OPERATION_RESULT = OperationResult( + cygrpc.CallError.ok, cygrpc.CompletionType.operation_complete, True) + + +class RpcTest(object): + + def setUp(self): + self.server_completion_queue = cygrpc.CompletionQueue() + self.server = cygrpc.Server(cygrpc.ChannelArgs([])) + self.server.register_completion_queue(self.server_completion_queue) + port = self.server.add_http2_port(b'[::]:0') + self.server.start() + self.channel = cygrpc.Channel('localhost:{}'.format(port).encode(), + cygrpc.ChannelArgs([])) + + self._server_shutdown_tag = 'server_shutdown_tag' + self.server_condition = threading.Condition() + self.server_driver = QueueDriver(self.server_condition, + self.server_completion_queue) + with self.server_condition: + self.server_driver.add_due({ + self._server_shutdown_tag, + }) + + self.client_condition = threading.Condition() + self.client_completion_queue = cygrpc.CompletionQueue() + self.client_driver = QueueDriver(self.client_condition, + self.client_completion_queue) + + def tearDown(self): + self.server.shutdown(self.server_completion_queue, + self._server_shutdown_tag) + self.server.cancel_all_calls() diff --git a/src/python/grpcio_tests/tests/unit/_cython/_no_messages_server_completion_queue_per_call_test.py b/src/python/grpcio_tests/tests/unit/_cython/_no_messages_server_completion_queue_per_call_test.py new file mode 100644 index 00000000000..14cc66675c3 --- /dev/null +++ b/src/python/grpcio_tests/tests/unit/_cython/_no_messages_server_completion_queue_per_call_test.py @@ -0,0 +1,131 @@ +# Copyright 2017 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Test a corner-case at the level of the Cython API.""" + +import threading +import unittest + +from grpc._cython import cygrpc + +from tests.unit._cython import _common + + +class Test(_common.RpcTest, unittest.TestCase): + + def _do_rpcs(self): + server_call_condition = threading.Condition() + server_call_completion_queue = cygrpc.CompletionQueue() + server_call_driver = _common.QueueDriver(server_call_condition, + server_call_completion_queue) + + server_request_call_tag = 'server_request_call_tag' + server_send_initial_metadata_tag = 'server_send_initial_metadata_tag' + server_complete_rpc_tag = 'server_complete_rpc_tag' + + with self.server_condition: + server_request_call_start_batch_result = self.server.request_call( + server_call_completion_queue, self.server_completion_queue, + server_request_call_tag) + self.server_driver.add_due({ + server_request_call_tag, + }) + + client_call = self.channel.create_call( + None, _common.EMPTY_FLAGS, self.client_completion_queue, + b'/twinkies', None, _common.INFINITE_FUTURE) + client_receive_initial_metadata_tag = 'client_receive_initial_metadata_tag' + client_complete_rpc_tag = 'client_complete_rpc_tag' + with self.client_condition: + client_receive_initial_metadata_start_batch_result = ( + client_call.start_client_batch( + cygrpc.Operations([ + cygrpc.operation_receive_initial_metadata( + _common.EMPTY_FLAGS), + ]), client_receive_initial_metadata_tag)) + client_complete_rpc_start_batch_result = client_call.start_client_batch( + cygrpc.Operations([ + cygrpc.operation_send_initial_metadata( + _common.INVOCATION_METADATA, _common.EMPTY_FLAGS), + cygrpc.operation_send_close_from_client( + _common.EMPTY_FLAGS), + cygrpc.operation_receive_status_on_client( + _common.EMPTY_FLAGS), + ]), client_complete_rpc_tag) + self.client_driver.add_due({ + client_receive_initial_metadata_tag, + client_complete_rpc_tag, + }) + + server_request_call_event = self.server_driver.event_with_tag( + server_request_call_tag) + + with server_call_condition: + server_send_initial_metadata_start_batch_result = ( + server_request_call_event.operation_call.start_server_batch([ + cygrpc.operation_send_initial_metadata( + _common.INITIAL_METADATA, _common.EMPTY_FLAGS), + ], server_send_initial_metadata_tag)) + server_call_driver.add_due({ + server_send_initial_metadata_tag, + }) + server_send_initial_metadata_event = server_call_driver.event_with_tag( + server_send_initial_metadata_tag) + + with server_call_condition: + server_complete_rpc_start_batch_result = ( + server_request_call_event.operation_call.start_server_batch([ + cygrpc.operation_receive_close_on_server( + _common.EMPTY_FLAGS), + cygrpc.operation_send_status_from_server( + _common.TRAILING_METADATA, cygrpc.StatusCode.ok, + b'test details', _common.EMPTY_FLAGS), + ], server_complete_rpc_tag)) + server_call_driver.add_due({ + server_complete_rpc_tag, + }) + server_complete_rpc_event = server_call_driver.event_with_tag( + server_complete_rpc_tag) + + client_receive_initial_metadata_event = self.client_driver.event_with_tag( + client_receive_initial_metadata_tag) + client_complete_rpc_event = self.client_driver.event_with_tag( + client_complete_rpc_tag) + + return (_common.OperationResult(server_request_call_start_batch_result, + server_request_call_event.type, + server_request_call_event.success), + _common.OperationResult( + client_receive_initial_metadata_start_batch_result, + client_receive_initial_metadata_event.type, + client_receive_initial_metadata_event.success), + _common.OperationResult(client_complete_rpc_start_batch_result, + client_complete_rpc_event.type, + client_complete_rpc_event.success), + _common.OperationResult( + server_send_initial_metadata_start_batch_result, + server_send_initial_metadata_event.type, + server_send_initial_metadata_event.success), + _common.OperationResult(server_complete_rpc_start_batch_result, + server_complete_rpc_event.type, + server_complete_rpc_event.success),) + + def test_rpcs(self): + expecteds = [(_common.SUCCESSFUL_OPERATION_RESULT,) * + 5] * _common.RPC_COUNT + actuallys = _common.execute_many_times(self._do_rpcs) + self.assertSequenceEqual(expecteds, actuallys) + + +if __name__ == '__main__': + unittest.main(verbosity=2) diff --git a/src/python/grpcio_tests/tests/unit/_cython/_no_messages_single_server_completion_queue_test.py b/src/python/grpcio_tests/tests/unit/_cython/_no_messages_single_server_completion_queue_test.py new file mode 100644 index 00000000000..1e44bcc4dc6 --- /dev/null +++ b/src/python/grpcio_tests/tests/unit/_cython/_no_messages_single_server_completion_queue_test.py @@ -0,0 +1,126 @@ +# Copyright 2017 gRPC authors. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +"""Test a corner-case at the level of the Cython API.""" + +import threading +import unittest + +from grpc._cython import cygrpc + +from tests.unit._cython import _common + + +class Test(_common.RpcTest, unittest.TestCase): + + def _do_rpcs(self): + server_request_call_tag = 'server_request_call_tag' + server_send_initial_metadata_tag = 'server_send_initial_metadata_tag' + server_complete_rpc_tag = 'server_complete_rpc_tag' + + with self.server_condition: + server_request_call_start_batch_result = self.server.request_call( + self.server_completion_queue, self.server_completion_queue, + server_request_call_tag) + self.server_driver.add_due({ + server_request_call_tag, + }) + + client_call = self.channel.create_call( + None, _common.EMPTY_FLAGS, self.client_completion_queue, + b'/twinkies', None, _common.INFINITE_FUTURE) + client_receive_initial_metadata_tag = 'client_receive_initial_metadata_tag' + client_complete_rpc_tag = 'client_complete_rpc_tag' + with self.client_condition: + client_receive_initial_metadata_start_batch_result = ( + client_call.start_client_batch( + cygrpc.Operations([ + cygrpc.operation_receive_initial_metadata( + _common.EMPTY_FLAGS), + ]), client_receive_initial_metadata_tag)) + client_complete_rpc_start_batch_result = client_call.start_client_batch( + cygrpc.Operations([ + cygrpc.operation_send_initial_metadata( + _common.INVOCATION_METADATA, _common.EMPTY_FLAGS), + cygrpc.operation_send_close_from_client( + _common.EMPTY_FLAGS), + cygrpc.operation_receive_status_on_client( + _common.EMPTY_FLAGS), + ]), client_complete_rpc_tag) + self.client_driver.add_due({ + client_receive_initial_metadata_tag, + client_complete_rpc_tag, + }) + + server_request_call_event = self.server_driver.event_with_tag( + server_request_call_tag) + + with self.server_condition: + server_send_initial_metadata_start_batch_result = ( + server_request_call_event.operation_call.start_server_batch([ + cygrpc.operation_send_initial_metadata( + _common.INITIAL_METADATA, _common.EMPTY_FLAGS), + ], server_send_initial_metadata_tag)) + self.server_driver.add_due({ + server_send_initial_metadata_tag, + }) + server_send_initial_metadata_event = self.server_driver.event_with_tag( + server_send_initial_metadata_tag) + + with self.server_condition: + server_complete_rpc_start_batch_result = ( + server_request_call_event.operation_call.start_server_batch([ + cygrpc.operation_receive_close_on_server( + _common.EMPTY_FLAGS), + cygrpc.operation_send_status_from_server( + _common.TRAILING_METADATA, cygrpc.StatusCode.ok, + b'test details', _common.EMPTY_FLAGS), + ], server_complete_rpc_tag)) + self.server_driver.add_due({ + server_complete_rpc_tag, + }) + server_complete_rpc_event = self.server_driver.event_with_tag( + server_complete_rpc_tag) + + client_receive_initial_metadata_event = self.client_driver.event_with_tag( + client_receive_initial_metadata_tag) + client_complete_rpc_event = self.client_driver.event_with_tag( + client_complete_rpc_tag) + + return (_common.OperationResult(server_request_call_start_batch_result, + server_request_call_event.type, + server_request_call_event.success), + _common.OperationResult( + client_receive_initial_metadata_start_batch_result, + client_receive_initial_metadata_event.type, + client_receive_initial_metadata_event.success), + _common.OperationResult(client_complete_rpc_start_batch_result, + client_complete_rpc_event.type, + client_complete_rpc_event.success), + _common.OperationResult( + server_send_initial_metadata_start_batch_result, + server_send_initial_metadata_event.type, + server_send_initial_metadata_event.success), + _common.OperationResult(server_complete_rpc_start_batch_result, + server_complete_rpc_event.type, + server_complete_rpc_event.success),) + + def test_rpcs(self): + expecteds = [(_common.SUCCESSFUL_OPERATION_RESULT,) * + 5] * _common.RPC_COUNT + actuallys = _common.execute_many_times(self._do_rpcs) + self.assertSequenceEqual(expecteds, actuallys) + + +if __name__ == '__main__': + unittest.main(verbosity=2) From 54f3c2eefe65a2f5586b8c1ee8b7c9981502fe85 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 11 Sep 2017 13:12:04 -0700 Subject: [PATCH 15/21] Automatically fetch schema from BigQuery --- tools/profiling/microbenchmarks/bm2bq.py | 43 +++++------------------- 1 file changed, 8 insertions(+), 35 deletions(-) diff --git a/tools/profiling/microbenchmarks/bm2bq.py b/tools/profiling/microbenchmarks/bm2bq.py index 0797f3e21d3..6c5040de56d 100755 --- a/tools/profiling/microbenchmarks/bm2bq.py +++ b/tools/profiling/microbenchmarks/bm2bq.py @@ -22,42 +22,15 @@ import sys import json import csv import bm_json +import json +import subprocess + +columns = [] -columns = [ - ('jenkins_build', 'integer'), - ('jenkins_job', 'string'), - ('date', 'timestamp'), - ('cpu_scaling_enabled', 'boolean'), - ('num_cpus', 'integer'), - ('mhz_per_cpu', 'integer'), - ('library_build_type', 'string'), - ('name', 'string'), - ('fixture', 'string'), - ('client_mutator', 'string'), - ('server_mutator', 'string'), - ('request_size', 'integer'), - ('response_size', 'integer'), - ('request_count', 'integer'), - ('iterations', 'integer'), - ('time_unit', 'string'), - ('real_time', 'integer'), - ('cpu_time', 'integer'), - ('bytes_per_second', 'float'), - ('allocs_per_iteration', 'float'), - ('locks_per_iteration', 'float'), - ('writes_per_iteration', 'float'), - ('bandwidth_kilobits', 'integer'), - ('cli_transport_stalls_per_iteration', 'float'), - ('cli_stream_stalls_per_iteration', 'float'), - ('svr_transport_stalls_per_iteration', 'float'), - ('svr_stream_stalls_per_iteration', 'float'), - ('atm_cas_per_iteration', 'float'), - ('atm_add_per_iteration', 'float'), - ('end_of_stream', 'boolean'), - ('header_bytes_per_iteration', 'float'), - ('framing_bytes_per_iteration', 'float'), - ('nows_per_iteration', 'float'), -] +for row in json.loads( + subprocess.check_output([ + 'bq','--format=json','show','microbenchmarks.microbenchmarks']))['schema']['fields']: + columns.append((row['name'], row['type'])) SANITIZE = { 'integer': int, From 4b7fe94a9728686065a0cd4275ca64e24dc32c3c Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 11 Sep 2017 13:30:13 -0700 Subject: [PATCH 16/21] Automatically generate BigQuery schema details for all counters --- src/core/lib/debug/stats_data_bq_schema.sql | 33 +++++++++++++++++++++ tools/codegen/core/gen_stats_data.py | 6 ++++ 2 files changed, 39 insertions(+) create mode 100644 src/core/lib/debug/stats_data_bq_schema.sql diff --git a/src/core/lib/debug/stats_data_bq_schema.sql b/src/core/lib/debug/stats_data_bq_schema.sql new file mode 100644 index 00000000000..b669555fa0d --- /dev/null +++ b/src/core/lib/debug/stats_data_bq_schema.sql @@ -0,0 +1,33 @@ +client_calls_created_per_iteration:INTEGER, +server_calls_created_per_iteration:INTEGER, +syscall_poll_per_iteration:INTEGER, +syscall_wait_per_iteration:INTEGER, +histogram_slow_lookups_per_iteration:INTEGER, +syscall_write_per_iteration:INTEGER, +syscall_read_per_iteration:INTEGER, +tcp_backup_pollers_created_per_iteration:INTEGER, +tcp_backup_poller_polls_per_iteration:INTEGER, +http2_op_batches_per_iteration:INTEGER, +http2_op_cancel_per_iteration:INTEGER, +http2_op_send_initial_metadata_per_iteration:INTEGER, +http2_op_send_message_per_iteration:INTEGER, +http2_op_send_trailing_metadata_per_iteration:INTEGER, +http2_op_recv_initial_metadata_per_iteration:INTEGER, +http2_op_recv_message_per_iteration:INTEGER, +http2_op_recv_trailing_metadata_per_iteration:INTEGER, +http2_settings_writes_per_iteration:INTEGER, +http2_pings_sent_per_iteration:INTEGER, +http2_writes_begun_per_iteration:INTEGER, +http2_writes_offloaded_per_iteration:INTEGER, +http2_writes_continued_per_iteration:INTEGER, +http2_partial_writes_per_iteration:INTEGER, +combiner_locks_initiated_per_iteration:INTEGER, +combiner_locks_scheduled_items_per_iteration:INTEGER, +combiner_locks_scheduled_final_items_per_iteration:INTEGER, +combiner_locks_offloaded_per_iteration:INTEGER, +executor_scheduled_short_items_per_iteration:INTEGER, +executor_scheduled_long_items_per_iteration:INTEGER, +executor_scheduled_to_self_per_iteration:INTEGER, +executor_wakeup_initiated_per_iteration:INTEGER, +executor_queue_drained_per_iteration:INTEGER, +executor_push_retries_per_iteration:INTEGER diff --git a/tools/codegen/core/gen_stats_data.py b/tools/codegen/core/gen_stats_data.py index 8e4ef594af2..cb01321ed3a 100755 --- a/tools/codegen/core/gen_stats_data.py +++ b/tools/codegen/core/gen_stats_data.py @@ -313,3 +313,9 @@ with open('src/core/lib/debug/stats_data.c', 'w') as C: len(inst_map['Histogram']), ','.join('grpc_stats_table_%d' % x for x in histo_bucket_boundaries)) print >>C, "void (*const grpc_stats_inc_histogram[%d])(grpc_exec_ctx *exec_ctx, int x) = {%s};" % ( len(inst_map['Histogram']), ','.join('grpc_stats_inc_%s' % histogram.name.lower() for histogram in inst_map['Histogram'])) + +with open('src/core/lib/debug/stats_data_bq_schema.sql', 'w') as S: + columns = [] + for counter in inst_map['Counter']: + columns.append(('%s_per_iteration' % counter.name, 'INTEGER')) + print >>S, ',\n'.join('%s:%s' % x for x in columns) From 20104ce596b8ccbe4cd167e245654f56e8d86edc Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Mon, 11 Sep 2017 15:26:57 -0700 Subject: [PATCH 17/21] Add a clamp to remove ubsan failure --- src/core/ext/transport/chttp2/transport/flow_control.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/ext/transport/chttp2/transport/flow_control.c b/src/core/ext/transport/chttp2/transport/flow_control.c index cec99f6fb69..39aa521029b 100644 --- a/src/core/ext/transport/chttp2/transport/flow_control.c +++ b/src/core/ext/transport/chttp2/transport/flow_control.c @@ -483,7 +483,8 @@ grpc_chttp2_flowctl_action grpc_chttp2_flowctl_get_bdp_action( if (grpc_bdp_estimator_get_bw(&tfc->bdp_estimator, &bw_dbl)) { // we target the max of BDP or bandwidth in microseconds. int32_t frame_size = (int32_t)GPR_CLAMP( - GPR_MAX((int32_t)bw_dbl / 1000, bdp), 16384, 16777215); + GPR_MAX((int32_t)GPR_CLAMP(bw_dbl, 0, INT_MAX) / 1000, bdp), 16384, + 16777215); grpc_chttp2_flowctl_urgency frame_size_urgency = delta_is_significant( tfc, frame_size, GRPC_CHTTP2_SETTINGS_MAX_FRAME_SIZE); if (frame_size_urgency != GRPC_CHTTP2_FLOWCTL_NO_ACTION_NEEDED) { From a6721a0979029a2492631e075946556ce7c0eaf2 Mon Sep 17 00:00:00 2001 From: "K.K. Yap" Date: Mon, 11 Sep 2017 16:20:04 -0700 Subject: [PATCH 18/21] Comment that IPv6 Any accepts both IPv4 and IPv6 connections. --- include/grpc++/server_builder.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/include/grpc++/server_builder.h b/include/grpc++/server_builder.h index 21ae70d13ad..bbf45b3e74c 100644 --- a/include/grpc++/server_builder.h +++ b/include/grpc++/server_builder.h @@ -136,8 +136,10 @@ class ServerBuilder { /// It can be invoked multiple times. /// /// \param addr_uri The address to try to bind to the server in URI form. If - /// the scheme name is omitted, "dns:///" is assumed. Valid values include - /// dns:///localhost:1234, / 192.168.1.1:31416, dns:///[::1]:27182, etc.). + /// the scheme name is omitted, "dns:///" is assumed. To bind to any address, + /// please use IPv6 any, i.e., [::]:, which also accepts IPv4 + /// connections. Valid values include dns:///localhost:1234, / + /// 192.168.1.1:31416, dns:///[::1]:27182, etc.). /// \params creds The credentials associated with the server. /// \param selected_port[out] If not `nullptr`, gets populated with the port /// number bound to the \a grpc::Server for the corresponding endpoint after From 29828c58d33fbbc6503784f0d03064ced6a22841 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 12 Sep 2017 16:42:55 +0200 Subject: [PATCH 19/21] differentiate run_tests suite runtime --- tools/run_tests/run_tests_matrix.py | 50 ++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/tools/run_tests/run_tests_matrix.py b/tools/run_tests/run_tests_matrix.py index 00680b02d3c..957e7b569e2 100755 --- a/tools/run_tests/run_tests_matrix.py +++ b/tools/run_tests/run_tests_matrix.py @@ -29,9 +29,11 @@ from python_utils.filter_pull_request_tests import filter_tests _ROOT = os.path.abspath(os.path.join(os.path.dirname(sys.argv[0]), '../..')) os.chdir(_ROOT) +_DEFAULT_RUNTESTS_TIMEOUT = 1*60*60 + # Set the timeout high to allow enough time for sanitizers and pre-building # clang docker. -_RUNTESTS_TIMEOUT = 4*60*60 +_CPP_RUNTESTS_TIMEOUT = 4*60*60 # Number of jobs assigned to each run_tests.py instance _DEFAULT_INNER_JOBS = 2 @@ -51,8 +53,11 @@ def _report_filename_internal_ci(name): def _docker_jobspec(name, runtests_args=[], runtests_envs={}, - inner_jobs=_DEFAULT_INNER_JOBS): + inner_jobs=_DEFAULT_INNER_JOBS, + timeout_seconds=None): """Run a single instance of run_tests.py in a docker container""" + if not timeout_seconds: + timeout_seconds = _DEFAULT_RUNTESTS_TIMEOUT test_job = jobset.JobSpec( cmdline=['python', 'tools/run_tests/run_tests.py', '--use_docker', @@ -62,15 +67,18 @@ def _docker_jobspec(name, runtests_args=[], runtests_envs={}, '--report_suite_name', '%s' % name] + runtests_args, environ=runtests_envs, shortname='run_tests_%s' % name, - timeout_seconds=_RUNTESTS_TIMEOUT) + timeout_seconds=timeout_seconds) return test_job def _workspace_jobspec(name, runtests_args=[], workspace_name=None, - runtests_envs={}, inner_jobs=_DEFAULT_INNER_JOBS): + runtests_envs={}, inner_jobs=_DEFAULT_INNER_JOBS, + timeout_seconds=None): """Run a single instance of run_tests.py in a separate workspace""" if not workspace_name: workspace_name = 'workspace_%s' % name + if not timeout_seconds: + timeout_seconds = _DEFAULT_RUNTESTS_TIMEOUT env = {'WORKSPACE_NAME': workspace_name} env.update(runtests_envs) test_job = jobset.JobSpec( @@ -82,14 +90,15 @@ def _workspace_jobspec(name, runtests_args=[], workspace_name=None, '--report_suite_name', '%s' % name] + runtests_args, environ=env, shortname='run_tests_%s' % name, - timeout_seconds=_RUNTESTS_TIMEOUT) + timeout_seconds=timeout_seconds) return test_job def _generate_jobs(languages, configs, platforms, iomgr_platform = 'native', arch=None, compiler=None, labels=[], extra_args=[], extra_envs={}, - inner_jobs=_DEFAULT_INNER_JOBS): + inner_jobs=_DEFAULT_INNER_JOBS, + timeout_seconds=None): result = [] for language in languages: for platform in platforms: @@ -110,10 +119,12 @@ def _generate_jobs(languages, configs, platforms, iomgr_platform = 'native', runtests_args += extra_args if platform == 'linux': job = _docker_jobspec(name=name, runtests_args=runtests_args, - runtests_envs=extra_envs, inner_jobs=inner_jobs) + runtests_envs=extra_envs, inner_jobs=inner_jobs, + timeout_seconds=timeout_seconds) else: job = _workspace_jobspec(name=name, runtests_args=runtests_args, - runtests_envs=extra_envs, inner_jobs=inner_jobs) + runtests_envs=extra_envs, inner_jobs=inner_jobs, + timeout_seconds=timeout_seconds) job.labels = [platform, config, language, iomgr_platform] + labels result.append(job) @@ -136,7 +147,8 @@ def _create_test_jobs(extra_args=[], inner_jobs=_DEFAULT_INNER_JOBS): platforms=['linux', 'macos', 'windows'], labels=['basictests', 'corelang'], extra_args=extra_args, - inner_jobs=inner_jobs) + inner_jobs=inner_jobs, + timeout_seconds=_CPP_RUNTESTS_TIMEOUT) test_jobs += _generate_jobs(languages=['csharp', 'node', 'python'], configs=['dbg', 'opt'], @@ -151,7 +163,8 @@ def _create_test_jobs(extra_args=[], inner_jobs=_DEFAULT_INNER_JOBS): platforms=['linux', 'macos'], labels=['basictests', 'corelang'], extra_args=extra_args, - inner_jobs=inner_jobs) + inner_jobs=inner_jobs, + timeout_seconds=_CPP_RUNTESTS_TIMEOUT) test_jobs += _generate_jobs(languages=['ruby', 'php'], configs=['dbg', 'opt'], @@ -174,13 +187,15 @@ def _create_test_jobs(extra_args=[], inner_jobs=_DEFAULT_INNER_JOBS): platforms=['linux'], labels=['sanitizers', 'corelang'], extra_args=extra_args, - inner_jobs=inner_jobs) + inner_jobs=inner_jobs, + timeout_seconds=_CPP_RUNTESTS_TIMEOUT) test_jobs += _generate_jobs(languages=['c++'], configs=['asan', 'tsan'], platforms=['linux'], labels=['sanitizers', 'corelang'], extra_args=extra_args, - inner_jobs=inner_jobs) + inner_jobs=inner_jobs, + timeout_seconds=_CPP_RUNTESTS_TIMEOUT) return test_jobs @@ -207,7 +222,8 @@ def _create_portability_test_jobs(extra_args=[], inner_jobs=_DEFAULT_INNER_JOBS) compiler=compiler, labels=['portability', 'corelang'], extra_args=extra_args, - inner_jobs=inner_jobs) + inner_jobs=inner_jobs, + timeout_seconds=_CPP_RUNTESTS_TIMEOUT) # portability C on Windows 64-bit (x86 is the default) test_jobs += _generate_jobs(languages=['c'], @@ -246,10 +262,11 @@ def _create_portability_test_jobs(extra_args=[], inner_jobs=_DEFAULT_INNER_JOBS) configs=['dbg'], platforms=['linux'], labels=['portability', 'corelang'], extra_args=extra_args, - extra_envs={'GRPC_DNS_RESOLVER': 'ares'}) + extra_envs={'GRPC_DNS_RESOLVER': 'ares'}, + timeout_seconds=_CPP_RUNTESTS_TIMEOUT) # TODO(zyc): Turn on this test after adding c-ares support on windows. - # C with the c-ares DNS resolver on Windonws + # C with the c-ares DNS resolver on Windows # test_jobs += _generate_jobs(languages=['c'], # configs=['dbg'], platforms=['windows'], # labels=['portability', 'corelang'], @@ -292,7 +309,8 @@ def _create_portability_test_jobs(extra_args=[], inner_jobs=_DEFAULT_INNER_JOBS) iomgr_platform='uv', labels=['portability', 'corelang'], extra_args=extra_args, - inner_jobs=inner_jobs) + inner_jobs=inner_jobs, + timeout_seconds=_CPP_RUNTESTS_TIMEOUT) test_jobs += _generate_jobs(languages=['node'], configs=['dbg'], From 0852acd8b3700b052676b21b096fc641fc4194aa Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 12 Sep 2017 13:15:03 -0700 Subject: [PATCH 20/21] Revert "Let alarms trigger at end of jobset.py instead of clearing them" --- tools/run_tests/python_utils/jobset.py | 22 ++++++++++++++++------ tools/run_tests/run_performance_tests.py | 8 ++++---- 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/tools/run_tests/python_utils/jobset.py b/tools/run_tests/python_utils/jobset.py index 6151a7276a9..08d652ae3f3 100755 --- a/tools/run_tests/python_utils/jobset.py +++ b/tools/run_tests/python_utils/jobset.py @@ -71,8 +71,10 @@ def platform_string(): if platform_string() == 'windows': pass else: + have_alarm = False def alarm_handler(unused_signum, unused_frame): - pass + global have_alarm + have_alarm = False signal.signal(signal.SIGCHLD, lambda unused_signum, unused_frame: None) signal.signal(signal.SIGALRM, alarm_handler) @@ -365,9 +367,10 @@ class Jobset(object): """Manages one run of jobs.""" def __init__(self, check_cancelled, maxjobs, newline_on_success, travis, - stop_on_failure, add_env, quiet_success, max_time): + stop_on_failure, add_env, quiet_success, max_time, clear_alarms): self._running = set() self._check_cancelled = check_cancelled + self._clear_alarms = clear_alarms self._cancelled = False self._failures = 0 self._completed = 0 @@ -452,7 +455,10 @@ class Jobset(object): if platform_string() == 'windows': time.sleep(0.1) else: - signal.alarm(10) + global have_alarm + if not have_alarm: + have_alarm = True + signal.alarm(10) signal.pause() def cancelled(self): @@ -468,7 +474,10 @@ class Jobset(object): while self._running: if self.cancelled(): pass # poll cancellation self.reap() - if platform_string() != 'windows': + # Clear the alarms when finished to avoid a race condition causing job + # failures. Don't do this when running multi-VM tests because clearing + # the alarms causes the test to stall + if platform_string() != 'windows' and self._clear_alarms: signal.alarm(0) return not self.cancelled() and self._failures == 0 @@ -498,7 +507,8 @@ def run(cmdlines, add_env={}, skip_jobs=False, quiet_success=False, - max_time=-1): + max_time=-1, + clear_alarms=True): if skip_jobs: resultset = {} skipped_job_result = JobResult() @@ -510,7 +520,7 @@ def run(cmdlines, js = Jobset(check_cancelled, maxjobs if maxjobs is not None else _DEFAULT_MAX_JOBS, newline_on_success, travis, stop_on_failure, add_env, - quiet_success, max_time) + quiet_success, max_time, clear_alarms) for cmdline, remaining in tag_remaining(cmdlines): if not js.start(cmdline): break diff --git a/tools/run_tests/run_performance_tests.py b/tools/run_tests/run_performance_tests.py index 3bfd736c51a..9b20fae78fd 100755 --- a/tools/run_tests/run_performance_tests.py +++ b/tools/run_tests/run_performance_tests.py @@ -183,7 +183,7 @@ def archive_repo(languages): jobset.message('START', 'Archiving local repository.', do_newline=True) num_failures, _ = jobset.run( - [archive_job], newline_on_success=True, maxjobs=1) + [archive_job], newline_on_success=True, maxjobs=1, clear_alarms=False) if num_failures == 0: jobset.message('SUCCESS', 'Archive with local repository created successfully.', @@ -215,7 +215,7 @@ def prepare_remote_hosts(hosts, prepare_local=False): timeout_seconds=prepare_timeout)) jobset.message('START', 'Preparing hosts.', do_newline=True) num_failures, _ = jobset.run( - prepare_jobs, newline_on_success=True, maxjobs=10) + prepare_jobs, newline_on_success=True, maxjobs=10, clear_alarms=False) if num_failures == 0: jobset.message('SUCCESS', 'Prepare step completed successfully.', @@ -248,7 +248,7 @@ def build_on_remote_hosts(hosts, languages=scenario_config.LANGUAGES.keys(), bui timeout_seconds=build_timeout)) jobset.message('START', 'Building.', do_newline=True) num_failures, _ = jobset.run( - build_jobs, newline_on_success=True, maxjobs=10) + build_jobs, newline_on_success=True, maxjobs=10, clear_alarms=False) if num_failures == 0: jobset.message('SUCCESS', 'Built successfully.', @@ -414,7 +414,7 @@ def run_collect_perf_profile_jobs(hosts_and_base_names, scenario_name, flame_gra perf_report_jobs.append(perf_report_processor_job(host, perf_base_name, output_filename, flame_graph_reports)) jobset.message('START', 'Collecting perf reports from qps workers', do_newline=True) - failures, _ = jobset.run(perf_report_jobs, newline_on_success=True, maxjobs=1) + failures, _ = jobset.run(perf_report_jobs, newline_on_success=True, maxjobs=1, clear_alarms=False) jobset.message('END', 'Collecting perf reports from qps workers', do_newline=True) return failures From 40c346ced94ecdb5a5d818d160f86e1be1c77046 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Tue, 12 Sep 2017 22:39:30 +0200 Subject: [PATCH 21/21] Adding --base to generate_projects.py --- tools/buildgen/generate_projects.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/buildgen/generate_projects.py b/tools/buildgen/generate_projects.py index f885b266972..559ea1624bc 100755 --- a/tools/buildgen/generate_projects.py +++ b/tools/buildgen/generate_projects.py @@ -34,6 +34,7 @@ argp.add_argument('build_files', nargs='+', default=[]) argp.add_argument('--templates', nargs='+', default=[]) argp.add_argument('--output_merged', default=None, type=str) argp.add_argument('--jobs', '-j', default=multiprocessing.cpu_count(), type=int) +argp.add_argument('--base', default='.', type=str) args = argp.parse_args() json = args.build_files @@ -69,7 +70,7 @@ jobs = [] for template in reversed(sorted(templates)): root, f = os.path.split(template) if os.path.splitext(f)[1] == '.template': - out_dir = '.' + root[len('templates'):] + out_dir = args.base + root[len('templates'):] out = out_dir + '/' + os.path.splitext(f)[0] if not os.path.exists(out_dir): os.makedirs(out_dir)