From ce8f977593dc111e9c1516accbbb88913812090f Mon Sep 17 00:00:00 2001 From: Robbie Shade <rjshade@google.com> Date: Mon, 1 Aug 2016 16:06:00 -0400 Subject: [PATCH 01/83] Call orphan_cb before FD shutdown --- src/core/lib/iomgr/udp_server.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/core/lib/iomgr/udp_server.c b/src/core/lib/iomgr/udp_server.c index 48032412a2e..12e929fa6a8 100644 --- a/src/core/lib/iomgr/udp_server.c +++ b/src/core/lib/iomgr/udp_server.c @@ -171,6 +171,8 @@ static void deactivated_all_ports(grpc_exec_ctx *exec_ctx, grpc_udp_server *s) { sp->destroyed_closure.cb = destroyed_port; sp->destroyed_closure.cb_arg = s; + /* Call the orphan_cb to signal that the FD is about to be closed and + * should no longer be used. */ GPR_ASSERT(sp->orphan_cb); sp->orphan_cb(sp->emfd); @@ -197,6 +199,11 @@ void grpc_udp_server_destroy(grpc_exec_ctx *exec_ctx, grpc_udp_server *s, /* shutdown all fd's */ if (s->active_ports) { for (i = 0; i < s->nports; i++) { + /* Call the orphan_cb to signal that the FD is about to be closed and + * should no longer be used. */ + GPR_ASSERT(sp->orphan_cb); + sp->orphan_cb(sp->emfd); + grpc_fd_shutdown(exec_ctx, s->ports[i].emfd); } gpr_mu_unlock(&s->mu); From 26f90888d6d98322974a75dc8938dc070edea4c5 Mon Sep 17 00:00:00 2001 From: Robbie Shade <rjshade@google.com> Date: Tue, 2 Aug 2016 15:26:00 -0400 Subject: [PATCH 02/83] Remove GRPC_NEED_UDP ifdefs --- src/core/lib/iomgr/udp_server.c | 2 -- test/core/iomgr/udp_server_test.c | 8 -------- 2 files changed, 10 deletions(-) diff --git a/src/core/lib/iomgr/udp_server.c b/src/core/lib/iomgr/udp_server.c index 48032412a2e..f68288169b7 100644 --- a/src/core/lib/iomgr/udp_server.c +++ b/src/core/lib/iomgr/udp_server.c @@ -38,7 +38,6 @@ #include <grpc/support/port_platform.h> -#ifdef GRPC_NEED_UDP #ifdef GPR_POSIX_SOCKET #include "src/core/lib/iomgr/udp_server.h" @@ -439,4 +438,3 @@ void grpc_udp_server_start(grpc_exec_ctx *exec_ctx, grpc_udp_server *s, } #endif -#endif diff --git a/test/core/iomgr/udp_server_test.c b/test/core/iomgr/udp_server_test.c index a959a7e07fa..2a304275043 100644 --- a/test/core/iomgr/udp_server_test.c +++ b/test/core/iomgr/udp_server_test.c @@ -48,8 +48,6 @@ #include "src/core/lib/iomgr/iomgr.h" #include "test/core/util/test_config.h" -#ifdef GRPC_NEED_UDP - #define LOG_TEST(x) gpr_log(GPR_INFO, "%s", #x) static grpc_pollset *g_pollset; @@ -229,9 +227,3 @@ int main(int argc, char **argv) { grpc_iomgr_shutdown(); return 0; } - -#else - -int main(int argc, char **argv) { return 0; } - -#endif From cb78540024d485b69a6477c69ccf03048f90cc9a Mon Sep 17 00:00:00 2001 From: tania-m <tania.mathern@gmail.com> Date: Thu, 8 Sep 2016 20:28:26 +0200 Subject: [PATCH 03/83] Update greeter_client.js Switched declaration order --- examples/node/static_codegen/greeter_client.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/node/static_codegen/greeter_client.js b/examples/node/static_codegen/greeter_client.js index da80cf34d8e..9b93e003a5c 100644 --- a/examples/node/static_codegen/greeter_client.js +++ b/examples/node/static_codegen/greeter_client.js @@ -39,13 +39,13 @@ var grpc = require('grpc'); function main() { var client = new services.GreeterClient('localhost:50051', grpc.credentials.createInsecure()); + var request = new messages.HelloRequest(); var user; if (process.argv.length >= 3) { user = process.argv[2]; } else { user = 'world'; } - var request = new messages.HelloRequest(); request.setName(user); client.sayHello(request, function(err, response) { console.log('Greeting:', response.getMessage()); From c809be2af5dab9041c310cefc68f40977409f85e Mon Sep 17 00:00:00 2001 From: bithavoc <im@bithavoc.io> Date: Fri, 16 Sep 2016 10:36:58 -0500 Subject: [PATCH 04/83] Fix ruby readme link to grpc quickstart --- src/ruby/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ruby/README.md b/src/ruby/README.md index 31795754866..67e94dd3540 100644 --- a/src/ruby/README.md +++ b/src/ruby/README.md @@ -73,5 +73,5 @@ Directory structure is the layout for [ruby extensions][] [ruby extensions]:http://guides.rubygems.org/gems-with-extensions/ [rubydoc]: http://www.rubydoc.info/gems/grpc -[grpc.io]: http://www.grpc.io/docs/installation/ruby.html +[grpc.io]: http://www.grpc.io/docs/quickstart/ruby.html [Debian jessie-backports]:http://backports.debian.org/Instructions/ From 9c79e8de96c168c40691bebf8db18c6acd3e813b Mon Sep 17 00:00:00 2001 From: Jan Tattermusch <jtattermusch@google.com> Date: Mon, 19 Sep 2016 14:33:18 +0200 Subject: [PATCH 05/83] add matrix run script --- tools/run_tests/run_tests_in_workspace.sh | 44 ++++ tools/run_tests/run_tests_matrix.py | 239 ++++++++++++++++++++++ 2 files changed, 283 insertions(+) create mode 100755 tools/run_tests/run_tests_in_workspace.sh create mode 100755 tools/run_tests/run_tests_matrix.py diff --git a/tools/run_tests/run_tests_in_workspace.sh b/tools/run_tests/run_tests_in_workspace.sh new file mode 100755 index 00000000000..0e7604dbc55 --- /dev/null +++ b/tools/run_tests/run_tests_in_workspace.sh @@ -0,0 +1,44 @@ +#!/bin/bash +# Copyright 2015, Google Inc. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +# +# Create a workspace in a subdirectory to allow running multiple builds in isolation. +# WORKSPACE_NAME env variable needs to contain name of the workspace to create. +# All cmdline args will be passed to run_tests.py script (executed in the +# newly created workspace) +set -ex + +cd $(dirname $0)/../.. + +rm -rf "${WORKSPACE_NAME}" +git clone --recursive . "${WORKSPACE_NAME}" + +echo "Running run_tests.py in workspace ${WORKSPACE_NAME}" +"${WORKSPACE_NAME}/tools/run_tests/run_tests.py" $@ + diff --git a/tools/run_tests/run_tests_matrix.py b/tools/run_tests/run_tests_matrix.py new file mode 100755 index 00000000000..2344e2f9fd8 --- /dev/null +++ b/tools/run_tests/run_tests_matrix.py @@ -0,0 +1,239 @@ +#!/usr/bin/env python2.7 +# Copyright 2015, Google Inc. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +"""Run test matrix.""" + +import argparse +import jobset +import os +import report_utils +import sys + +_ROOT = os.path.abspath(os.path.join(os.path.dirname(sys.argv[0]), '../..')) +os.chdir(_ROOT) + +# TODO(jtattermusch): this is not going to be enough for sanitizers. +_RUNTESTS_TIMEOUT = 30*60 + + +def _docker_jobspec(name, runtests_args=[]): + """Run a single instance of run_tests.py in a docker container""" + # TODO: fix copying report files from inside docker.... + test_job = jobset.JobSpec( + cmdline=['python', 'tools/run_tests/run_tests.py', + '--use_docker', + '-t', + '-j', '3', + '-x', 'report_%s.xml' % name] + runtests_args, + shortname='run_tests_%s' % name, + timeout_seconds=_RUNTESTS_TIMEOUT) + return test_job + + +def _workspace_jobspec(name, runtests_args=[], workspace_name=None): + """Run a single instance of run_tests.py in a separate workspace""" + env = {'WORKSPACE_NAME': workspace_name} + test_job = jobset.JobSpec( + cmdline=['tools/run_tests/run_tests_in_workspace.sh', + '-t', + '-j', '3', + '-x', '../report_%s.xml' % name] + runtests_args, + environ=env, + shortname='run_tests_%s' % name, + timeout_seconds=_RUNTESTS_TIMEOUT) + return test_job + + +def _generate_jobs(languages, configs, platforms, + arch=None, compiler=None, + labels=[]): + result = [] + for language in languages: + for platform in platforms: + for config in configs: + name = '%s_%s_%s' % (language, platform, config) + runtests_args = ['-l', language, + '-c', config] + if arch or compiler: + name += '_%s_%s' % (arch, compiler) + runtests_args += ['--arch', arch, + '--compiler', compiler] + + if platform == 'linux': + job = _docker_jobspec(name=name, runtests_args=runtests_args) + else: + job = _workspace_jobspec(name=name, runtests_args=runtests_args) + + job.labels = [platform, config, language] + labels + result.append(job) + return result + + +def _create_test_jobs(): + test_jobs = [] + # supported on linux only + test_jobs += _generate_jobs(languages=['sanity', 'php7'], + configs=['dbg', 'opt'], + platforms=['linux'], + labels=['basictests']) + + # supported on all platforms. + test_jobs += _generate_jobs(languages=['c', 'csharp', 'node', 'python'], + configs=['dbg', 'opt'], + platforms=['linux', 'macos', 'windows'], + labels=['basictests']) + + # supported on linux and mac. + test_jobs += _generate_jobs(languages=['c++', 'ruby', 'php'], + configs=['dbg', 'opt'], + platforms=['linux', 'macos'], + labels=['basictests']) + + # supported on mac only. + test_jobs += _generate_jobs(languages=['objc'], + configs=['dbg', 'opt'], + platforms=['macos'], + labels=['basictests']) + + # sanitizers + test_jobs += _generate_jobs(languages=['c'], + configs=['msan', 'asan', 'tsan'], + platforms=['linux'], + labels=['sanitizers']) + test_jobs += _generate_jobs(languages=['c++'], + configs=['asan', 'tsan'], + platforms=['linux'], + labels=['sanitizers']) + return test_jobs + + +def _create_portability_test_jobs(): + test_jobs = [] + # portability C x86 + test_jobs += _generate_jobs(languages=['c'], + configs=['dbg'], + platforms=['linux'], + arch='x86', + compiler='default', + labels=['portability']) + + # portability C and C++ on x64 + for compiler in ['gcc4.4', 'gcc4.6', 'gcc5.3', + 'clang3.5', 'clang3.6', 'clang3.7']: + test_jobs += _generate_jobs(languages=['c', 'c++'], + configs=['dbg'], + platforms=['linux'], + arch='x64', + compiler=compiler, + labels=['portability']) + + # portability C on Windows + for arch in ['x86', 'x64']: + for compiler in ['vs2013', 'vs2015']: + test_jobs += _generate_jobs(languages=['c'], + configs=['dbg'], + platforms=['windows'], + arch=arch, + compiler=compiler, + labels=['portability']) + + test_jobs += _generate_jobs(languages=['python'], + configs=['dbg'], + platforms=['linux'], + arch='default', + compiler='python3.4', + labels=['portability']) + + test_jobs += _generate_jobs(languages=['csharp'], + configs=['dbg'], + platforms=['linux'], + arch='default', + compiler='coreclr', + labels=['portability']) + + for compiler in ['node5', 'node6', 'node0.12']: + test_jobs += _generate_jobs(languages=['node'], + configs=['dbg'], + platforms=['linux'], + arch='default', + compiler=compiler, + labels=['portability']) + return test_jobs + + +all_jobs = _create_test_jobs() + _create_portability_test_jobs() + +all_labels = set() +for job in all_jobs: + for label in job.labels: + all_labels.add(label) + +argp = argparse.ArgumentParser(description='Run a matrix of run_tests.py tests.') +argp.add_argument('-f', '--filter', + choices=sorted(all_labels), + nargs='+', + default=[], + help='Filter targets to run by label with AND semantics.') +args = argp.parse_args() + +jobs = [] +for job in all_jobs: + if not args.filter or all(filter in job.labels for filter in args.filter): + jobs.append(job) + +if not jobs: + jobset.message('FAILED', 'No test suites match given criteria.', + do_newline=True) + sys.exit(1) + +print('IMPORTANT: The changes you are testing need to be locally committed') +print('because only the committed changes in the current branch will be') +print('copied to the docker environment or into subworkspaces.') + +print +print 'Will run these tests:' +for job in jobs: + print ' %s' % job.shortname +print + +jobset.message('START', 'Running test matrix.', do_newline=True) +num_failures, resultset = jobset.run(jobs, + newline_on_success=True, + travis=True, + maxjobs=2) +report_utils.render_junit_xml_report(resultset, 'report.xml') + +if num_failures == 0: + jobset.message('SUCCESS', 'All run_tests.py instance finished successfully.', + do_newline=True) +else: + jobset.message('FAILED', 'Some run_tests.py instance have failed.', + do_newline=True) + sys.exit(1) From ba5196acdda6a72f689d5c793f2a2526423e1b55 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch <jtattermusch@google.com> Date: Mon, 19 Sep 2016 18:20:37 +0200 Subject: [PATCH 06/83] improve report collection --- tools/run_tests/dockerize/docker_run_tests.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/run_tests/dockerize/docker_run_tests.sh b/tools/run_tests/dockerize/docker_run_tests.sh index 8c6143d24f9..ef02d266253 100755 --- a/tools/run_tests/dockerize/docker_run_tests.sh +++ b/tools/run_tests/dockerize/docker_run_tests.sh @@ -63,6 +63,7 @@ echo '</body></html>' >> index.html cd .. zip -r reports.zip reports -find . -name report.xml | xargs zip reports.zip +find . -name report.xml | xargs -r zip reports.zip +find . -name 'report_*.xml' | xargs -r zip reports.zip exit $exit_code From a1906d5ea7b445c465ce368b7f8c0c8f396279a3 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch <jtattermusch@google.com> Date: Mon, 19 Sep 2016 18:37:17 +0200 Subject: [PATCH 07/83] run matrix improvements --- .../dockerize/build_docker_and_run_tests.sh | 12 ++++++------ tools/run_tests/run_tests_in_workspace.sh | 2 ++ tools/run_tests/run_tests_matrix.py | 17 ++++++++++++++--- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/tools/run_tests/dockerize/build_docker_and_run_tests.sh b/tools/run_tests/dockerize/build_docker_and_run_tests.sh index c2ea6f2c6eb..b4b172ddef6 100755 --- a/tools/run_tests/dockerize/build_docker_and_run_tests.sh +++ b/tools/run_tests/dockerize/build_docker_and_run_tests.sh @@ -44,9 +44,6 @@ mkdir -p /tmp/ccache # its cache location now that --download-cache is deprecated). mkdir -p /tmp/xdg-cache-home -# Create a local branch so the child Docker script won't complain -git branch -f jenkins-docker - # Inputs # DOCKERFILE_DIR - Directory in which Dockerfile file is located. # DOCKER_RUN_SCRIPT - Script to run under docker (relative to grpc repo root) @@ -86,9 +83,12 @@ docker run \ $DOCKER_IMAGE_NAME \ bash -l "/var/local/jenkins/grpc/$DOCKER_RUN_SCRIPT" || DOCKER_FAILED="true" -docker cp "$CONTAINER_NAME:/var/local/git/grpc/reports.zip" $git_root || true -unzip -o $git_root/reports.zip -d $git_root || true -rm -f reports.zip +# use unique name for reports.zip to prevent clash between concurrent +# run_tests.py runs +TEMP_REPORTS_ZIP=`mktemp` +docker cp "$CONTAINER_NAME:/var/local/git/grpc/reports.zip" ${TEMP_REPORTS_ZIP} || true +unzip -o ${TEMP_REPORTS_ZIP} -d $git_root || true +rm -f ${TEMP_REPORTS_ZIP} # remove the container, possibly killing it first docker rm -f $CONTAINER_NAME || true diff --git a/tools/run_tests/run_tests_in_workspace.sh b/tools/run_tests/run_tests_in_workspace.sh index 0e7604dbc55..b0c9ad05f76 100755 --- a/tools/run_tests/run_tests_in_workspace.sh +++ b/tools/run_tests/run_tests_in_workspace.sh @@ -37,6 +37,8 @@ set -ex cd $(dirname $0)/../.. rm -rf "${WORKSPACE_NAME}" +# TODO(jtattermusch): clone --recursive fetches the submodules from github. +# Try avoiding that to save time and network capacity. git clone --recursive . "${WORKSPACE_NAME}" echo "Running run_tests.py in workspace ${WORKSPACE_NAME}" diff --git a/tools/run_tests/run_tests_matrix.py b/tools/run_tests/run_tests_matrix.py index 2344e2f9fd8..bc2e37abd89 100755 --- a/tools/run_tests/run_tests_matrix.py +++ b/tools/run_tests/run_tests_matrix.py @@ -32,6 +32,7 @@ import argparse import jobset +import multiprocessing import os import report_utils import sys @@ -40,8 +41,12 @@ _ROOT = os.path.abspath(os.path.join(os.path.dirname(sys.argv[0]), '../..')) os.chdir(_ROOT) # TODO(jtattermusch): this is not going to be enough for sanitizers. +# TODO(jtattermusch): this is not going to be enough for rebuilding clang docker. _RUNTESTS_TIMEOUT = 30*60 +# Number of jobs assigned to each run_tests.py instance +_INNER_JOBS = 2 + def _docker_jobspec(name, runtests_args=[]): """Run a single instance of run_tests.py in a docker container""" @@ -50,7 +55,7 @@ def _docker_jobspec(name, runtests_args=[]): cmdline=['python', 'tools/run_tests/run_tests.py', '--use_docker', '-t', - '-j', '3', + '-j', str(_INNER_JOBS), '-x', 'report_%s.xml' % name] + runtests_args, shortname='run_tests_%s' % name, timeout_seconds=_RUNTESTS_TIMEOUT) @@ -59,11 +64,13 @@ def _docker_jobspec(name, runtests_args=[]): def _workspace_jobspec(name, runtests_args=[], workspace_name=None): """Run a single instance of run_tests.py in a separate workspace""" + if not workspace_name: + workspace_name = 'workspace_%s' % name env = {'WORKSPACE_NAME': workspace_name} test_job = jobset.JobSpec( cmdline=['tools/run_tests/run_tests_in_workspace.sh', '-t', - '-j', '3', + '-j', str(_INNER_JOBS), '-x', '../report_%s.xml' % name] + runtests_args, environ=env, shortname='run_tests_%s' % name, @@ -196,6 +203,10 @@ for job in all_jobs: all_labels.add(label) argp = argparse.ArgumentParser(description='Run a matrix of run_tests.py tests.') +argp.add_argument('-j', '--jobs', + default=multiprocessing.cpu_count()/_INNER_JOBS, + type=int, + help='Number of concurrent run_tests.py instances.') argp.add_argument('-f', '--filter', choices=sorted(all_labels), nargs='+', @@ -227,7 +238,7 @@ jobset.message('START', 'Running test matrix.', do_newline=True) num_failures, resultset = jobset.run(jobs, newline_on_success=True, travis=True, - maxjobs=2) + maxjobs=args.jobs) report_utils.render_junit_xml_report(resultset, 'report.xml') if num_failures == 0: From 060eb8794a9064fd046952c16dba096ce364eeac Mon Sep 17 00:00:00 2001 From: Jan Tattermusch <jtattermusch@google.com> Date: Tue, 20 Sep 2016 16:06:13 +0200 Subject: [PATCH 08/83] address some TODOs --- tools/run_tests/run_tests_matrix.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tools/run_tests/run_tests_matrix.py b/tools/run_tests/run_tests_matrix.py index bc2e37abd89..06ab02e32fb 100755 --- a/tools/run_tests/run_tests_matrix.py +++ b/tools/run_tests/run_tests_matrix.py @@ -40,9 +40,9 @@ import sys _ROOT = os.path.abspath(os.path.join(os.path.dirname(sys.argv[0]), '../..')) os.chdir(_ROOT) -# TODO(jtattermusch): this is not going to be enough for sanitizers. -# TODO(jtattermusch): this is not going to be enough for rebuilding clang docker. -_RUNTESTS_TIMEOUT = 30*60 +# Set the timeout high to allow enough time for sanitizers and pre-building +# clang docker. +_RUNTESTS_TIMEOUT = 2*60*60 # Number of jobs assigned to each run_tests.py instance _INNER_JOBS = 2 @@ -50,7 +50,6 @@ _INNER_JOBS = 2 def _docker_jobspec(name, runtests_args=[]): """Run a single instance of run_tests.py in a docker container""" - # TODO: fix copying report files from inside docker.... test_job = jobset.JobSpec( cmdline=['python', 'tools/run_tests/run_tests.py', '--use_docker', @@ -203,6 +202,8 @@ for job in all_jobs: all_labels.add(label) argp = argparse.ArgumentParser(description='Run a matrix of run_tests.py tests.') +# TODO(jtattermusch): allow running tests with --build_only flag +# TODO(jtattermusch): allow running tests with --force_default_poller flag. argp.add_argument('-j', '--jobs', default=multiprocessing.cpu_count()/_INNER_JOBS, type=int, From 6d7c6ef332376def2d0666e4f721ae6fba7176a3 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch <jtattermusch@google.com> Date: Thu, 22 Sep 2016 13:40:48 +0200 Subject: [PATCH 09/83] support passing extra args to run_tests.py --- tools/run_tests/run_tests_matrix.py | 90 +++++++++++++++++++---------- 1 file changed, 59 insertions(+), 31 deletions(-) diff --git a/tools/run_tests/run_tests_matrix.py b/tools/run_tests/run_tests_matrix.py index 06ab02e32fb..ae7fdd84f9b 100755 --- a/tools/run_tests/run_tests_matrix.py +++ b/tools/run_tests/run_tests_matrix.py @@ -79,7 +79,7 @@ def _workspace_jobspec(name, runtests_args=[], workspace_name=None): def _generate_jobs(languages, configs, platforms, arch=None, compiler=None, - labels=[]): + labels=[], extra_args=[]): result = [] for language in languages: for platform in platforms: @@ -92,6 +92,7 @@ def _generate_jobs(languages, configs, platforms, runtests_args += ['--arch', arch, '--compiler', compiler] + runtests_args += extra_args if platform == 'linux': job = _docker_jobspec(name=name, runtests_args=runtests_args) else: @@ -102,45 +103,51 @@ def _generate_jobs(languages, configs, platforms, return result -def _create_test_jobs(): +def _create_test_jobs(extra_args=[]): test_jobs = [] # supported on linux only test_jobs += _generate_jobs(languages=['sanity', 'php7'], configs=['dbg', 'opt'], platforms=['linux'], - labels=['basictests']) + labels=['basictests'], + extra_args=extra_args) # supported on all platforms. test_jobs += _generate_jobs(languages=['c', 'csharp', 'node', 'python'], - configs=['dbg', 'opt'], - platforms=['linux', 'macos', 'windows'], - labels=['basictests']) + configs=['dbg', 'opt'], + platforms=['linux', 'macos', 'windows'], + labels=['basictests'], + extra_args=extra_args) # supported on linux and mac. test_jobs += _generate_jobs(languages=['c++', 'ruby', 'php'], - configs=['dbg', 'opt'], - platforms=['linux', 'macos'], - labels=['basictests']) + configs=['dbg', 'opt'], + platforms=['linux', 'macos'], + labels=['basictests'], + extra_args=extra_args) # supported on mac only. test_jobs += _generate_jobs(languages=['objc'], configs=['dbg', 'opt'], platforms=['macos'], - labels=['basictests']) + labels=['basictests'], + extra_args=extra_args) # sanitizers test_jobs += _generate_jobs(languages=['c'], configs=['msan', 'asan', 'tsan'], platforms=['linux'], - labels=['sanitizers']) + labels=['sanitizers'], + extra_args=extra_args) test_jobs += _generate_jobs(languages=['c++'], configs=['asan', 'tsan'], platforms=['linux'], - labels=['sanitizers']) + labels=['sanitizers'], + extra_args=extra_args) return test_jobs -def _create_portability_test_jobs(): +def _create_portability_test_jobs(extra_args=[]): test_jobs = [] # portability C x86 test_jobs += _generate_jobs(languages=['c'], @@ -148,7 +155,8 @@ def _create_portability_test_jobs(): platforms=['linux'], arch='x86', compiler='default', - labels=['portability']) + labels=['portability'], + extra_args=extra_args) # portability C and C++ on x64 for compiler in ['gcc4.4', 'gcc4.6', 'gcc5.3', @@ -158,7 +166,8 @@ def _create_portability_test_jobs(): platforms=['linux'], arch='x64', compiler=compiler, - labels=['portability']) + labels=['portability'], + extra_args=extra_args) # portability C on Windows for arch in ['x86', 'x64']: @@ -168,53 +177,72 @@ def _create_portability_test_jobs(): platforms=['windows'], arch=arch, compiler=compiler, - labels=['portability']) + labels=['portability'], + extra_args=extra_args) test_jobs += _generate_jobs(languages=['python'], configs=['dbg'], platforms=['linux'], arch='default', compiler='python3.4', - labels=['portability']) + labels=['portability'], + extra_args=extra_args) test_jobs += _generate_jobs(languages=['csharp'], configs=['dbg'], platforms=['linux'], arch='default', compiler='coreclr', - labels=['portability']) + labels=['portability'], + extra_args=extra_args) for compiler in ['node5', 'node6', 'node0.12']: test_jobs += _generate_jobs(languages=['node'], - configs=['dbg'], - platforms=['linux'], - arch='default', - compiler=compiler, - labels=['portability']) + configs=['dbg'], + platforms=['linux'], + arch='default', + compiler=compiler, + labels=['portability'], + extra_args=extra_args) return test_jobs -all_jobs = _create_test_jobs() + _create_portability_test_jobs() +def _allowed_labels(): + """Returns a list of existing job labels.""" + all_labels = set() + for job in _create_test_jobs() + _create_portability_test_jobs(): + for label in job.labels: + all_labels.add(label) + return sorted(all_labels) -all_labels = set() -for job in all_jobs: - for label in job.labels: - all_labels.add(label) argp = argparse.ArgumentParser(description='Run a matrix of run_tests.py tests.') -# TODO(jtattermusch): allow running tests with --build_only flag -# TODO(jtattermusch): allow running tests with --force_default_poller flag. argp.add_argument('-j', '--jobs', default=multiprocessing.cpu_count()/_INNER_JOBS, type=int, help='Number of concurrent run_tests.py instances.') argp.add_argument('-f', '--filter', - choices=sorted(all_labels), + choices=_allowed_labels(), nargs='+', default=[], help='Filter targets to run by label with AND semantics.') +argp.add_argument('--build_only', + default=False, + action='store_const', + const=True, + help='Pass --build_only flag to run_tests.py instances.') +argp.add_argument('--force_default_poller', default=False, action='store_const', const=True, + help='Pass --force_default_poller to run_tests.py instances.') args = argp.parse_args() +extra_args = [] +if args.build_only: + extra_args.append('--build_only') +if args.force_default_poller: + extra_args.append('--force_default_poller') + +all_jobs = _create_test_jobs(extra_args=extra_args) + _create_portability_test_jobs(extra_args=extra_args) + jobs = [] for job in all_jobs: if not args.filter or all(filter in job.labels for filter in args.filter): From 1c037a30441466ed06d9e11c7e56e749afc99bed Mon Sep 17 00:00:00 2001 From: Jan Tattermusch <jtattermusch@google.com> Date: Thu, 22 Sep 2016 14:28:07 +0200 Subject: [PATCH 10/83] node6 portability test not yet implemented --- tools/run_tests/run_tests_matrix.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/run_tests/run_tests_matrix.py b/tools/run_tests/run_tests_matrix.py index ae7fdd84f9b..d87632d8891 100755 --- a/tools/run_tests/run_tests_matrix.py +++ b/tools/run_tests/run_tests_matrix.py @@ -196,7 +196,7 @@ def _create_portability_test_jobs(extra_args=[]): labels=['portability'], extra_args=extra_args) - for compiler in ['node5', 'node6', 'node0.12']: + for compiler in ['node5', 'node0.12']: test_jobs += _generate_jobs(languages=['node'], configs=['dbg'], platforms=['linux'], From c1e44906f87fbd6552a5ed3cc372cce1ec170cbb Mon Sep 17 00:00:00 2001 From: Jan Tattermusch <jtattermusch@google.com> Date: Thu, 22 Sep 2016 14:31:10 +0200 Subject: [PATCH 11/83] support --dry_run flag --- tools/run_tests/run_tests_matrix.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tools/run_tests/run_tests_matrix.py b/tools/run_tests/run_tests_matrix.py index d87632d8891..a31b0dd9177 100755 --- a/tools/run_tests/run_tests_matrix.py +++ b/tools/run_tests/run_tests_matrix.py @@ -233,6 +233,11 @@ argp.add_argument('--build_only', help='Pass --build_only flag to run_tests.py instances.') argp.add_argument('--force_default_poller', default=False, action='store_const', const=True, help='Pass --force_default_poller to run_tests.py instances.') +argp.add_argument('--dry_run', + default=False, + action='store_const', + const=True, + help='Only print what would be run.') args = argp.parse_args() extra_args = [] @@ -263,6 +268,10 @@ for job in jobs: print ' %s' % job.shortname print +if args.dry_run: + print '--dry_run was used, exiting' + sys.exit(1) + jobset.message('START', 'Running test matrix.', do_newline=True) num_failures, resultset = jobset.run(jobs, newline_on_success=True, From 7b9c21ab2c467df62ab3c871e6544cfb70a23a44 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch <jtattermusch@google.com> Date: Thu, 22 Sep 2016 14:44:27 +0200 Subject: [PATCH 12/83] better --dry_run printouts --- tools/run_tests/run_tests_matrix.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/run_tests/run_tests_matrix.py b/tools/run_tests/run_tests_matrix.py index a31b0dd9177..d5f90478257 100755 --- a/tools/run_tests/run_tests_matrix.py +++ b/tools/run_tests/run_tests_matrix.py @@ -265,7 +265,10 @@ print('copied to the docker environment or into subworkspaces.') print print 'Will run these tests:' for job in jobs: - print ' %s' % job.shortname + if args.dry_run: + print ' %s: "%s"' % (job.shortname, ' '.join(job.cmdline)) + else: + print ' %s' % job.shortname print if args.dry_run: From 27170357abbb9609b9f0ac47c1926a7eb383ca67 Mon Sep 17 00:00:00 2001 From: Yuchen Zeng <zyc@google.com> Date: Fri, 23 Sep 2016 14:14:18 -0700 Subject: [PATCH 13/83] Improve ProtoReflectionDescriptorDatabase --- build.yaml | 1 - test/cpp/util/proto_reflection_descriptor_database.cc | 9 ++++++--- test/cpp/util/proto_reflection_descriptor_database.h | 2 +- tools/run_tests/sources_and_headers.json | 3 +-- vsprojects/vcxproj/grpc_cli_libs/grpc_cli_libs.vcxproj | 3 --- 5 files changed, 8 insertions(+), 10 deletions(-) diff --git a/build.yaml b/build.yaml index 3701c0d8149..5e7b5bbe00b 100644 --- a/build.yaml +++ b/build.yaml @@ -1094,7 +1094,6 @@ libs: deps: - grpc++_reflection - grpc++ - - grpc++_test_config - name: grpc_plugin_support build: protoc language: c++ diff --git a/test/cpp/util/proto_reflection_descriptor_database.cc b/test/cpp/util/proto_reflection_descriptor_database.cc index f0d14c686a3..ae633ea7f4e 100644 --- a/test/cpp/util/proto_reflection_descriptor_database.cc +++ b/test/cpp/util/proto_reflection_descriptor_database.cc @@ -314,13 +314,16 @@ ProtoReflectionDescriptorDatabase::GetStream() { return stream_; } -void ProtoReflectionDescriptorDatabase::DoOneRequest( +bool ProtoReflectionDescriptorDatabase::DoOneRequest( const ServerReflectionRequest& request, ServerReflectionResponse& response) { + bool request_succeed = false; stream_mutex_.lock(); - GetStream()->Write(request); - GetStream()->Read(&response); + if (GetStream()->Write(request) && GetStream()->Read(&response)) { + request_succeed = true; + } stream_mutex_.unlock(); + return request_succeed; } } // namespace grpc diff --git a/test/cpp/util/proto_reflection_descriptor_database.h b/test/cpp/util/proto_reflection_descriptor_database.h index 0e69696d5fb..54114f10814 100644 --- a/test/cpp/util/proto_reflection_descriptor_database.h +++ b/test/cpp/util/proto_reflection_descriptor_database.h @@ -111,7 +111,7 @@ class ProtoReflectionDescriptorDatabase : public protobuf::DescriptorDatabase { const std::shared_ptr<ClientStream> GetStream(); - void DoOneRequest( + bool DoOneRequest( const grpc::reflection::v1alpha::ServerReflectionRequest& request, grpc::reflection::v1alpha::ServerReflectionResponse& response); diff --git a/tools/run_tests/sources_and_headers.json b/tools/run_tests/sources_and_headers.json index 1b8f19a298d..38357e193d6 100644 --- a/tools/run_tests/sources_and_headers.json +++ b/tools/run_tests/sources_and_headers.json @@ -4693,8 +4693,7 @@ { "deps": [ "grpc++", - "grpc++_reflection", - "grpc++_test_config" + "grpc++_reflection" ], "headers": [ "test/cpp/util/cli_call.h", diff --git a/vsprojects/vcxproj/grpc_cli_libs/grpc_cli_libs.vcxproj b/vsprojects/vcxproj/grpc_cli_libs/grpc_cli_libs.vcxproj index 2790884ee1b..4c61baa506d 100644 --- a/vsprojects/vcxproj/grpc_cli_libs/grpc_cli_libs.vcxproj +++ b/vsprojects/vcxproj/grpc_cli_libs/grpc_cli_libs.vcxproj @@ -176,9 +176,6 @@ <ProjectReference Include="$(SolutionDir)\..\vsprojects\vcxproj\.\grpc++\grpc++.vcxproj"> <Project>{C187A093-A0FE-489D-A40A-6E33DE0F9FEB}</Project> </ProjectReference> - <ProjectReference Include="$(SolutionDir)\..\vsprojects\vcxproj\.\grpc++_test_config\grpc++_test_config.vcxproj"> - <Project>{3F7D093D-11F9-C4BC-BEB7-18EB28E3F290}</Project> - </ProjectReference> </ItemGroup> <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" /> <ImportGroup Label="ExtensionTargets"> From 0e08b861a908611377a4cb6ba23d6f0422e9d102 Mon Sep 17 00:00:00 2001 From: Julien Boeuf <jboeuf@google.com> Date: Mon, 26 Sep 2016 10:24:07 -0700 Subject: [PATCH 14/83] Updating roots from Mozilla. --- etc/roots.pem | 111 -------------------------------------------------- 1 file changed, 111 deletions(-) diff --git a/etc/roots.pem b/etc/roots.pem index d376e58ff50..79357e01f2b 100644 --- a/etc/roots.pem +++ b/etc/roots.pem @@ -1706,38 +1706,6 @@ fQjGGoe9GKhzvSbKYAydzpmfz1wPMOG+FDHqAjAU9JM8SaczepBGR7NjfRObTrdv GDeAU/7dIOA1mjbRxwG55tzd8/8dLDoWV9mSOdY= -----END CERTIFICATE----- -# Issuer: CN=IGC/A O=PM/SGDN OU=DCSSI -# Subject: CN=IGC/A O=PM/SGDN OU=DCSSI -# Label: "IGC/A" -# Serial: 245102874772 -# MD5 Fingerprint: 0c:7f:dd:6a:f4:2a:b9:c8:9b:bd:20:7e:a9:db:5c:37 -# SHA1 Fingerprint: 60:d6:89:74:b5:c2:65:9e:8a:0f:c1:88:7c:88:d2:46:69:1b:18:2c -# SHA256 Fingerprint: b9:be:a7:86:0a:96:2e:a3:61:1d:ab:97:ab:6d:a3:e2:1c:10:68:b9:7d:55:57:5e:d0:e1:12:79:c1:1c:89:32 ------BEGIN CERTIFICATE----- -MIIEAjCCAuqgAwIBAgIFORFFEJQwDQYJKoZIhvcNAQEFBQAwgYUxCzAJBgNVBAYT -AkZSMQ8wDQYDVQQIEwZGcmFuY2UxDjAMBgNVBAcTBVBhcmlzMRAwDgYDVQQKEwdQ -TS9TR0ROMQ4wDAYDVQQLEwVEQ1NTSTEOMAwGA1UEAxMFSUdDL0ExIzAhBgkqhkiG -9w0BCQEWFGlnY2FAc2dkbi5wbS5nb3V2LmZyMB4XDTAyMTIxMzE0MjkyM1oXDTIw -MTAxNzE0MjkyMlowgYUxCzAJBgNVBAYTAkZSMQ8wDQYDVQQIEwZGcmFuY2UxDjAM -BgNVBAcTBVBhcmlzMRAwDgYDVQQKEwdQTS9TR0ROMQ4wDAYDVQQLEwVEQ1NTSTEO -MAwGA1UEAxMFSUdDL0ExIzAhBgkqhkiG9w0BCQEWFGlnY2FAc2dkbi5wbS5nb3V2 -LmZyMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAsh/R0GLFMzvABIaI -s9z4iPf930Pfeo2aSVz2TqrMHLmh6yeJ8kbpO0px1R2OLc/mratjUMdUC24SyZA2 -xtgv2pGqaMVy/hcKshd+ebUyiHDKcMCWSo7kVc0dJ5S/znIq7Fz5cyD+vfcuiWe4 -u0dzEvfRNWk68gq5rv9GQkaiv6GFGvm/5P9JhfejcIYyHF2fYPepraX/z9E0+X1b -F8bc1g4oa8Ld8fUzaJ1O/Id8NhLWo4DoQw1VYZTqZDdH6nfK0LJYBcNdfrGoRpAx -Vs5wKpayMLh35nnAvSk7/ZR3TL0gzUEl4C7HG7vupARB0l2tEmqKm0f7yd1GQOGd -PDPQtQIDAQABo3cwdTAPBgNVHRMBAf8EBTADAQH/MAsGA1UdDwQEAwIBRjAVBgNV -HSAEDjAMMAoGCCqBegF5AQEBMB0GA1UdDgQWBBSjBS8YYFDCiQrdKyFP/45OqDAx -NjAfBgNVHSMEGDAWgBSjBS8YYFDCiQrdKyFP/45OqDAxNjANBgkqhkiG9w0BAQUF -AAOCAQEABdwm2Pp3FURo/C9mOnTgXeQp/wYHE4RKq89toB9RlPhJy3Q2FLwV3duJ -L92PoF189RLrn544pEfMs5bZvpwlqwN+Mw+VgQ39FuCIvjfwbF3QMZsyK10XZZOY -YLxuj7GoPB7ZHPOpJkL5ZB3C55L29B5aqhlSXa/oovdgoPaN8In1buAKBQGVyYsg -Crpa/JosPL3Dt8ldeCUFP1YUmwza+zpI/pdpXsoQhvdOlgQITeywvl3cO45Pwf2a -NjSaTFR+FwNIlQgRHAdvhQh+XU3Endv7rs6y0bO4g2wdsrN58dhwmX7wEwLOXt1R -0982gaEbeC9xs/FZTEYYKKuF0mBWWg== ------END CERTIFICATE----- - # Issuer: O=SECOM Trust Systems CO.,LTD. OU=Security Communication EV RootCA1 # Subject: O=SECOM Trust Systems CO.,LTD. OU=Security Communication EV RootCA1 # Label: "Security Communication EV RootCA1" @@ -2047,48 +2015,6 @@ h7U/2k3ZIQAw3pDaDtMaSKk+hQsUi4y8QZ5q9w5wwDX3OaJdZtB7WZ+oRxKaJyOk LY4ng5IgodcVf/EuGO70SH8vf/GhGLWhC5SgYiAynB321O+/TIho -----END CERTIFICATE----- -# Issuer: CN=EBG Elektronik Sertifika Hizmet Sağlayıcısı O=EBG Bilişim Teknolojileri ve Hizmetleri A.Ş. -# Subject: CN=EBG Elektronik Sertifika Hizmet Sağlayıcısı O=EBG Bilişim Teknolojileri ve Hizmetleri A.Ş. -# Label: "EBG Elektronik Sertifika Hizmet Sa\xC4\x9Flay\xc4\xb1\x63\xc4\xb1s\xc4\xb1" -# Serial: 5525761995591021570 -# MD5 Fingerprint: 2c:20:26:9d:cb:1a:4a:00:85:b5:b7:5a:ae:c2:01:37 -# SHA1 Fingerprint: 8c:96:ba:eb:dd:2b:07:07:48:ee:30:32:66:a0:f3:98:6e:7c:ae:58 -# SHA256 Fingerprint: 35:ae:5b:dd:d8:f7:ae:63:5c:ff:ba:56:82:a8:f0:0b:95:f4:84:62:c7:10:8e:e9:a0:e5:29:2b:07:4a:af:b2 ------BEGIN CERTIFICATE----- -MIIF5zCCA8+gAwIBAgIITK9zQhyOdAIwDQYJKoZIhvcNAQEFBQAwgYAxODA2BgNV -BAMML0VCRyBFbGVrdHJvbmlrIFNlcnRpZmlrYSBIaXptZXQgU2HEn2xhecSxY8Sx -c8SxMTcwNQYDVQQKDC5FQkcgQmlsacWfaW0gVGVrbm9sb2ppbGVyaSB2ZSBIaXpt -ZXRsZXJpIEEuxZ4uMQswCQYDVQQGEwJUUjAeFw0wNjA4MTcwMDIxMDlaFw0xNjA4 -MTQwMDMxMDlaMIGAMTgwNgYDVQQDDC9FQkcgRWxla3Ryb25payBTZXJ0aWZpa2Eg -SGl6bWV0IFNhxJ9sYXnEsWPEsXPEsTE3MDUGA1UECgwuRUJHIEJpbGnFn2ltIFRl -a25vbG9qaWxlcmkgdmUgSGl6bWV0bGVyaSBBLsWeLjELMAkGA1UEBhMCVFIwggIi -MA0GCSqGSIb3DQEBAQUAA4ICDwAwggIKAoICAQDuoIRh0DpqZhAy2DE4f6en5f2h -4fuXd7hxlugTlkaDT7byX3JWbhNgpQGR4lvFzVcfd2NR/y8927k/qqk153nQ9dAk -tiHq6yOU/im/+4mRDGSaBUorzAzu8T2bgmmkTPiab+ci2hC6X5L8GCcKqKpE+i4s -tPtGmggDg3KriORqcsnlZR9uKg+ds+g75AxuetpX/dfreYteIAbTdgtsApWjluTL -dlHRKJ2hGvxEok3MenaoDT2/F08iiFD9rrbskFBKW5+VQarKD7JK/oCZTqNGFav4 -c0JqwmZ2sQomFd2TkuzbqV9UIlKRcF0T6kjsbgNs2d1s/OsNA/+mgxKb8amTD8Um -TDGyY5lhcucqZJnSuOl14nypqZoaqsNW2xCaPINStnuWt6yHd6i58mcLlEOzrz5z -+kI2sSXFCjEmN1ZnuqMLfdb3ic1nobc6HmZP9qBVFCVMLDMNpkGMvQQxahByCp0O -Lna9XvNRiYuoP1Vzv9s6xiQFlpJIqkuNKgPlV5EQ9GooFW5Hd4RcUXSfGenmHmMW -OeMRFeNYGkS9y8RsZteEBt8w9DeiQyJ50hBs37vmExH8nYQKE3vwO9D8owrXieqW -fo1IhR5kX9tUoqzVegJ5a9KK8GfaZXINFHDk6Y54jzJ0fFfy1tb0Nokb+Clsi7n2 -l9GkLqq+CxnCRelwXQIDAJ3Zo2MwYTAPBgNVHRMBAf8EBTADAQH/MA4GA1UdDwEB -/wQEAwIBBjAdBgNVHQ4EFgQU587GT/wWZ5b6SqMHwQSny2re2kcwHwYDVR0jBBgw -FoAU587GT/wWZ5b6SqMHwQSny2re2kcwDQYJKoZIhvcNAQEFBQADggIBAJuYml2+ -8ygjdsZs93/mQJ7ANtyVDR2tFcU22NU57/IeIl6zgrRdu0waypIN30ckHrMk2pGI -6YNw3ZPX6bqz3xZaPt7gyPvT/Wwp+BVGoGgmzJNSroIBk5DKd8pNSe/iWtkqvTDO -TLKBtjDOWU/aWR1qeqRFsIImgYZ29fUQALjuswnoT4cCB64kXPBfrAowzIpAoHME -wfuJJPaaHFy3PApnNgUIMbOv2AFoKuB4j3TeuFGkjGwgPaL7s9QJ/XvCgKqTbCmY -Iai7FvOpEl90tYeY8pUm3zTvilORiF0alKM/fCL414i6poyWqD1SNGKfAB5UVUJn -xk1Gj7sURT0KlhaOEKGXmdXTMIXM3rRyt7yKPBgpaP3ccQfuJDlq+u2lrDgv+R4Q -DgZxGhBM/nV+/x5XOULK1+EVoVZVWRvRo68R2E7DpSvvkL/A7IITW43WciyTTo9q -Kd+FPNMN4KIYEsxVL0e3p5sC/kH2iExt2qkBR4NkJ2IQgtYSe14DHzSpyZH+r11t -hie3I6p1GMog57AP14kOpmciY/SDQSsGS7tY1dHXt7kQY9iJSrSq3RZj9W6+YKH4 -7ejWkE8axsWgKdOnIaj1Wjz3x0miIZpKlVIglnKaZsv30oZDfCK+lvm9AahH3eU7 -QPl1K5srRmSGjR70j/sHd9DqSaIcjVIUpgqT ------END CERTIFICATE----- - # Issuer: O=certSIGN OU=certSIGN ROOT CA # Subject: O=certSIGN OU=certSIGN ROOT CA # Label: "certSIGN ROOT CA" @@ -2427,43 +2353,6 @@ Y7BXN0Ute4qcvwXqZVUz9zkQxSgqIXobisQk+T8VyJoVIPVVYpbtbZNQvOSqeK3Z ywplh6ZmwcSBo3c6WB4L7oOLnR7SUqTMHW+wmG2UMbX4cQrcufx9MmDm66+KAQ== -----END CERTIFICATE----- -# Issuer: CN=Juur-SK O=AS Sertifitseerimiskeskus -# Subject: CN=Juur-SK O=AS Sertifitseerimiskeskus -# Label: "Juur-SK" -# Serial: 999181308 -# MD5 Fingerprint: aa:8e:5d:d9:f8:db:0a:58:b7:8d:26:87:6c:82:35:55 -# SHA1 Fingerprint: 40:9d:4b:d9:17:b5:5c:27:b6:9b:64:cb:98:22:44:0d:cd:09:b8:89 -# SHA256 Fingerprint: ec:c3:e9:c3:40:75:03:be:e0:91:aa:95:2f:41:34:8f:f8:8b:aa:86:3b:22:64:be:fa:c8:07:90:15:74:e9:39 ------BEGIN CERTIFICATE----- -MIIE5jCCA86gAwIBAgIEO45L/DANBgkqhkiG9w0BAQUFADBdMRgwFgYJKoZIhvcN -AQkBFglwa2lAc2suZWUxCzAJBgNVBAYTAkVFMSIwIAYDVQQKExlBUyBTZXJ0aWZp -dHNlZXJpbWlza2Vza3VzMRAwDgYDVQQDEwdKdXVyLVNLMB4XDTAxMDgzMDE0MjMw -MVoXDTE2MDgyNjE0MjMwMVowXTEYMBYGCSqGSIb3DQEJARYJcGtpQHNrLmVlMQsw -CQYDVQQGEwJFRTEiMCAGA1UEChMZQVMgU2VydGlmaXRzZWVyaW1pc2tlc2t1czEQ -MA4GA1UEAxMHSnV1ci1TSzCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEB -AIFxNj4zB9bjMI0TfncyRsvPGbJgMUaXhvSYRqTCZUXP00B841oiqBB4M8yIsdOB -SvZiF3tfTQou0M+LI+5PAk676w7KvRhj6IAcjeEcjT3g/1tf6mTll+g/mX8MCgkz -ABpTpyHhOEvWgxutr2TC+Rx6jGZITWYfGAriPrsfB2WThbkasLnE+w0R9vXW+RvH -LCu3GFH+4Hv2qEivbDtPL+/40UceJlfwUR0zlv/vWT3aTdEVNMfqPxZIe5EcgEMP -PbgFPtGzlc3Yyg/CQ2fbt5PgIoIuvvVoKIO5wTtpeyDaTpxt4brNj3pssAki14sL -2xzVWiZbDcDq5WDQn/413z8CAwEAAaOCAawwggGoMA8GA1UdEwEB/wQFMAMBAf8w -ggEWBgNVHSAEggENMIIBCTCCAQUGCisGAQQBzh8BAQEwgfYwgdAGCCsGAQUFBwIC -MIHDHoHAAFMAZQBlACAAcwBlAHIAdABpAGYAaQBrAGEAYQB0ACAAbwBuACAAdgDk -AGwAagBhAHMAdABhAHQAdQBkACAAQQBTAC0AaQBzACAAUwBlAHIAdABpAGYAaQB0 -AHMAZQBlAHIAaQBtAGkAcwBrAGUAcwBrAHUAcwAgAGEAbABhAG0ALQBTAEsAIABz -AGUAcgB0AGkAZgBpAGsAYQBhAHQAaQBkAGUAIABrAGkAbgBuAGkAdABhAG0AaQBz -AGUAawBzMCEGCCsGAQUFBwIBFhVodHRwOi8vd3d3LnNrLmVlL2Nwcy8wKwYDVR0f -BCQwIjAgoB6gHIYaaHR0cDovL3d3dy5zay5lZS9qdXVyL2NybC8wHQYDVR0OBBYE -FASqekej5ImvGs8KQKcYP2/v6X2+MB8GA1UdIwQYMBaAFASqekej5ImvGs8KQKcY -P2/v6X2+MA4GA1UdDwEB/wQEAwIB5jANBgkqhkiG9w0BAQUFAAOCAQEAe8EYlFOi -CfP+JmeaUOTDBS8rNXiRTHyoERF5TElZrMj3hWVcRrs7EKACr81Ptcw2Kuxd/u+g -kcm2k298gFTsxwhwDY77guwqYHhpNjbRxZyLabVAyJRld/JXIWY7zoVAtjNjGr95 -HvxcHdMdkxuLDF2FvZkwMhgJkVLpfKG6/2SSmuz+Ne6ML678IIbsSt4beDI3poHS -na9aEhbKmVv8b20OxaAehsmR0FyYgl9jDIpaq9iVpszLita/ZEuOyoqysOkhMp6q -qIWYNIE5ITuoOlIyPfZrN4YGWhWY3PARZv40ILcD9EEQfTmEeZZyY7aWAuVrua0Z -TbvGRNs2yyqcjg== ------END CERTIFICATE----- - # Issuer: CN=Hongkong Post Root CA 1 O=Hongkong Post # Subject: CN=Hongkong Post Root CA 1 O=Hongkong Post # Label: "Hongkong Post Root CA 1" From 94777240032e41dd0cfb56218a09912395aded05 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" <roth@google.com> Date: Tue, 27 Sep 2016 13:07:00 -0700 Subject: [PATCH 15/83] Change C++ API to expose wait_for_ready instead of fail_fast. --- include/grpc++/impl/codegen/client_context.h | 13 +++++++++---- src/cpp/client/client_context.cc | 2 +- test/cpp/end2end/client_crash_test.cc | 4 ++-- test/cpp/end2end/hybrid_end2end_test.cc | 10 +++++----- test/cpp/end2end/server_crash_test_client.cc | 2 +- test/cpp/qps/driver.cc | 6 +++--- 6 files changed, 21 insertions(+), 16 deletions(-) diff --git a/include/grpc++/impl/codegen/client_context.h b/include/grpc++/impl/codegen/client_context.h index 387d807c4b2..faabddecc3c 100644 --- a/include/grpc++/impl/codegen/client_context.h +++ b/include/grpc++/impl/codegen/client_context.h @@ -226,8 +226,13 @@ class ClientContext { /// EXPERIMENTAL: Set this request to be cacheable void set_cacheable(bool cacheable) { cacheable_ = cacheable; } - /// EXPERIMENTAL: Trigger fail-fast or not on this request - void set_fail_fast(bool fail_fast) { fail_fast_ = fail_fast; } + /// EXPERIMENTAL: Trigger wait-for-ready or not on this request + void set_wait_for_ready(bool wait_for_ready) { + wait_for_ready_ = wait_for_ready; + } + + /// DEPRECATED: Use set_wait_for_ready() instead. + void set_fail_fast(bool fail_fast) { wait_for_ready_ = !fail_fast; } #ifndef GRPC_CXX0X_NO_CHRONO /// Return the deadline for the client call. @@ -347,14 +352,14 @@ class ClientContext { uint32_t initial_metadata_flags() const { return (idempotent_ ? GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST : 0) | - (fail_fast_ ? 0 : GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY) | + (wait_for_ready_ ? GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY : 0) | (cacheable_ ? GRPC_INITIAL_METADATA_CACHEABLE_REQUEST : 0); } grpc::string authority() { return authority_; } bool initial_metadata_received_; - bool fail_fast_; + bool wait_for_ready_; bool idempotent_; bool cacheable_; std::shared_ptr<Channel> channel_; diff --git a/src/cpp/client/client_context.cc b/src/cpp/client/client_context.cc index 5b6aaa777bc..fb9a7c0459e 100644 --- a/src/cpp/client/client_context.cc +++ b/src/cpp/client/client_context.cc @@ -59,7 +59,7 @@ static ClientContext::GlobalCallbacks* g_client_callbacks = ClientContext::ClientContext() : initial_metadata_received_(false), - fail_fast_(true), + wait_for_ready_(false), idempotent_(false), cacheable_(false), call_(nullptr), diff --git a/test/cpp/end2end/client_crash_test.cc b/test/cpp/end2end/client_crash_test.cc index c452ad2beb4..966c04b0e3e 100644 --- a/test/cpp/end2end/client_crash_test.cc +++ b/test/cpp/end2end/client_crash_test.cc @@ -89,7 +89,7 @@ TEST_F(CrashTest, KillBeforeWrite) { EchoRequest request; EchoResponse response; ClientContext context; - context.set_fail_fast(false); + context.set_wait_for_ready(true); auto stream = stub->BidiStream(&context); @@ -115,7 +115,7 @@ TEST_F(CrashTest, KillAfterWrite) { EchoRequest request; EchoResponse response; ClientContext context; - context.set_fail_fast(false); + context.set_wait_for_ready(true); auto stream = stub->BidiStream(&context); diff --git a/test/cpp/end2end/hybrid_end2end_test.cc b/test/cpp/end2end/hybrid_end2end_test.cc index 82361d0e90d..8cd2e663470 100644 --- a/test/cpp/end2end/hybrid_end2end_test.cc +++ b/test/cpp/end2end/hybrid_end2end_test.cc @@ -261,7 +261,7 @@ class HybridEnd2endTest : public ::testing::Test { EchoRequest send_request; EchoResponse recv_response; ClientContext cli_ctx; - cli_ctx.set_fail_fast(false); + cli_ctx.set_wait_for_ready(true); send_request.set_message("Hello"); Status recv_status = stub_->Echo(&cli_ctx, send_request, &recv_response); EXPECT_EQ(send_request.message(), recv_response.message()); @@ -275,7 +275,7 @@ class HybridEnd2endTest : public ::testing::Test { EchoRequest send_request; EchoResponse recv_response; ClientContext cli_ctx; - cli_ctx.set_fail_fast(false); + cli_ctx.set_wait_for_ready(true); send_request.set_message("Hello"); Status recv_status = stub->Echo(&cli_ctx, send_request, &recv_response); EXPECT_EQ(send_request.message() + "_dup", recv_response.message()); @@ -287,7 +287,7 @@ class HybridEnd2endTest : public ::testing::Test { EchoResponse recv_response; grpc::string expected_message; ClientContext cli_ctx; - cli_ctx.set_fail_fast(false); + cli_ctx.set_wait_for_ready(true); send_request.set_message("Hello"); auto stream = stub_->RequestStream(&cli_ctx, &recv_response); for (int i = 0; i < 5; i++) { @@ -304,7 +304,7 @@ class HybridEnd2endTest : public ::testing::Test { EchoRequest request; EchoResponse response; ClientContext context; - context.set_fail_fast(false); + context.set_wait_for_ready(true); request.set_message("hello"); auto stream = stub_->ResponseStream(&context, request); @@ -324,7 +324,7 @@ class HybridEnd2endTest : public ::testing::Test { EchoRequest request; EchoResponse response; ClientContext context; - context.set_fail_fast(false); + context.set_wait_for_ready(true); grpc::string msg("hello"); auto stream = stub_->BidiStream(&context); diff --git a/test/cpp/end2end/server_crash_test_client.cc b/test/cpp/end2end/server_crash_test_client.cc index 10a251c952f..5df09cd8536 100644 --- a/test/cpp/end2end/server_crash_test_client.cc +++ b/test/cpp/end2end/server_crash_test_client.cc @@ -65,7 +65,7 @@ int main(int argc, char** argv) { EchoRequest request; EchoResponse response; grpc::ClientContext context; - context.set_fail_fast(false); + context.set_wait_for_ready(true); if (FLAGS_mode == "bidi") { auto stream = stub->BidiStream(&context); diff --git a/test/cpp/qps/driver.cc b/test/cpp/qps/driver.cc index b4c18bcb46e..7460bb526ac 100644 --- a/test/cpp/qps/driver.cc +++ b/test/cpp/qps/driver.cc @@ -83,7 +83,7 @@ static std::unordered_map<string, std::deque<int>> get_hosts_and_cores( auto stub = WorkerService::NewStub( CreateChannel(*it, InsecureChannelCredentials())); grpc::ClientContext ctx; - ctx.set_fail_fast(false); + ctx.set_wait_for_ready(true); CoreRequest dummy; CoreResponse cores; grpc::Status s = stub->CoreCount(&ctx, dummy, &cores); @@ -167,7 +167,7 @@ namespace runsc { static ClientContext* AllocContext(list<ClientContext>* contexts) { contexts->emplace_back(); auto context = &contexts->back(); - context->set_fail_fast(false); + context->set_wait_for_ready(true); return context; } @@ -527,7 +527,7 @@ bool RunQuit() { CreateChannel(workers[i], InsecureChannelCredentials())); Void dummy; grpc::ClientContext ctx; - ctx.set_fail_fast(false); + ctx.set_wait_for_ready(true); Status s = stub->QuitWorker(&ctx, dummy, &dummy); if (!s.ok()) { gpr_log(GPR_ERROR, "Worker %zu could not be properly quit because %s", i, From e8b87865b268856d5510abb6ffd044224c079c13 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" <roth@google.com> Date: Tue, 27 Sep 2016 13:16:18 -0700 Subject: [PATCH 16/83] Add doc of pending API cleanups for C++. --- doc/cpp/pending_api_cleanups.md | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 doc/cpp/pending_api_cleanups.md diff --git a/doc/cpp/pending_api_cleanups.md b/doc/cpp/pending_api_cleanups.md new file mode 100644 index 00000000000..3e77b657c62 --- /dev/null +++ b/doc/cpp/pending_api_cleanups.md @@ -0,0 +1,15 @@ +There are times when we make changes that include a temporary shim for +backward-compatibility (e.g., a macro or some other function to preserve +the original API) to avoid having to bump the major version number in +the next release. However, when we do eventually want to release a +feature that does change the API in a non-backward-compatible way, we +will wind up bumping the major version number anyway, at which point we +can take the opportunity to clean up any pending backward-compatibility +shims. + +This file lists all pending backward-compatibility changes that should +be cleaned up the next time we are going to bump the major version +number: + +- remove `ClientContext::set_fail_fast()` method from + `include/grpc++/impl/codegen/client_context.h` (commit `9477724`) From e529ea392824db09887f62cc00bbb46329a77003 Mon Sep 17 00:00:00 2001 From: Yuchen Zeng <zyc@google.com> Date: Tue, 27 Sep 2016 16:18:11 -0700 Subject: [PATCH 17/83] Rename variables --- test/cpp/util/proto_reflection_descriptor_database.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/cpp/util/proto_reflection_descriptor_database.cc b/test/cpp/util/proto_reflection_descriptor_database.cc index ae633ea7f4e..82d82a348d1 100644 --- a/test/cpp/util/proto_reflection_descriptor_database.cc +++ b/test/cpp/util/proto_reflection_descriptor_database.cc @@ -317,13 +317,13 @@ ProtoReflectionDescriptorDatabase::GetStream() { bool ProtoReflectionDescriptorDatabase::DoOneRequest( const ServerReflectionRequest& request, ServerReflectionResponse& response) { - bool request_succeed = false; + bool request_success = false; stream_mutex_.lock(); if (GetStream()->Write(request) && GetStream()->Read(&response)) { - request_succeed = true; + request_success = true; } stream_mutex_.unlock(); - return request_succeed; + return request_success; } } // namespace grpc From 15bc4627de675cc148989977e1f6afd3ea3c6755 Mon Sep 17 00:00:00 2001 From: Yuchen Zeng <zyc@google.com> Date: Tue, 27 Sep 2016 16:22:35 -0700 Subject: [PATCH 18/83] Rename variables ... again --- test/cpp/util/proto_reflection_descriptor_database.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/cpp/util/proto_reflection_descriptor_database.cc b/test/cpp/util/proto_reflection_descriptor_database.cc index 82d82a348d1..409b0de1b20 100644 --- a/test/cpp/util/proto_reflection_descriptor_database.cc +++ b/test/cpp/util/proto_reflection_descriptor_database.cc @@ -317,13 +317,13 @@ ProtoReflectionDescriptorDatabase::GetStream() { bool ProtoReflectionDescriptorDatabase::DoOneRequest( const ServerReflectionRequest& request, ServerReflectionResponse& response) { - bool request_success = false; + bool success = false; stream_mutex_.lock(); if (GetStream()->Write(request) && GetStream()->Read(&response)) { - request_success = true; + success = true; } stream_mutex_.unlock(); - return request_success; + return success; } } // namespace grpc From 16c26ed252fa5a186db6e93248325b0f918c8625 Mon Sep 17 00:00:00 2001 From: Muxi Yan <mxyan@google.com> Date: Mon, 26 Sep 2016 17:22:03 -0700 Subject: [PATCH 19/83] Add support of PUT method --- .../cronet/transport/cronet_transport.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.c b/src/core/ext/transport/cronet/transport/cronet_transport.c index 366690acf2b..6c2b53f2917 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.c +++ b/src/core/ext/transport/cronet/transport/cronet_transport.c @@ -531,7 +531,8 @@ static void create_grpc_frame(gpr_slice_buffer *write_slice_buffer, */ static void convert_metadata_to_cronet_headers( grpc_linked_mdelem *head, const char *host, char **pp_url, - cronet_bidirectional_stream_header **pp_headers, size_t *p_num_headers) { + cronet_bidirectional_stream_header **pp_headers, size_t *p_num_headers, + const char ** method) { grpc_linked_mdelem *curr = head; /* Walk the linked list and get number of header fields */ size_t num_headers_available = 0; @@ -558,11 +559,20 @@ static void convert_metadata_to_cronet_headers( curr = curr->next; const char *key = grpc_mdstr_as_c_string(mdelem->key); const char *value = grpc_mdstr_as_c_string(mdelem->value); - if (mdelem->key == GRPC_MDSTR_METHOD || mdelem->key == GRPC_MDSTR_SCHEME || + if (mdelem->key == GRPC_MDSTR_SCHEME || mdelem->key == GRPC_MDSTR_AUTHORITY) { /* Cronet populates these fields on its own */ continue; } + if (mdelem->key == GRPC_MDSTR_METHOD) { + if (mdelem->value == GRPC_MDSTR_PUT) { + *method = grpc_static_metadata_strings[74]; + } else { + /* POST method in default*/ + *method = grpc_static_metadata_strings[71]; + } + continue; + } if (mdelem->key == GRPC_MDSTR_PATH) { /* Create URL by appending :path value to the hostname */ gpr_asprintf(pp_url, "https://%s%s", host, value); @@ -760,14 +770,15 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, &cronet_callbacks); CRONET_LOG(GPR_DEBUG, "%p = cronet_bidirectional_stream_create()", s->cbs); char *url; + const char *method; s->header_array.headers = NULL; convert_metadata_to_cronet_headers( stream_op->send_initial_metadata->list.head, s->curr_ct.host, &url, - &s->header_array.headers, &s->header_array.count); + &s->header_array.headers, &s->header_array.count, &method); s->header_array.capacity = s->header_array.count; CRONET_LOG(GPR_DEBUG, "cronet_bidirectional_stream_start(%p, %s)", s->cbs, url); - cronet_bidirectional_stream_start(s->cbs, url, 0, "POST", &s->header_array, + cronet_bidirectional_stream_start(s->cbs, url, 0, method, &s->header_array, false); stream_state->state_op_done[OP_SEND_INITIAL_METADATA] = true; result = ACTION_TAKEN_WITH_CALLBACK; From fdea83d42afbebfecfea1edf9143619640115824 Mon Sep 17 00:00:00 2001 From: Muxi Yan <mxyan@google.com> Date: Tue, 27 Sep 2016 16:11:18 -0700 Subject: [PATCH 20/83] Update grpc objc API for support of PUT method --- src/objective-c/GRPCClient/GRPCCall.h | 7 ++++++- src/objective-c/GRPCClient/GRPCCall.m | 14 ++++++++++++-- .../GRPCClient/private/GRPCWrappedCall.h | 4 ++++ .../GRPCClient/private/GRPCWrappedCall.m | 14 ++++++++++++-- src/objective-c/ProtoRPC/ProtoRPC.h | 3 ++- src/objective-c/ProtoRPC/ProtoRPC.m | 9 ++++++--- src/objective-c/ProtoRPC/ProtoService.h | 3 ++- src/objective-c/ProtoRPC/ProtoService.m | 6 ++++-- 8 files changed, 48 insertions(+), 12 deletions(-) diff --git a/src/objective-c/GRPCClient/GRPCCall.h b/src/objective-c/GRPCClient/GRPCCall.h index b9e741dfa8f..fc59e5f5e91 100644 --- a/src/objective-c/GRPCClient/GRPCCall.h +++ b/src/objective-c/GRPCClient/GRPCCall.h @@ -225,7 +225,12 @@ extern id const kGRPCTrailersKey; */ - (instancetype)initWithHost:(NSString *)host path:(NSString *)path - requestsWriter:(GRXWriter *)requestsWriter NS_DESIGNATED_INITIALIZER; + requestsWriter:(GRXWriter *)requestsWriter; + +- (instancetype)initWithHost:(NSString *)host + path:(NSString *)path + requestsWriter:(GRXWriter *)requestsWriter + http2Method:(NSString *)http2Method NS_DESIGNATED_INITIALIZER; /** * Finishes the request side of this call, notifies the server that the RPC should be cancelled, and diff --git a/src/objective-c/GRPCClient/GRPCCall.m b/src/objective-c/GRPCClient/GRPCCall.m index eecda4c03a0..6bd80c8b3fe 100644 --- a/src/objective-c/GRPCClient/GRPCCall.m +++ b/src/objective-c/GRPCClient/GRPCCall.m @@ -75,6 +75,7 @@ NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey"; NSString *_host; NSString *_path; + NSString *_http2Method; GRPCWrappedCall *_wrappedCall; GRPCConnectivityMonitor *_connectivityMonitor; @@ -109,13 +110,20 @@ NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey"; } - (instancetype)init { - return [self initWithHost:nil path:nil requestsWriter:nil]; + return [self initWithHost:nil path:nil requestsWriter:nil http2Method:nil]; +} + +- (instancetype)initWithHost:(NSString *)host + path:(NSString *)path + requestsWriter:(GRXWriter *)requestWriter{ + return [self initWithHost:host path:path requestsWriter:requestWriter http2Method:@"POST"]; } // Designated initializer - (instancetype)initWithHost:(NSString *)host path:(NSString *)path - requestsWriter:(GRXWriter *)requestWriter { + requestsWriter:(GRXWriter *)requestWriter + http2Method:(NSString *)http2Method { if (!host || !path) { [NSException raise:NSInvalidArgumentException format:@"Neither host nor path can be nil."]; } @@ -126,6 +134,7 @@ NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey"; if ((self = [super init])) { _host = [host copy]; _path = [path copy]; + _http2Method = http2Method; // Serial queue to invoke the non-reentrant methods of the grpc_call object. _callQueue = dispatch_queue_create("io.grpc.call", NULL); @@ -231,6 +240,7 @@ NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey"; - (void)sendHeaders:(NSDictionary *)headers { // TODO(jcanizales): Add error handlers for async failures [_wrappedCall startBatchWithOperations:@[[[GRPCOpSendMetadata alloc] initWithMetadata:headers + http2Method:_http2Method handler:nil]]]; } diff --git a/src/objective-c/GRPCClient/private/GRPCWrappedCall.h b/src/objective-c/GRPCClient/private/GRPCWrappedCall.h index e37ed1b59fb..8b64b27e567 100644 --- a/src/objective-c/GRPCClient/private/GRPCWrappedCall.h +++ b/src/objective-c/GRPCClient/private/GRPCWrappedCall.h @@ -45,6 +45,10 @@ @interface GRPCOpSendMetadata : GRPCOperation - (instancetype)initWithMetadata:(NSDictionary *)metadata + handler:(void(^)())handler; + +- (instancetype)initWithMetadata:(NSDictionary *)metadata + http2Method:(NSString *)http2Method handler:(void(^)())handler NS_DESIGNATED_INITIALIZER; @end diff --git a/src/objective-c/GRPCClient/private/GRPCWrappedCall.m b/src/objective-c/GRPCClient/private/GRPCWrappedCall.m index 13394296609..2836f99bf17 100644 --- a/src/objective-c/GRPCClient/private/GRPCWrappedCall.m +++ b/src/objective-c/GRPCClient/private/GRPCWrappedCall.m @@ -64,16 +64,26 @@ @implementation GRPCOpSendMetadata - (instancetype)init { - return [self initWithMetadata:nil handler:nil]; + return [self initWithMetadata:nil http2Method:nil handler:nil]; } -- (instancetype)initWithMetadata:(NSDictionary *)metadata handler:(void (^)())handler { +- (instancetype)initWithMetadata:(NSDictionary *)metadata + handler:(void (^)())handler { + return [self initWithMetadata:metadata http2Method:@"POST" handler:handler]; +} + +- (instancetype)initWithMetadata:(NSDictionary *)metadata + http2Method:(NSString *)http2Method + handler:(void (^)())handler { if (self = [super init]) { _op.op = GRPC_OP_SEND_INITIAL_METADATA; _op.data.send_initial_metadata.count = metadata.count; _op.data.send_initial_metadata.metadata = metadata.grpc_metadataArray; _op.data.send_initial_metadata.maybe_compression_level.is_set = false; _op.data.send_initial_metadata.maybe_compression_level.level = 0; + if ([http2Method isEqualToString:@"PUT"]) { + _op.flags = GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST; + } _handler = handler; } return self; diff --git a/src/objective-c/ProtoRPC/ProtoRPC.h b/src/objective-c/ProtoRPC/ProtoRPC.h index 04fc1e45f16..509a15abae8 100644 --- a/src/objective-c/ProtoRPC/ProtoRPC.h +++ b/src/objective-c/ProtoRPC/ProtoRPC.h @@ -47,7 +47,8 @@ __attribute__((deprecated("Please use GRPCProtoCall."))) method:(GRPCProtoMethod *)method requestsWriter:(GRXWriter *)requestsWriter responseClass:(Class)responseClass - responsesWriteable:(id<GRXWriteable>)responsesWriteable NS_DESIGNATED_INITIALIZER; + responsesWriteable:(id<GRXWriteable>)responsesWriteable + http2Method:(NSString *)http2Method NS_DESIGNATED_INITIALIZER; - (void)start; @end diff --git a/src/objective-c/ProtoRPC/ProtoRPC.m b/src/objective-c/ProtoRPC/ProtoRPC.m index 83d1b655e8d..67405d2a196 100644 --- a/src/objective-c/ProtoRPC/ProtoRPC.m +++ b/src/objective-c/ProtoRPC/ProtoRPC.m @@ -65,7 +65,8 @@ static NSError *ErrorForBadProto(id proto, Class expectedClass, NSError *parsing #pragma clang diagnostic ignored "-Wobjc-designated-initializers" - (instancetype)initWithHost:(NSString *)host path:(NSString *)path - requestsWriter:(GRXWriter *)requestsWriter { + requestsWriter:(GRXWriter *)requestsWriter + http2Method:(NSString *)http2Method { [NSException raise:NSInvalidArgumentException format:@"Please use ProtoRPC's designated initializer instead."]; return nil; @@ -77,7 +78,8 @@ static NSError *ErrorForBadProto(id proto, Class expectedClass, NSError *parsing method:(GRPCProtoMethod *)method requestsWriter:(GRXWriter *)requestsWriter responseClass:(Class)responseClass - responsesWriteable:(id<GRXWriteable>)responsesWriteable { + responsesWriteable:(id<GRXWriteable>)responsesWriteable + http2Method:(NSString *)http2Method { // Because we can't tell the type system to constrain the class, we need to check at runtime: if (![responseClass respondsToSelector:@selector(parseFromData:error:)]) { [NSException raise:NSInvalidArgumentException @@ -91,7 +93,8 @@ static NSError *ErrorForBadProto(id proto, Class expectedClass, NSError *parsing } return [proto data]; }]; - if ((self = [super initWithHost:host path:method.HTTPPath requestsWriter:bytesWriter])) { + if ((self = [super initWithHost:host path:method.HTTPPath requestsWriter:bytesWriter + http2Method:http2Method])) { __weak ProtoRPC *weakSelf = self; // A writeable that parses the proto messages received. diff --git a/src/objective-c/ProtoRPC/ProtoService.h b/src/objective-c/ProtoRPC/ProtoService.h index 7faae1b49c9..80fab37fd55 100644 --- a/src/objective-c/ProtoRPC/ProtoService.h +++ b/src/objective-c/ProtoRPC/ProtoService.h @@ -47,7 +47,8 @@ __attribute__((deprecated("Please use GRPCProtoService."))) - (GRPCProtoCall *)RPCToMethod:(NSString *)method requestsWriter:(GRXWriter *)requestsWriter responseClass:(Class)responseClass - responsesWriteable:(id<GRXWriteable>)responsesWriteable; + responsesWriteable:(id<GRXWriteable>)responsesWriteable + http2Method:(NSString *)http2Method; @end diff --git a/src/objective-c/ProtoRPC/ProtoService.m b/src/objective-c/ProtoRPC/ProtoService.m index 3487fac59d3..fc3e0170728 100644 --- a/src/objective-c/ProtoRPC/ProtoService.m +++ b/src/objective-c/ProtoRPC/ProtoService.m @@ -68,7 +68,8 @@ - (GRPCProtoCall *)RPCToMethod:(NSString *)method requestsWriter:(GRXWriter *)requestsWriter responseClass:(Class)responseClass - responsesWriteable:(id<GRXWriteable>)responsesWriteable { + responsesWriteable:(id<GRXWriteable>)responsesWriteable + http2Method:(NSString *)http2Method { GRPCProtoMethod *methodName = [[GRPCProtoMethod alloc] initWithPackage:_packageName service:_serviceName method:method]; @@ -76,7 +77,8 @@ method:methodName requestsWriter:requestsWriter responseClass:responseClass - responsesWriteable:responsesWriteable]; + responsesWriteable:responsesWriteable + http2Method:http2Method]; } @end From e97f7c0ba6c2f7c55e2ea650f52597a06bcfa6fd Mon Sep 17 00:00:00 2001 From: Muxi Yan <mxyan@google.com> Date: Wed, 28 Sep 2016 11:25:57 -0700 Subject: [PATCH 21/83] Allow more general flags to be passed to ObjC API --- src/objective-c/GRPCClient/GRPCCall.h | 2 +- src/objective-c/GRPCClient/GRPCCall.m | 12 ++++++------ src/objective-c/GRPCClient/private/GRPCWrappedCall.h | 2 +- src/objective-c/GRPCClient/private/GRPCWrappedCall.m | 10 ++++------ src/objective-c/ProtoRPC/ProtoRPC.h | 2 +- src/objective-c/ProtoRPC/ProtoRPC.m | 6 +++--- src/objective-c/ProtoRPC/ProtoService.h | 2 +- src/objective-c/ProtoRPC/ProtoService.m | 4 ++-- 8 files changed, 19 insertions(+), 21 deletions(-) diff --git a/src/objective-c/GRPCClient/GRPCCall.h b/src/objective-c/GRPCClient/GRPCCall.h index fc59e5f5e91..9a58311fe16 100644 --- a/src/objective-c/GRPCClient/GRPCCall.h +++ b/src/objective-c/GRPCClient/GRPCCall.h @@ -230,7 +230,7 @@ extern id const kGRPCTrailersKey; - (instancetype)initWithHost:(NSString *)host path:(NSString *)path requestsWriter:(GRXWriter *)requestsWriter - http2Method:(NSString *)http2Method NS_DESIGNATED_INITIALIZER; + flags:(uint32_t)flags NS_DESIGNATED_INITIALIZER; /** * Finishes the request side of this call, notifies the server that the RPC should be cancelled, and diff --git a/src/objective-c/GRPCClient/GRPCCall.m b/src/objective-c/GRPCClient/GRPCCall.m index 6bd80c8b3fe..8a58db70514 100644 --- a/src/objective-c/GRPCClient/GRPCCall.m +++ b/src/objective-c/GRPCClient/GRPCCall.m @@ -75,7 +75,7 @@ NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey"; NSString *_host; NSString *_path; - NSString *_http2Method; + uint32_t _flags; GRPCWrappedCall *_wrappedCall; GRPCConnectivityMonitor *_connectivityMonitor; @@ -110,20 +110,20 @@ NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey"; } - (instancetype)init { - return [self initWithHost:nil path:nil requestsWriter:nil http2Method:nil]; + return [self initWithHost:nil path:nil requestsWriter:nil flags:0]; } - (instancetype)initWithHost:(NSString *)host path:(NSString *)path requestsWriter:(GRXWriter *)requestWriter{ - return [self initWithHost:host path:path requestsWriter:requestWriter http2Method:@"POST"]; + return [self initWithHost:host path:path requestsWriter:requestWriter flags:0]; } // Designated initializer - (instancetype)initWithHost:(NSString *)host path:(NSString *)path requestsWriter:(GRXWriter *)requestWriter - http2Method:(NSString *)http2Method { + flags:(uint32_t)flags { if (!host || !path) { [NSException raise:NSInvalidArgumentException format:@"Neither host nor path can be nil."]; } @@ -134,7 +134,7 @@ NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey"; if ((self = [super init])) { _host = [host copy]; _path = [path copy]; - _http2Method = http2Method; + _flags = flags; // Serial queue to invoke the non-reentrant methods of the grpc_call object. _callQueue = dispatch_queue_create("io.grpc.call", NULL); @@ -240,7 +240,7 @@ NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey"; - (void)sendHeaders:(NSDictionary *)headers { // TODO(jcanizales): Add error handlers for async failures [_wrappedCall startBatchWithOperations:@[[[GRPCOpSendMetadata alloc] initWithMetadata:headers - http2Method:_http2Method + flags:_flags handler:nil]]]; } diff --git a/src/objective-c/GRPCClient/private/GRPCWrappedCall.h b/src/objective-c/GRPCClient/private/GRPCWrappedCall.h index 8b64b27e567..52233c82420 100644 --- a/src/objective-c/GRPCClient/private/GRPCWrappedCall.h +++ b/src/objective-c/GRPCClient/private/GRPCWrappedCall.h @@ -48,7 +48,7 @@ handler:(void(^)())handler; - (instancetype)initWithMetadata:(NSDictionary *)metadata - http2Method:(NSString *)http2Method + flags:(uint32_t)flags handler:(void(^)())handler NS_DESIGNATED_INITIALIZER; @end diff --git a/src/objective-c/GRPCClient/private/GRPCWrappedCall.m b/src/objective-c/GRPCClient/private/GRPCWrappedCall.m index 2836f99bf17..627b6aa86dd 100644 --- a/src/objective-c/GRPCClient/private/GRPCWrappedCall.m +++ b/src/objective-c/GRPCClient/private/GRPCWrappedCall.m @@ -64,16 +64,16 @@ @implementation GRPCOpSendMetadata - (instancetype)init { - return [self initWithMetadata:nil http2Method:nil handler:nil]; + return [self initWithMetadata:nil flags:0 handler:nil]; } - (instancetype)initWithMetadata:(NSDictionary *)metadata handler:(void (^)())handler { - return [self initWithMetadata:metadata http2Method:@"POST" handler:handler]; + return [self initWithMetadata:metadata flags:0 handler:handler]; } - (instancetype)initWithMetadata:(NSDictionary *)metadata - http2Method:(NSString *)http2Method + flags:(uint32_t)flags handler:(void (^)())handler { if (self = [super init]) { _op.op = GRPC_OP_SEND_INITIAL_METADATA; @@ -81,9 +81,7 @@ _op.data.send_initial_metadata.metadata = metadata.grpc_metadataArray; _op.data.send_initial_metadata.maybe_compression_level.is_set = false; _op.data.send_initial_metadata.maybe_compression_level.level = 0; - if ([http2Method isEqualToString:@"PUT"]) { - _op.flags = GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST; - } + _op.flags = flags; _handler = handler; } return self; diff --git a/src/objective-c/ProtoRPC/ProtoRPC.h b/src/objective-c/ProtoRPC/ProtoRPC.h index 509a15abae8..75c9b9bb3bc 100644 --- a/src/objective-c/ProtoRPC/ProtoRPC.h +++ b/src/objective-c/ProtoRPC/ProtoRPC.h @@ -48,7 +48,7 @@ __attribute__((deprecated("Please use GRPCProtoCall."))) requestsWriter:(GRXWriter *)requestsWriter responseClass:(Class)responseClass responsesWriteable:(id<GRXWriteable>)responsesWriteable - http2Method:(NSString *)http2Method NS_DESIGNATED_INITIALIZER; + flags:(uint32_t)flags NS_DESIGNATED_INITIALIZER; - (void)start; @end diff --git a/src/objective-c/ProtoRPC/ProtoRPC.m b/src/objective-c/ProtoRPC/ProtoRPC.m index 67405d2a196..2514d616f16 100644 --- a/src/objective-c/ProtoRPC/ProtoRPC.m +++ b/src/objective-c/ProtoRPC/ProtoRPC.m @@ -66,7 +66,7 @@ static NSError *ErrorForBadProto(id proto, Class expectedClass, NSError *parsing - (instancetype)initWithHost:(NSString *)host path:(NSString *)path requestsWriter:(GRXWriter *)requestsWriter - http2Method:(NSString *)http2Method { + flags:(uint32_t)flags { [NSException raise:NSInvalidArgumentException format:@"Please use ProtoRPC's designated initializer instead."]; return nil; @@ -79,7 +79,7 @@ static NSError *ErrorForBadProto(id proto, Class expectedClass, NSError *parsing requestsWriter:(GRXWriter *)requestsWriter responseClass:(Class)responseClass responsesWriteable:(id<GRXWriteable>)responsesWriteable - http2Method:(NSString *)http2Method { + flags:(uint32_t)flags { // Because we can't tell the type system to constrain the class, we need to check at runtime: if (![responseClass respondsToSelector:@selector(parseFromData:error:)]) { [NSException raise:NSInvalidArgumentException @@ -94,7 +94,7 @@ static NSError *ErrorForBadProto(id proto, Class expectedClass, NSError *parsing return [proto data]; }]; if ((self = [super initWithHost:host path:method.HTTPPath requestsWriter:bytesWriter - http2Method:http2Method])) { + flags:flags])) { __weak ProtoRPC *weakSelf = self; // A writeable that parses the proto messages received. diff --git a/src/objective-c/ProtoRPC/ProtoService.h b/src/objective-c/ProtoRPC/ProtoService.h index 80fab37fd55..9a49ebd257b 100644 --- a/src/objective-c/ProtoRPC/ProtoService.h +++ b/src/objective-c/ProtoRPC/ProtoService.h @@ -48,7 +48,7 @@ __attribute__((deprecated("Please use GRPCProtoService."))) requestsWriter:(GRXWriter *)requestsWriter responseClass:(Class)responseClass responsesWriteable:(id<GRXWriteable>)responsesWriteable - http2Method:(NSString *)http2Method; + flags:(uint32_t)flags; @end diff --git a/src/objective-c/ProtoRPC/ProtoService.m b/src/objective-c/ProtoRPC/ProtoService.m index fc3e0170728..b237164a006 100644 --- a/src/objective-c/ProtoRPC/ProtoService.m +++ b/src/objective-c/ProtoRPC/ProtoService.m @@ -69,7 +69,7 @@ requestsWriter:(GRXWriter *)requestsWriter responseClass:(Class)responseClass responsesWriteable:(id<GRXWriteable>)responsesWriteable - http2Method:(NSString *)http2Method { + flags:(uint32_t)flags { GRPCProtoMethod *methodName = [[GRPCProtoMethod alloc] initWithPackage:_packageName service:_serviceName method:method]; @@ -78,7 +78,7 @@ requestsWriter:requestsWriter responseClass:responseClass responsesWriteable:responsesWriteable - http2Method:http2Method]; + flags:flags]; } @end From d7aef05f24472977ee3a976d5321470132ec785c Mon Sep 17 00:00:00 2001 From: Muxi Yan <mxyan@google.com> Date: Wed, 28 Sep 2016 12:05:30 -0700 Subject: [PATCH 22/83] Readability improvement --- src/core/ext/transport/cronet/transport/cronet_transport.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.c b/src/core/ext/transport/cronet/transport/cronet_transport.c index 6c2b53f2917..6d5fe318ccb 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.c +++ b/src/core/ext/transport/cronet/transport/cronet_transport.c @@ -566,10 +566,10 @@ static void convert_metadata_to_cronet_headers( } if (mdelem->key == GRPC_MDSTR_METHOD) { if (mdelem->value == GRPC_MDSTR_PUT) { - *method = grpc_static_metadata_strings[74]; + *method = (const char*)mdelem->value->slice.data.refcounted.bytes; } else { /* POST method in default*/ - *method = grpc_static_metadata_strings[71]; + *method = (const char*)(GRPC_MDSTR_POST->slice.data.refcounted.bytes); } continue; } From a4aebf873e485a39084bb003c6978b3306874bf7 Mon Sep 17 00:00:00 2001 From: Yuchen Zeng <zyc@google.com> Date: Wed, 28 Sep 2016 13:20:18 -0700 Subject: [PATCH 23/83] Remove unused inclusion --- test/cpp/util/grpc_tool.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/test/cpp/util/grpc_tool.cc b/test/cpp/util/grpc_tool.cc index 8fb325cf76e..03c33abe9f5 100644 --- a/test/cpp/util/grpc_tool.cc +++ b/test/cpp/util/grpc_tool.cc @@ -52,7 +52,6 @@ #include "test/cpp/util/proto_file_parser.h" #include "test/cpp/util/proto_reflection_descriptor_database.h" #include "test/cpp/util/service_describer.h" -#include "test/cpp/util/test_config.h" namespace grpc { namespace testing { From 59c9f904bfe5a9cc7102b44c9440a5e93fdaf159 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" <roth@google.com> Date: Wed, 28 Sep 2016 13:33:21 -0700 Subject: [PATCH 24/83] Rename GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY to GRPC_INITIAL_METADATA_WAIT_FOR_READY. Also add a flag to indicate whether wait_for_ready was explicitly set by the application. --- include/grpc++/impl/codegen/client_context.h | 6 ++++-- include/grpc/impl/codegen/grpc_types.h | 13 ++++++++++--- src/core/ext/client_config/client_channel.c | 4 ++-- src/cpp/client/client_context.cc | 1 + 4 files changed, 17 insertions(+), 7 deletions(-) diff --git a/include/grpc++/impl/codegen/client_context.h b/include/grpc++/impl/codegen/client_context.h index faabddecc3c..dd37e6a8503 100644 --- a/include/grpc++/impl/codegen/client_context.h +++ b/include/grpc++/impl/codegen/client_context.h @@ -229,10 +229,11 @@ class ClientContext { /// EXPERIMENTAL: Trigger wait-for-ready or not on this request void set_wait_for_ready(bool wait_for_ready) { wait_for_ready_ = wait_for_ready; + wait_for_ready_explicitly_set_ = true; } /// DEPRECATED: Use set_wait_for_ready() instead. - void set_fail_fast(bool fail_fast) { wait_for_ready_ = !fail_fast; } + void set_fail_fast(bool fail_fast) { set_wait_for_ready(!fail_fast); } #ifndef GRPC_CXX0X_NO_CHRONO /// Return the deadline for the client call. @@ -352,7 +353,7 @@ class ClientContext { uint32_t initial_metadata_flags() const { return (idempotent_ ? GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST : 0) | - (wait_for_ready_ ? GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY : 0) | + (wait_for_ready_ ? GRPC_INITIAL_METADATA_WAIT_FOR_READY : 0) | (cacheable_ ? GRPC_INITIAL_METADATA_CACHEABLE_REQUEST : 0); } @@ -360,6 +361,7 @@ class ClientContext { bool initial_metadata_received_; bool wait_for_ready_; + bool wait_for_ready_explicitly_set_; bool idempotent_; bool cacheable_; std::shared_ptr<Channel> channel_; diff --git a/include/grpc/impl/codegen/grpc_types.h b/include/grpc/impl/codegen/grpc_types.h index 191cdd0e5fb..e9da7e8b715 100644 --- a/include/grpc/impl/codegen/grpc_types.h +++ b/include/grpc/impl/codegen/grpc_types.h @@ -254,15 +254,22 @@ typedef enum grpc_call_error { /** Signal that the call is idempotent */ #define GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST (0x00000010u) /** Signal that the call should not return UNAVAILABLE before it has started */ -#define GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY (0x00000020u) +#define GRPC_INITIAL_METADATA_WAIT_FOR_READY (0x00000020u) +/** DEPRECATED: for backward compatibility */ +#define GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY \ + GRPC_INITIAL_METADATA_WAIT_FOR_READY /** Signal that the call is cacheable. GRPC is free to use GET verb */ #define GRPC_INITIAL_METADATA_CACHEABLE_REQUEST (0x00000040u) +/** Signal that GRPC_INITIAL_METADATA_WAIT_FOR_READY was explicitly set + by the calling application. */ +#define GRPC_INITIAL_METADATA_WAIT_FOR_READY_EXPLICITLY_SET (0x00000080u) /** Mask of all valid flags */ #define GRPC_INITIAL_METADATA_USED_MASK \ (GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST | \ - GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY | \ - GRPC_INITIAL_METADATA_CACHEABLE_REQUEST) + GRPC_INITIAL_METADATA_WAIT_FOR_READY | \ + GRPC_INITIAL_METADATA_CACHEABLE_REQUEST | \ + GRPC_INITIAL_METADATA_WAIT_FOR_READY_EXPLICITLY_SET) /** A single metadata element */ typedef struct grpc_metadata { diff --git a/src/core/ext/client_config/client_channel.c b/src/core/ext/client_config/client_channel.c index b2b4fea83cd..2c8113c1dbc 100644 --- a/src/core/ext/client_config/client_channel.c +++ b/src/core/ext/client_config/client_channel.c @@ -110,10 +110,10 @@ static void set_channel_connectivity_state_locked(grpc_exec_ctx *exec_ctx, if ((state == GRPC_CHANNEL_TRANSIENT_FAILURE || state == GRPC_CHANNEL_SHUTDOWN) && chand->lb_policy != NULL) { - /* cancel fail-fast picks */ + /* cancel picks with wait_for_ready=false */ grpc_lb_policy_cancel_picks( exec_ctx, chand->lb_policy, - /* mask= */ GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY, + /* mask= */ GRPC_INITIAL_METADATA_WAIT_FOR_READY, /* check= */ 0); } grpc_connectivity_state_set(exec_ctx, &chand->state_tracker, state, error, diff --git a/src/cpp/client/client_context.cc b/src/cpp/client/client_context.cc index fb9a7c0459e..b6008f47b13 100644 --- a/src/cpp/client/client_context.cc +++ b/src/cpp/client/client_context.cc @@ -60,6 +60,7 @@ static ClientContext::GlobalCallbacks* g_client_callbacks = ClientContext::ClientContext() : initial_metadata_received_(false), wait_for_ready_(false), + wait_for_ready_explicitly_set_(false), idempotent_(false), cacheable_(false), call_(nullptr), From 62cfbea9083b153753dabb90093101b2ca69f321 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" <roth@google.com> Date: Wed, 28 Sep 2016 13:35:20 -0700 Subject: [PATCH 25/83] Update pending API cleanups doc. --- doc/core/pending_api_cleanups.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/core/pending_api_cleanups.md b/doc/core/pending_api_cleanups.md index a12e8a9dfbd..a0a960e5e24 100644 --- a/doc/core/pending_api_cleanups.md +++ b/doc/core/pending_api_cleanups.md @@ -15,3 +15,5 @@ number: `include/grpc/impl/codegen/grpc_types.h` (commit `af00d8b`) - remove `ServerBuilder::SetMaxMessageSize()` method from `include/grpc++/server_builder.h` (commit `6980362`) +- remove `GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY` macro from + `include/grpc/impl/codegen/grpc_types.h` (commit `59c9f90`) From 9deb09fa36c80e3b3235365aa707e6d8575296b0 Mon Sep 17 00:00:00 2001 From: Muxi Yan <mxyan@google.com> Date: Wed, 28 Sep 2016 16:03:04 -0700 Subject: [PATCH 26/83] Use NS_OPTIONS flags for ObjC API --- src/objective-c/GRPCClient/GRPCCall.h | 15 ++++++++++++++- src/objective-c/GRPCClient/GRPCCall.m | 4 ++-- src/objective-c/ProtoRPC/ProtoRPC.h | 2 +- src/objective-c/ProtoRPC/ProtoRPC.m | 4 ++-- src/objective-c/ProtoRPC/ProtoService.h | 3 ++- src/objective-c/ProtoRPC/ProtoService.m | 2 +- 6 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/objective-c/GRPCClient/GRPCCall.h b/src/objective-c/GRPCClient/GRPCCall.h index 9a58311fe16..1bf25e290d9 100644 --- a/src/objective-c/GRPCClient/GRPCCall.h +++ b/src/objective-c/GRPCClient/GRPCCall.h @@ -154,6 +154,19 @@ typedef NS_ENUM(NSUInteger, GRPCErrorCode) { GRPCErrorCodeDataLoss = 15, }; +/** + * Flags for options of a gRPC call + * + */ +typedef NS_OPTIONS(NSUInteger, GRPCCallFlags) { + /** Signal that the call is idempotent */ + GRPCFlagIdempotentRequest = 0x00000010, + /** Signal that the call should not return UNAVAILABLE before it has started */ + GRPCFlagIgnoreConnectivity = 0x00000020, + /** Signal that the call is cacheable. GRPC is free to use GET verb */ + GRPCFlagCacheableRequest = 0x00000040, +}; + /** * Keys used in |NSError|'s |userInfo| dictionary to store the response headers and trailers sent by * the server. @@ -230,7 +243,7 @@ extern id const kGRPCTrailersKey; - (instancetype)initWithHost:(NSString *)host path:(NSString *)path requestsWriter:(GRXWriter *)requestsWriter - flags:(uint32_t)flags NS_DESIGNATED_INITIALIZER; + flags:(GRPCCallFlags)flags NS_DESIGNATED_INITIALIZER; /** * Finishes the request side of this call, notifies the server that the RPC should be cancelled, and diff --git a/src/objective-c/GRPCClient/GRPCCall.m b/src/objective-c/GRPCClient/GRPCCall.m index 8a58db70514..eb7998ed975 100644 --- a/src/objective-c/GRPCClient/GRPCCall.m +++ b/src/objective-c/GRPCClient/GRPCCall.m @@ -75,7 +75,7 @@ NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey"; NSString *_host; NSString *_path; - uint32_t _flags; + GRPCCallFlags _flags; GRPCWrappedCall *_wrappedCall; GRPCConnectivityMonitor *_connectivityMonitor; @@ -123,7 +123,7 @@ NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey"; - (instancetype)initWithHost:(NSString *)host path:(NSString *)path requestsWriter:(GRXWriter *)requestWriter - flags:(uint32_t)flags { + flags:(GRPCCallFlags)flags { if (!host || !path) { [NSException raise:NSInvalidArgumentException format:@"Neither host nor path can be nil."]; } diff --git a/src/objective-c/ProtoRPC/ProtoRPC.h b/src/objective-c/ProtoRPC/ProtoRPC.h index 75c9b9bb3bc..b2c3e8def32 100644 --- a/src/objective-c/ProtoRPC/ProtoRPC.h +++ b/src/objective-c/ProtoRPC/ProtoRPC.h @@ -48,7 +48,7 @@ __attribute__((deprecated("Please use GRPCProtoCall."))) requestsWriter:(GRXWriter *)requestsWriter responseClass:(Class)responseClass responsesWriteable:(id<GRXWriteable>)responsesWriteable - flags:(uint32_t)flags NS_DESIGNATED_INITIALIZER; + flags:(GRPCCallFlags)flags NS_DESIGNATED_INITIALIZER; - (void)start; @end diff --git a/src/objective-c/ProtoRPC/ProtoRPC.m b/src/objective-c/ProtoRPC/ProtoRPC.m index 2514d616f16..16cb04a83bf 100644 --- a/src/objective-c/ProtoRPC/ProtoRPC.m +++ b/src/objective-c/ProtoRPC/ProtoRPC.m @@ -66,7 +66,7 @@ static NSError *ErrorForBadProto(id proto, Class expectedClass, NSError *parsing - (instancetype)initWithHost:(NSString *)host path:(NSString *)path requestsWriter:(GRXWriter *)requestsWriter - flags:(uint32_t)flags { + flags:(GRPCCallFlags)flags { [NSException raise:NSInvalidArgumentException format:@"Please use ProtoRPC's designated initializer instead."]; return nil; @@ -79,7 +79,7 @@ static NSError *ErrorForBadProto(id proto, Class expectedClass, NSError *parsing requestsWriter:(GRXWriter *)requestsWriter responseClass:(Class)responseClass responsesWriteable:(id<GRXWriteable>)responsesWriteable - flags:(uint32_t)flags { + flags:(GRPCCallFlags)flags { // Because we can't tell the type system to constrain the class, we need to check at runtime: if (![responseClass respondsToSelector:@selector(parseFromData:error:)]) { [NSException raise:NSInvalidArgumentException diff --git a/src/objective-c/ProtoRPC/ProtoService.h b/src/objective-c/ProtoRPC/ProtoService.h index 9a49ebd257b..0fd5764dbc0 100644 --- a/src/objective-c/ProtoRPC/ProtoService.h +++ b/src/objective-c/ProtoRPC/ProtoService.h @@ -32,6 +32,7 @@ */ #import <Foundation/Foundation.h> +#import "ProtoRPC.h" @class GRPCProtoCall; @protocol GRXWriteable; @@ -48,7 +49,7 @@ __attribute__((deprecated("Please use GRPCProtoService."))) requestsWriter:(GRXWriter *)requestsWriter responseClass:(Class)responseClass responsesWriteable:(id<GRXWriteable>)responsesWriteable - flags:(uint32_t)flags; + flags:(GRPCCallFlags)flags; @end diff --git a/src/objective-c/ProtoRPC/ProtoService.m b/src/objective-c/ProtoRPC/ProtoService.m index b237164a006..53337cd983e 100644 --- a/src/objective-c/ProtoRPC/ProtoService.m +++ b/src/objective-c/ProtoRPC/ProtoService.m @@ -69,7 +69,7 @@ requestsWriter:(GRXWriter *)requestsWriter responseClass:(Class)responseClass responsesWriteable:(id<GRXWriteable>)responsesWriteable - flags:(uint32_t)flags { + flags:(GRPCCallFlags)flags { GRPCProtoMethod *methodName = [[GRPCProtoMethod alloc] initWithPackage:_packageName service:_serviceName method:method]; From a40d4711104708152c56c1246a41eb9160e18df2 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" <roth@google.com> Date: Thu, 29 Sep 2016 10:05:50 -0700 Subject: [PATCH 27/83] Plumb through GRPC_INITIAL_METADATA_WAIT_FOR_READY_EXPLICITLY_SET. --- include/grpc++/impl/codegen/client_context.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/include/grpc++/impl/codegen/client_context.h b/include/grpc++/impl/codegen/client_context.h index dd37e6a8503..a330ed06bbe 100644 --- a/include/grpc++/impl/codegen/client_context.h +++ b/include/grpc++/impl/codegen/client_context.h @@ -354,7 +354,10 @@ class ClientContext { uint32_t initial_metadata_flags() const { return (idempotent_ ? GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST : 0) | (wait_for_ready_ ? GRPC_INITIAL_METADATA_WAIT_FOR_READY : 0) | - (cacheable_ ? GRPC_INITIAL_METADATA_CACHEABLE_REQUEST : 0); + (cacheable_ ? GRPC_INITIAL_METADATA_CACHEABLE_REQUEST : 0) | + (wait_for_ready_explicitly_set_ + ? GRPC_INITIAL_METADATA_WAIT_FOR_READY_EXPLICITLY_SET + : 0); } grpc::string authority() { return authority_; } From eb7574b39b2d3c1d3f077bdebe7e548eca283d6e Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" <roth@google.com> Date: Fri, 30 Sep 2016 10:48:19 -0700 Subject: [PATCH 28/83] clang-format --- include/grpc/impl/codegen/grpc_types.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/include/grpc/impl/codegen/grpc_types.h b/include/grpc/impl/codegen/grpc_types.h index 64602ea228c..ebeef038d1b 100644 --- a/include/grpc/impl/codegen/grpc_types.h +++ b/include/grpc/impl/codegen/grpc_types.h @@ -260,7 +260,7 @@ typedef enum grpc_call_error { #define GRPC_INITIAL_METADATA_WAIT_FOR_READY (0x00000020u) /** DEPRECATED: for backward compatibility */ #define GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY \ - GRPC_INITIAL_METADATA_WAIT_FOR_READY + GRPC_INITIAL_METADATA_WAIT_FOR_READY /** Signal that the call is cacheable. GRPC is free to use GET verb */ #define GRPC_INITIAL_METADATA_CACHEABLE_REQUEST (0x00000040u) /** Signal that GRPC_INITIAL_METADATA_WAIT_FOR_READY was explicitly set @@ -268,10 +268,10 @@ typedef enum grpc_call_error { #define GRPC_INITIAL_METADATA_WAIT_FOR_READY_EXPLICITLY_SET (0x00000080u) /** Mask of all valid flags */ -#define GRPC_INITIAL_METADATA_USED_MASK \ - (GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST | \ - GRPC_INITIAL_METADATA_WAIT_FOR_READY | \ - GRPC_INITIAL_METADATA_CACHEABLE_REQUEST | \ +#define GRPC_INITIAL_METADATA_USED_MASK \ + (GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST | \ + GRPC_INITIAL_METADATA_WAIT_FOR_READY | \ + GRPC_INITIAL_METADATA_CACHEABLE_REQUEST | \ GRPC_INITIAL_METADATA_WAIT_FOR_READY_EXPLICITLY_SET) /** A single metadata element */ From 92eb6b92d7ecb554d90d1f1f698ec1476160b6fc Mon Sep 17 00:00:00 2001 From: David Garcia Quintas <dgq@google.com> Date: Fri, 30 Sep 2016 14:07:39 -0700 Subject: [PATCH 29/83] Use call's deadline for internal grpclb call --- src/core/ext/client_config/client_channel.c | 6 +-- src/core/ext/client_config/lb_policy.h | 6 ++- src/core/ext/lb_policy/grpclb/grpclb.c | 55 +++++++++------------ 3 files changed, 31 insertions(+), 36 deletions(-) diff --git a/src/core/ext/client_config/client_channel.c b/src/core/ext/client_config/client_channel.c index feb4cbde7b5..0c1b2445bfb 100644 --- a/src/core/ext/client_config/client_channel.c +++ b/src/core/ext/client_config/client_channel.c @@ -602,9 +602,9 @@ static bool pick_subchannel(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, int r; GRPC_LB_POLICY_REF(lb_policy, "pick_subchannel"); gpr_mu_unlock(&chand->mu); - const grpc_lb_policy_pick_args inputs = {calld->pollent, initial_metadata, - initial_metadata_flags, - &calld->lb_token_mdelem}; + const grpc_lb_policy_pick_args inputs = { + calld->pollent, initial_metadata, initial_metadata_flags, + &calld->lb_token_mdelem, calld->deadline}; r = grpc_lb_policy_pick(exec_ctx, lb_policy, &inputs, connected_subchannel, NULL, on_ready); GRPC_LB_POLICY_UNREF(exec_ctx, lb_policy, "pick_subchannel"); diff --git a/src/core/ext/client_config/lb_policy.h b/src/core/ext/client_config/lb_policy.h index 7fb3e08cb3e..376bb2da631 100644 --- a/src/core/ext/client_config/lb_policy.h +++ b/src/core/ext/client_config/lb_policy.h @@ -59,10 +59,14 @@ typedef struct grpc_lb_policy_pick_args { grpc_polling_entity *pollent; /** Initial metadata associated with the picking call. */ grpc_metadata_batch *initial_metadata; - /** See \a GRPC_INITIAL_METADATA_* in grpc_types.h */ + /** Bitmask used for selective cancelling. See \a + * grpc_lb_policy_cancel_picks() and \a GRPC_INITIAL_METADATA_* in + * grpc_types.h */ uint32_t initial_metadata_flags; /** Storage for LB token in \a initial_metadata, or NULL if not used */ grpc_linked_mdelem *lb_token_mdelem_storage; + /** Deadline associated with the picking call. */ + gpr_timespec deadline; } grpc_lb_policy_pick_args; struct grpc_lb_policy_vtable { diff --git a/src/core/ext/lb_policy/grpclb/grpclb.c b/src/core/ext/lb_policy/grpclb/grpclb.c index bdfe65a62a4..b9307d18662 100644 --- a/src/core/ext/lb_policy/grpclb/grpclb.c +++ b/src/core/ext/lb_policy/grpclb/grpclb.c @@ -199,18 +199,8 @@ static void wrapped_rr_closure(grpc_exec_ctx *exec_ctx, void *arg, typedef struct pending_pick { struct pending_pick *next; - /* polling entity for the pick()'s async notification */ - grpc_polling_entity *pollent; - - /* the initial metadata for the pick. See grpc_lb_policy_pick() */ - grpc_metadata_batch *initial_metadata; - - /* storage for the lb token initial metadata mdelem */ - grpc_linked_mdelem *lb_token_mdelem_storage; - - /* bitmask passed to pick() and used for selective cancelling. See - * grpc_lb_policy_cancel_picks() */ - uint32_t initial_metadata_flags; + /* original pick()'s arguments */ + grpc_lb_policy_pick_args pick_args; /* output argument where to store the pick()ed connected subchannel, or NULL * upon error. */ @@ -232,11 +222,8 @@ static void add_pending_pick(pending_pick **root, memset(pp, 0, sizeof(pending_pick)); memset(&pp->wrapped_on_complete_arg, 0, sizeof(wrapped_rr_closure_arg)); pp->next = *root; - pp->pollent = pick_args->pollent; + pp->pick_args = *pick_args; pp->target = target; - pp->initial_metadata = pick_args->initial_metadata; - pp->initial_metadata_flags = pick_args->initial_metadata_flags; - pp->lb_token_mdelem_storage = pick_args->lb_token_mdelem_storage; pp->wrapped_on_complete_arg.wrapped_closure = on_complete; pp->wrapped_on_complete_arg.target = target; pp->wrapped_on_complete_arg.initial_metadata = pick_args->initial_metadata; @@ -283,9 +270,13 @@ typedef struct glb_lb_policy { /** mutex protecting remaining members */ gpr_mu mu; + /** who the client is trying to communicate with */ const char *server_name; grpc_client_channel_factory *cc_factory; + /** deadline for the original client's call */ + gpr_timespec deadline; + /** for communicating with the LB server */ grpc_channel *lb_channel; @@ -486,10 +477,8 @@ static void rr_handover(grpc_exec_ctx *exec_ctx, glb_lb_policy *glb_policy, gpr_log(GPR_INFO, "Pending pick about to PICK from 0x%" PRIxPTR "", (intptr_t)glb_policy->rr_policy); } - const grpc_lb_policy_pick_args pick_args = { - pp->pollent, pp->initial_metadata, pp->initial_metadata_flags, - pp->lb_token_mdelem_storage}; - grpc_lb_policy_pick(exec_ctx, glb_policy->rr_policy, &pick_args, pp->target, + grpc_lb_policy_pick(exec_ctx, glb_policy->rr_policy, &pp->pick_args, + pp->target, (void **)&pp->wrapped_on_complete_arg.lb_token, &pp->wrapped_on_complete); pp->wrapped_on_complete_arg.owning_pending_node = pp; @@ -698,7 +687,7 @@ static void glb_cancel_pick(grpc_exec_ctx *exec_ctx, grpc_lb_policy *pol, pending_pick *next = pp->next; if (pp->target == target) { grpc_polling_entity_del_from_pollset_set( - exec_ctx, pp->pollent, glb_policy->base.interested_parties); + exec_ctx, pp->pick_args.pollent, glb_policy->base.interested_parties); *target = NULL; grpc_exec_ctx_sched( exec_ctx, &pp->wrapped_on_complete, @@ -729,10 +718,10 @@ static void glb_cancel_picks(grpc_exec_ctx *exec_ctx, grpc_lb_policy *pol, glb_policy->pending_picks = NULL; while (pp != NULL) { pending_pick *next = pp->next; - if ((pp->initial_metadata_flags & initial_metadata_flags_mask) == + if ((pp->pick_args.initial_metadata_flags & initial_metadata_flags_mask) == initial_metadata_flags_eq) { grpc_polling_entity_del_from_pollset_set( - exec_ctx, pp->pollent, glb_policy->base.interested_parties); + exec_ctx, pp->pick_args.pollent, glb_policy->base.interested_parties); grpc_exec_ctx_sched( exec_ctx, &pp->wrapped_on_complete, GRPC_ERROR_CREATE_REFERENCING("Pick Cancelled", &error, 1), NULL); @@ -767,8 +756,6 @@ static int glb_pick(grpc_exec_ctx *exec_ctx, grpc_lb_policy *pol, const grpc_lb_policy_pick_args *pick_args, grpc_connected_subchannel **target, void **user_data, grpc_closure *on_complete) { - glb_lb_policy *glb_policy = (glb_lb_policy *)pol; - if (pick_args->lb_token_mdelem_storage == NULL) { *target = NULL; grpc_exec_ctx_sched( @@ -779,8 +766,10 @@ static int glb_pick(grpc_exec_ctx *exec_ctx, grpc_lb_policy *pol, return 1; } + glb_lb_policy *glb_policy = (glb_lb_policy *)pol; gpr_mu_lock(&glb_policy->mu); - int r; + glb_policy->deadline = pick_args->deadline; + bool pick_done; if (glb_policy->rr_policy != NULL) { if (grpc_lb_glb_trace) { @@ -799,10 +788,11 @@ static int glb_pick(grpc_exec_ctx *exec_ctx, grpc_lb_policy *pol, grpc_closure_init(&glb_policy->wrapped_on_complete, wrapped_rr_closure, &glb_policy->wc_arg); - r = grpc_lb_policy_pick(exec_ctx, glb_policy->rr_policy, pick_args, target, + pick_done = + grpc_lb_policy_pick(exec_ctx, glb_policy->rr_policy, pick_args, target, (void **)&glb_policy->wc_arg.lb_token, &glb_policy->wrapped_on_complete); - if (r != 0) { + if (pick_done) { /* synchronous grpc_lb_policy_pick call. Unref the RR policy. */ if (grpc_lb_glb_trace) { gpr_log(GPR_INFO, "Unreffing RR (0x%" PRIxPTR ")", @@ -815,6 +805,8 @@ static int glb_pick(grpc_exec_ctx *exec_ctx, grpc_lb_policy *pol, pick_args->initial_metadata, pick_args->lb_token_mdelem_storage, GRPC_MDELEM_REF(glb_policy->wc_arg.lb_token)); } + /* else, the pending pick will be registered and taken care of by the + * pending pick list inside the RR policy (glb_policy->rr_policy) */ } else { grpc_polling_entity_add_to_pollset_set(exec_ctx, pick_args->pollent, glb_policy->base.interested_parties); @@ -824,10 +816,10 @@ static int glb_pick(grpc_exec_ctx *exec_ctx, grpc_lb_policy *pol, if (!glb_policy->started_picking) { start_picking(exec_ctx, glb_policy); } - r = 0; + pick_done = false; } gpr_mu_unlock(&glb_policy->mu); - return r; + return pick_done; } static grpc_connectivity_state glb_check_connectivity( @@ -937,8 +929,7 @@ static lb_client_data *lb_client_data_create(glb_lb_policy *glb_policy) { grpc_closure_init(&lb_client->close_sent, close_sent_cb, lb_client); grpc_closure_init(&lb_client->srv_status_rcvd, srv_status_rcvd_cb, lb_client); - /* TODO(dgq): get the deadline from the parent channel. */ - lb_client->deadline = gpr_inf_future(GPR_CLOCK_MONOTONIC); + lb_client->deadline = glb_policy->deadline; /* Note the following LB call progresses every time there's activity in \a * glb_policy->base.interested_parties, which is comprised of the polling From bd57bba0c22b352478023bc2b43203f808a44399 Mon Sep 17 00:00:00 2001 From: David Garcia Quintas <dgq@google.com> Date: Fri, 30 Sep 2016 14:44:56 -0700 Subject: [PATCH 30/83] Use a reasonable deadline in grpclb test --- test/cpp/grpclb/grpclb_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cpp/grpclb/grpclb_test.cc b/test/cpp/grpclb/grpclb_test.cc index bbb983fc09c..5fc03721afc 100644 --- a/test/cpp/grpclb/grpclb_test.cc +++ b/test/cpp/grpclb/grpclb_test.cc @@ -460,7 +460,7 @@ static void perform_request(client_fixture *cf) { c = grpc_channel_create_call(cf->client, NULL, GRPC_PROPAGATE_DEFAULTS, cf->cq, "/foo", "foo.test.google.fr:1234", - GRPC_TIMEOUT_SECONDS_TO_DEADLINE(1000), NULL); + GRPC_TIMEOUT_SECONDS_TO_DEADLINE(5), NULL); gpr_log(GPR_INFO, "Call 0x%" PRIxPTR " created", (intptr_t)c); GPR_ASSERT(c); char *peer; From 22f7973179b42e36ba46105fc11fa19d25fd9d07 Mon Sep 17 00:00:00 2001 From: Muxi Yan <mxyan@google.com> Date: Fri, 30 Sep 2016 14:24:56 -0700 Subject: [PATCH 31/83] ObjC API update --- src/objective-c/GRPCClient/GRPCCall.h | 24 ++++++++------- src/objective-c/GRPCClient/GRPCCall.m | 41 ++++++++++++++++++------- src/objective-c/ProtoRPC/ProtoRPC.h | 3 +- src/objective-c/ProtoRPC/ProtoRPC.m | 9 ++---- src/objective-c/ProtoRPC/ProtoService.h | 3 +- src/objective-c/ProtoRPC/ProtoService.m | 6 ++-- 6 files changed, 50 insertions(+), 36 deletions(-) diff --git a/src/objective-c/GRPCClient/GRPCCall.h b/src/objective-c/GRPCClient/GRPCCall.h index 1bf25e290d9..4b28cade42e 100644 --- a/src/objective-c/GRPCClient/GRPCCall.h +++ b/src/objective-c/GRPCClient/GRPCCall.h @@ -158,13 +158,12 @@ typedef NS_ENUM(NSUInteger, GRPCErrorCode) { * Flags for options of a gRPC call * */ -typedef NS_OPTIONS(NSUInteger, GRPCCallFlags) { +typedef NS_ENUM(NSUInteger, GRPCCallAttr) { + GRPCCallAttrDefault = 0, /** Signal that the call is idempotent */ - GRPCFlagIdempotentRequest = 0x00000010, - /** Signal that the call should not return UNAVAILABLE before it has started */ - GRPCFlagIgnoreConnectivity = 0x00000020, + GRPCCallAttrIdempotentRequest = 1, /** Signal that the call is cacheable. GRPC is free to use GET verb */ - GRPCFlagCacheableRequest = 0x00000040, + GRPCCallAttrCacheableRequest = 2, }; /** @@ -238,12 +237,7 @@ extern id const kGRPCTrailersKey; */ - (instancetype)initWithHost:(NSString *)host path:(NSString *)path - requestsWriter:(GRXWriter *)requestsWriter; - -- (instancetype)initWithHost:(NSString *)host - path:(NSString *)path - requestsWriter:(GRXWriter *)requestsWriter - flags:(GRPCCallFlags)flags NS_DESIGNATED_INITIALIZER; + requestsWriter:(GRXWriter *)requestsWriter NS_DESIGNATED_INITIALIZER; /** * Finishes the request side of this call, notifies the server that the RPC should be cancelled, and @@ -251,6 +245,14 @@ extern id const kGRPCTrailersKey; */ - (void)cancel; +/** + * Set the call flag for a specific host path. + * + * Host parameter should not contain the scheme (http:// or https://), only the name or IP addr + * and the port number, for example @"localhost:5050". + */ ++ (void)setCallAttribute:(GRPCCallAttr)callAttr host:(NSString *)host path:(NSString *)path; + // TODO(jcanizales): Let specify a deadline. As a category of GRXWriter? @end diff --git a/src/objective-c/GRPCClient/GRPCCall.m b/src/objective-c/GRPCClient/GRPCCall.m index eb7998ed975..2c0b7799452 100644 --- a/src/objective-c/GRPCClient/GRPCCall.m +++ b/src/objective-c/GRPCClient/GRPCCall.m @@ -47,6 +47,7 @@ NSString * const kGRPCHeadersKey = @"io.grpc.HeadersKey"; NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey"; +static NSMutableDictionary *callFlags; @interface GRPCCall () <GRXWriteable> // Make them read-write. @@ -75,7 +76,6 @@ NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey"; NSString *_host; NSString *_path; - GRPCCallFlags _flags; GRPCWrappedCall *_wrappedCall; GRPCConnectivityMonitor *_connectivityMonitor; @@ -107,23 +107,43 @@ NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey"; // TODO(jcanizales): If grpc_init is idempotent, this should be changed from load to initialize. + (void)load { grpc_init(); + callFlags = [NSMutableDictionary dictionary]; } -- (instancetype)init { - return [self initWithHost:nil path:nil requestsWriter:nil flags:0]; ++ (void)setCallAttribute:(GRPCCallAttr)callAttr host:(NSString *)host path:(NSString *)path { + NSString *hostAndPath = [NSString stringWithFormat:@"%@%@", host, path]; + switch (callAttr) { + case GRPCCallAttrDefault: + callFlags[hostAndPath] = @0; + break; + case GRPCCallAttrIdempotentRequest: + callFlags[hostAndPath] = @GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST; + break; + case GRPCCallAttrCacheableRequest: + callFlags[hostAndPath] = @GRPC_INITIAL_METADATA_CACHEABLE_REQUEST; + break; + default: + break; + } } -- (instancetype)initWithHost:(NSString *)host - path:(NSString *)path - requestsWriter:(GRXWriter *)requestWriter{ - return [self initWithHost:host path:path requestsWriter:requestWriter flags:0]; ++ (uint32_t)getCallFlag:(NSString *)host path:(NSString *)path { + NSString *hostAndPath = [NSString stringWithFormat:@"%@%@", host, path]; + if (nil != [callFlags objectForKey:hostAndPath]) { + return [callFlags[hostAndPath] intValue]; + } else { + return 0; + } +} + +- (instancetype)init { + return [self initWithHost:nil path:nil requestsWriter:nil]; } // Designated initializer - (instancetype)initWithHost:(NSString *)host path:(NSString *)path - requestsWriter:(GRXWriter *)requestWriter - flags:(GRPCCallFlags)flags { + requestsWriter:(GRXWriter *)requestWriter { if (!host || !path) { [NSException raise:NSInvalidArgumentException format:@"Neither host nor path can be nil."]; } @@ -134,7 +154,6 @@ NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey"; if ((self = [super init])) { _host = [host copy]; _path = [path copy]; - _flags = flags; // Serial queue to invoke the non-reentrant methods of the grpc_call object. _callQueue = dispatch_queue_create("io.grpc.call", NULL); @@ -240,7 +259,7 @@ NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey"; - (void)sendHeaders:(NSDictionary *)headers { // TODO(jcanizales): Add error handlers for async failures [_wrappedCall startBatchWithOperations:@[[[GRPCOpSendMetadata alloc] initWithMetadata:headers - flags:_flags + flags:(uint32_t)[GRPCCall getCallFlag:_host path:_path] handler:nil]]]; } diff --git a/src/objective-c/ProtoRPC/ProtoRPC.h b/src/objective-c/ProtoRPC/ProtoRPC.h index b2c3e8def32..04fc1e45f16 100644 --- a/src/objective-c/ProtoRPC/ProtoRPC.h +++ b/src/objective-c/ProtoRPC/ProtoRPC.h @@ -47,8 +47,7 @@ __attribute__((deprecated("Please use GRPCProtoCall."))) method:(GRPCProtoMethod *)method requestsWriter:(GRXWriter *)requestsWriter responseClass:(Class)responseClass - responsesWriteable:(id<GRXWriteable>)responsesWriteable - flags:(GRPCCallFlags)flags NS_DESIGNATED_INITIALIZER; + responsesWriteable:(id<GRXWriteable>)responsesWriteable NS_DESIGNATED_INITIALIZER; - (void)start; @end diff --git a/src/objective-c/ProtoRPC/ProtoRPC.m b/src/objective-c/ProtoRPC/ProtoRPC.m index 16cb04a83bf..83d1b655e8d 100644 --- a/src/objective-c/ProtoRPC/ProtoRPC.m +++ b/src/objective-c/ProtoRPC/ProtoRPC.m @@ -65,8 +65,7 @@ static NSError *ErrorForBadProto(id proto, Class expectedClass, NSError *parsing #pragma clang diagnostic ignored "-Wobjc-designated-initializers" - (instancetype)initWithHost:(NSString *)host path:(NSString *)path - requestsWriter:(GRXWriter *)requestsWriter - flags:(GRPCCallFlags)flags { + requestsWriter:(GRXWriter *)requestsWriter { [NSException raise:NSInvalidArgumentException format:@"Please use ProtoRPC's designated initializer instead."]; return nil; @@ -78,8 +77,7 @@ static NSError *ErrorForBadProto(id proto, Class expectedClass, NSError *parsing method:(GRPCProtoMethod *)method requestsWriter:(GRXWriter *)requestsWriter responseClass:(Class)responseClass - responsesWriteable:(id<GRXWriteable>)responsesWriteable - flags:(GRPCCallFlags)flags { + responsesWriteable:(id<GRXWriteable>)responsesWriteable { // Because we can't tell the type system to constrain the class, we need to check at runtime: if (![responseClass respondsToSelector:@selector(parseFromData:error:)]) { [NSException raise:NSInvalidArgumentException @@ -93,8 +91,7 @@ static NSError *ErrorForBadProto(id proto, Class expectedClass, NSError *parsing } return [proto data]; }]; - if ((self = [super initWithHost:host path:method.HTTPPath requestsWriter:bytesWriter - flags:flags])) { + if ((self = [super initWithHost:host path:method.HTTPPath requestsWriter:bytesWriter])) { __weak ProtoRPC *weakSelf = self; // A writeable that parses the proto messages received. diff --git a/src/objective-c/ProtoRPC/ProtoService.h b/src/objective-c/ProtoRPC/ProtoService.h index 0fd5764dbc0..5a19fb35db0 100644 --- a/src/objective-c/ProtoRPC/ProtoService.h +++ b/src/objective-c/ProtoRPC/ProtoService.h @@ -48,8 +48,7 @@ __attribute__((deprecated("Please use GRPCProtoService."))) - (GRPCProtoCall *)RPCToMethod:(NSString *)method requestsWriter:(GRXWriter *)requestsWriter responseClass:(Class)responseClass - responsesWriteable:(id<GRXWriteable>)responsesWriteable - flags:(GRPCCallFlags)flags; + responsesWriteable:(id<GRXWriteable>)responsesWriteable; @end diff --git a/src/objective-c/ProtoRPC/ProtoService.m b/src/objective-c/ProtoRPC/ProtoService.m index 53337cd983e..3487fac59d3 100644 --- a/src/objective-c/ProtoRPC/ProtoService.m +++ b/src/objective-c/ProtoRPC/ProtoService.m @@ -68,8 +68,7 @@ - (GRPCProtoCall *)RPCToMethod:(NSString *)method requestsWriter:(GRXWriter *)requestsWriter responseClass:(Class)responseClass - responsesWriteable:(id<GRXWriteable>)responsesWriteable - flags:(GRPCCallFlags)flags { + responsesWriteable:(id<GRXWriteable>)responsesWriteable { GRPCProtoMethod *methodName = [[GRPCProtoMethod alloc] initWithPackage:_packageName service:_serviceName method:method]; @@ -77,8 +76,7 @@ method:methodName requestsWriter:requestsWriter responseClass:responseClass - responsesWriteable:responsesWriteable - flags:flags]; + responsesWriteable:responsesWriteable]; } @end From 1bd5c77a655ade2bcc39110996dcdec16209e105 Mon Sep 17 00:00:00 2001 From: Muxi Yan <mxyan@google.com> Date: Fri, 30 Sep 2016 15:13:58 -0700 Subject: [PATCH 32/83] Add idempotent test --- src/objective-c/tests/GRPCClientTests.m | 33 +++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/objective-c/tests/GRPCClientTests.m b/src/objective-c/tests/GRPCClientTests.m index 916a335802a..ce6ceee586a 100644 --- a/src/objective-c/tests/GRPCClientTests.m +++ b/src/objective-c/tests/GRPCClientTests.m @@ -317,4 +317,37 @@ static GRPCProtoMethod *kUnaryCallMethod; } +- (void)testIdempotentProtoRPC { + __weak XCTestExpectation *response = [self expectationWithDescription:@"Expected response."]; + __weak XCTestExpectation *completion = [self expectationWithDescription:@"RPC completed."]; + + RMTSimpleRequest *request = [RMTSimpleRequest message]; + request.responseSize = 100; + request.fillUsername = YES; + request.fillOauthScope = YES; + GRXWriter *requestsWriter = [GRXWriter writerWithValue:[request data]]; + + GRPCCall *call = [[GRPCCall alloc] initWithHost:kHostAddress + path:kUnaryCallMethod.HTTPPath + requestsWriter:requestsWriter]; + [GRPCCall setCallAttribute:GRPCCallAttrIdempotentRequest host:kHostAddress path:kUnaryCallMethod.HTTPPath]; + + id<GRXWriteable> responsesWriteable = [[GRXWriteable alloc] initWithValueHandler:^(NSData *value) { + XCTAssertNotNil(value, @"nil value received as response."); + XCTAssertGreaterThan(value.length, 0, @"Empty response received."); + RMTSimpleResponse *responseProto = [RMTSimpleResponse parseFromData:value error:NULL]; + // We expect empty strings, not nil: + XCTAssertNotNil(responseProto.username, @"Response's username is nil."); + XCTAssertNotNil(responseProto.oauthScope, @"Response's OAuth scope is nil."); + [response fulfill]; + } completionHandler:^(NSError *errorOrNil) { + XCTAssertNil(errorOrNil, @"Finished with unexpected error: %@", errorOrNil); + [completion fulfill]; + }]; + + [call startWithWriteable:responsesWriteable]; + + [self waitForExpectationsWithTimeout:8 handler:nil]; +} + @end From 461fed13cbc581e5593830b228a45e656163b012 Mon Sep 17 00:00:00 2001 From: Muxi Yan <mxyan@google.com> Date: Fri, 30 Sep 2016 16:25:35 -0700 Subject: [PATCH 33/83] Resolve memory leak in cronet_transport --- .../cronet/transport/cronet_transport.c | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.c b/src/core/ext/transport/cronet/transport/cronet_transport.c index 366690acf2b..1a731f5885c 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.c +++ b/src/core/ext/transport/cronet/transport/cronet_transport.c @@ -341,6 +341,11 @@ static void on_failed(cronet_bidirectional_stream *stream, int net_error) { gpr_free(s->state.ws.write_buffer); s->state.ws.write_buffer = NULL; } + if (s->state.rs.read_buffer && + s->state.rs.read_buffer != s->state.rs.grpc_header_bytes) { + gpr_free(s->state.rs.read_buffer); + s->state.rs.read_buffer = NULL; + } gpr_mu_unlock(&s->mu); execute_from_storage(s); } @@ -363,6 +368,11 @@ static void on_canceled(cronet_bidirectional_stream *stream) { gpr_free(s->state.ws.write_buffer); s->state.ws.write_buffer = NULL; } + if (s->state.rs.read_buffer && + s->state.rs.read_buffer != s->state.rs.grpc_header_bytes) { + gpr_free(s->state.rs.read_buffer); + s->state.rs.read_buffer = NULL; + } gpr_mu_unlock(&s->mu); execute_from_storage(s); } @@ -377,6 +387,11 @@ static void on_succeeded(cronet_bidirectional_stream *stream) { cronet_bidirectional_stream_destroy(s->cbs); s->state.state_callback_received[OP_SUCCEEDED] = true; s->cbs = NULL; + if (s->state.rs.read_buffer && + s->state.rs.read_buffer != s->state.rs.grpc_header_bytes) { + gpr_free(s->state.rs.read_buffer); + s->state.rs.read_buffer = NULL; + } gpr_mu_unlock(&s->mu); execute_from_storage(s); } @@ -901,6 +916,11 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, uint8_t *dst_p = GPR_SLICE_START_PTR(read_data_slice); memcpy(dst_p, stream_state->rs.read_buffer, (size_t)stream_state->rs.length_field); + if (stream_state.rs.read_buffer && + stream_state.rs.read_buffer != stream_state.rs.grpc_header_bytes) { + gpr_free(stream_state.rs.read_buffer); + stream_state.rs.read_buffer = NULL; + } gpr_slice_buffer_init(&stream_state->rs.read_slice_buffer); gpr_slice_buffer_add(&stream_state->rs.read_slice_buffer, read_data_slice); From 0814e3cb179513a1ef96141f51e410416c2fea62 Mon Sep 17 00:00:00 2001 From: Muxi Yan <mxyan@google.com> Date: Fri, 30 Sep 2016 16:28:24 -0700 Subject: [PATCH 34/83] Minor fixes --- src/core/ext/transport/cronet/transport/cronet_transport.c | 4 ++-- src/objective-c/ProtoRPC/ProtoService.h | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.c b/src/core/ext/transport/cronet/transport/cronet_transport.c index 6d5fe318ccb..6b0ed12b1e7 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.c +++ b/src/core/ext/transport/cronet/transport/cronet_transport.c @@ -566,10 +566,10 @@ static void convert_metadata_to_cronet_headers( } if (mdelem->key == GRPC_MDSTR_METHOD) { if (mdelem->value == GRPC_MDSTR_PUT) { - *method = (const char*)mdelem->value->slice.data.refcounted.bytes; + *method = "PUT"; } else { /* POST method in default*/ - *method = (const char*)(GRPC_MDSTR_POST->slice.data.refcounted.bytes); + *method = "POST"; } continue; } diff --git a/src/objective-c/ProtoRPC/ProtoService.h b/src/objective-c/ProtoRPC/ProtoService.h index 5a19fb35db0..7faae1b49c9 100644 --- a/src/objective-c/ProtoRPC/ProtoService.h +++ b/src/objective-c/ProtoRPC/ProtoService.h @@ -32,7 +32,6 @@ */ #import <Foundation/Foundation.h> -#import "ProtoRPC.h" @class GRPCProtoCall; @protocol GRXWriteable; From 20f49619ad4fc1f7ca25e72f4ff5527971186e68 Mon Sep 17 00:00:00 2001 From: Muxi Yan <mxyan@google.com> Date: Fri, 30 Sep 2016 16:51:58 -0700 Subject: [PATCH 35/83] Simplify the changes using macro --- .../cronet/transport/cronet_transport.c | 32 +++++++------------ 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.c b/src/core/ext/transport/cronet/transport/cronet_transport.c index 1a731f5885c..90f18e413d6 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.c +++ b/src/core/ext/transport/cronet/transport/cronet_transport.c @@ -57,6 +57,14 @@ if (grpc_cronet_trace) gpr_log(__VA_ARGS__); \ } while (0) +#define free_read_buffer(state_rs) \ + if ((state_rs).read_buffer && \ + (state_rs).read_buffer != (state_rs).grpc_header_bytes) { \ + gpr_free((state_rs).read_buffer); \ + (state_rs).read_buffer = NULL; \ + } + + /* TODO (makdharma): Hook up into the wider tracing mechanism */ int grpc_cronet_trace = 0; @@ -341,11 +349,7 @@ static void on_failed(cronet_bidirectional_stream *stream, int net_error) { gpr_free(s->state.ws.write_buffer); s->state.ws.write_buffer = NULL; } - if (s->state.rs.read_buffer && - s->state.rs.read_buffer != s->state.rs.grpc_header_bytes) { - gpr_free(s->state.rs.read_buffer); - s->state.rs.read_buffer = NULL; - } + free_read_buffer(s->state.rs); gpr_mu_unlock(&s->mu); execute_from_storage(s); } @@ -368,11 +372,7 @@ static void on_canceled(cronet_bidirectional_stream *stream) { gpr_free(s->state.ws.write_buffer); s->state.ws.write_buffer = NULL; } - if (s->state.rs.read_buffer && - s->state.rs.read_buffer != s->state.rs.grpc_header_bytes) { - gpr_free(s->state.rs.read_buffer); - s->state.rs.read_buffer = NULL; - } + free_read_buffer(s->state.rs); gpr_mu_unlock(&s->mu); execute_from_storage(s); } @@ -387,11 +387,7 @@ static void on_succeeded(cronet_bidirectional_stream *stream) { cronet_bidirectional_stream_destroy(s->cbs); s->state.state_callback_received[OP_SUCCEEDED] = true; s->cbs = NULL; - if (s->state.rs.read_buffer && - s->state.rs.read_buffer != s->state.rs.grpc_header_bytes) { - gpr_free(s->state.rs.read_buffer); - s->state.rs.read_buffer = NULL; - } + free_read_buffer(s->state.rs); gpr_mu_unlock(&s->mu); execute_from_storage(s); } @@ -916,11 +912,7 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, uint8_t *dst_p = GPR_SLICE_START_PTR(read_data_slice); memcpy(dst_p, stream_state->rs.read_buffer, (size_t)stream_state->rs.length_field); - if (stream_state.rs.read_buffer && - stream_state.rs.read_buffer != stream_state.rs.grpc_header_bytes) { - gpr_free(stream_state.rs.read_buffer); - stream_state.rs.read_buffer = NULL; - } + free_read_buffer(stream_state->rs); gpr_slice_buffer_init(&stream_state->rs.read_slice_buffer); gpr_slice_buffer_add(&stream_state->rs.read_slice_buffer, read_data_slice); From 9f85272dd378165344287a36d3547d0472ee626d Mon Sep 17 00:00:00 2001 From: David Garcia Quintas <dgq@google.com> Date: Fri, 30 Sep 2016 14:47:15 -0700 Subject: [PATCH 36/83] bugfixes from integration testing --- src/core/ext/lb_policy/grpclb/grpclb.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/core/ext/lb_policy/grpclb/grpclb.c b/src/core/ext/lb_policy/grpclb/grpclb.c index b9307d18662..1eafd2c961f 100644 --- a/src/core/ext/lb_policy/grpclb/grpclb.c +++ b/src/core/ext/lb_policy/grpclb/grpclb.c @@ -578,7 +578,7 @@ static grpc_lb_policy *glb_create(grpc_exec_ctx *exec_ctx, &addr_strs[addr_index++], (const struct sockaddr *)&args->addresses->addresses[i] .address.addr, - true) == 0); + true) > 0); } } } @@ -649,7 +649,6 @@ static void glb_shutdown(grpc_exec_ctx *exec_ctx, grpc_lb_policy *pol) { *pp->target = NULL; grpc_exec_ctx_sched(exec_ctx, &pp->wrapped_on_complete, GRPC_ERROR_NONE, NULL); - gpr_free(pp); pp = next; } @@ -692,7 +691,6 @@ static void glb_cancel_pick(grpc_exec_ctx *exec_ctx, grpc_lb_policy *pol, grpc_exec_ctx_sched( exec_ctx, &pp->wrapped_on_complete, GRPC_ERROR_CREATE_REFERENCING("Pick Cancelled", &error, 1), NULL); - gpr_free(pp); } else { pp->next = glb_policy->pending_picks; glb_policy->pending_picks = pp; @@ -725,7 +723,6 @@ static void glb_cancel_picks(grpc_exec_ctx *exec_ctx, grpc_lb_policy *pol, grpc_exec_ctx_sched( exec_ctx, &pp->wrapped_on_complete, GRPC_ERROR_CREATE_REFERENCING("Pick Cancelled", &error, 1), NULL); - gpr_free(pp); } else { pp->next = glb_policy->pending_picks; glb_policy->pending_picks = pp; From 6c0b960a45915227410d417bee5965712331bcf9 Mon Sep 17 00:00:00 2001 From: Muxi Yan <mxyan@google.com> Date: Sun, 2 Oct 2016 14:32:06 -0700 Subject: [PATCH 37/83] Name revision --- src/objective-c/GRPCClient/GRPCCall.h | 18 +++++++++--------- src/objective-c/GRPCClient/GRPCCall.m | 24 ++++++++++-------------- 2 files changed, 19 insertions(+), 23 deletions(-) diff --git a/src/objective-c/GRPCClient/GRPCCall.h b/src/objective-c/GRPCClient/GRPCCall.h index 4b28cade42e..7645bb1d34a 100644 --- a/src/objective-c/GRPCClient/GRPCCall.h +++ b/src/objective-c/GRPCClient/GRPCCall.h @@ -155,15 +155,15 @@ typedef NS_ENUM(NSUInteger, GRPCErrorCode) { }; /** - * Flags for options of a gRPC call - * + * Safety remark of a gRPC method as defined in RFC 2616 Section 9.1 */ -typedef NS_ENUM(NSUInteger, GRPCCallAttr) { - GRPCCallAttrDefault = 0, - /** Signal that the call is idempotent */ - GRPCCallAttrIdempotentRequest = 1, - /** Signal that the call is cacheable. GRPC is free to use GET verb */ - GRPCCallAttrCacheableRequest = 2, +typedef NS_ENUM(NSUInteger, GRPCCallSafety) { + /** Signal that there is no guarantees on how the call affects the server state. */ + GRPCCallSafetyDefault = 0, + /** Signal that the call is idempotent. gRPC is free to use PUT verb. */ + GRPCCallSafetyIdempotentRequest = 1, + /** Signal that the call is cacheable and will not affect server state. gRPC is free to use GET verb. */ + GRPCCallSafetyCacheableRequest = 2, }; /** @@ -251,7 +251,7 @@ extern id const kGRPCTrailersKey; * Host parameter should not contain the scheme (http:// or https://), only the name or IP addr * and the port number, for example @"localhost:5050". */ -+ (void)setCallAttribute:(GRPCCallAttr)callAttr host:(NSString *)host path:(NSString *)path; ++ (void)setCallSafety:(GRPCCallSafety)callSafety host:(NSString *)host path:(NSString *)path; // TODO(jcanizales): Let specify a deadline. As a category of GRXWriter? @end diff --git a/src/objective-c/GRPCClient/GRPCCall.m b/src/objective-c/GRPCClient/GRPCCall.m index 2c0b7799452..43204345f55 100644 --- a/src/objective-c/GRPCClient/GRPCCall.m +++ b/src/objective-c/GRPCClient/GRPCCall.m @@ -110,16 +110,16 @@ static NSMutableDictionary *callFlags; callFlags = [NSMutableDictionary dictionary]; } -+ (void)setCallAttribute:(GRPCCallAttr)callAttr host:(NSString *)host path:(NSString *)path { - NSString *hostAndPath = [NSString stringWithFormat:@"%@%@", host, path]; - switch (callAttr) { - case GRPCCallAttrDefault: ++ (void)setCallSafety:(GRPCCallSafety)callSafety host:(NSString *)host path:(NSString *)path { + NSString *hostAndPath = [NSString stringWithFormat:@"%@/%@", host, path]; + switch (callSafety) { + case GRPCCallSafetyDefault: callFlags[hostAndPath] = @0; break; - case GRPCCallAttrIdempotentRequest: + case GRPCCallSafetyIdempotentRequest: callFlags[hostAndPath] = @GRPC_INITIAL_METADATA_IDEMPOTENT_REQUEST; break; - case GRPCCallAttrCacheableRequest: + case GRPCCallSafetyCacheableRequest: callFlags[hostAndPath] = @GRPC_INITIAL_METADATA_CACHEABLE_REQUEST; break; default: @@ -127,13 +127,9 @@ static NSMutableDictionary *callFlags; } } -+ (uint32_t)getCallFlag:(NSString *)host path:(NSString *)path { - NSString *hostAndPath = [NSString stringWithFormat:@"%@%@", host, path]; - if (nil != [callFlags objectForKey:hostAndPath]) { - return [callFlags[hostAndPath] intValue]; - } else { - return 0; - } ++ (uint32_t)callFlagsForHost:(NSString *)host path:(NSString *)path { + NSString *hostAndPath = [NSString stringWithFormat:@"%@/%@", host, path]; + return [callFlags[hostAndPath] intValue]; } - (instancetype)init { @@ -259,7 +255,7 @@ static NSMutableDictionary *callFlags; - (void)sendHeaders:(NSDictionary *)headers { // TODO(jcanizales): Add error handlers for async failures [_wrappedCall startBatchWithOperations:@[[[GRPCOpSendMetadata alloc] initWithMetadata:headers - flags:(uint32_t)[GRPCCall getCallFlag:_host path:_path] + flags:[GRPCCall callFlagsForHost:_host path:_path] handler:nil]]]; } From 88405f70c16ab7a7679bb189b321bb565764e829 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" <roth@google.com> Date: Mon, 3 Oct 2016 08:24:52 -0700 Subject: [PATCH 38/83] Centralized logic for choosing the right LB policy. --- src/core/ext/client_config/client_channel.c | 32 ++++++++++++++++--- .../ext/resolver/dns/native/dns_resolver.c | 11 ++----- .../ext/resolver/sockaddr/sockaddr_resolver.c | 8 ++--- 3 files changed, 33 insertions(+), 18 deletions(-) diff --git a/src/core/ext/client_config/client_channel.c b/src/core/ext/client_config/client_channel.c index feb4cbde7b5..31f435ef625 100644 --- a/src/core/ext/client_config/client_channel.c +++ b/src/core/ext/client_config/client_channel.c @@ -185,10 +185,34 @@ static void on_resolver_result_changed(grpc_exec_ctx *exec_ctx, void *arg, lb_policy_args.additional_args = grpc_resolver_result_get_lb_policy_args(chand->resolver_result); lb_policy_args.client_channel_factory = chand->client_channel_factory; - lb_policy = grpc_lb_policy_create( - exec_ctx, - grpc_resolver_result_get_lb_policy_name(chand->resolver_result), - &lb_policy_args); + + // Special case: If all of the addresses are balancer addresses, + // assume that we should use the grpclb policy, regardless of what the + // resolver actually specified. + const char* lb_policy_name = + grpc_resolver_result_get_lb_policy_name(chand->resolver_result); + bool found_backend_address = false; + for (size_t i = 0; i < lb_policy_args.addresses->num_addresses; ++i) { + if (!lb_policy_args.addresses->addresses[i].is_balancer) { + found_backend_address = true; + break; + } + } + if (!found_backend_address) { + if (lb_policy_name != NULL && strcmp(lb_policy_name, "grpclb") != 0) { + gpr_log(GPR_INFO, + "resolver requested LB policy %s but provided only balancer " + "addresses, no backend addresses -- forcing use of grpclb LB " + "policy", (lb_policy_name == NULL ? "(none)" : lb_policy_name)); + } + lb_policy_name = "grpclb"; + } + // Use pick_first if nothing was specified and we didn't select grpclb + // above. + if (lb_policy_name == NULL) lb_policy_name = "pick_first"; + + lb_policy = + grpc_lb_policy_create(exec_ctx, lb_policy_name, &lb_policy_args); if (lb_policy != NULL) { GRPC_LB_POLICY_REF(lb_policy, "config_change"); GRPC_ERROR_UNREF(state_error); diff --git a/src/core/ext/resolver/dns/native/dns_resolver.c b/src/core/ext/resolver/dns/native/dns_resolver.c index e8ac1b12ae8..3908547893b 100644 --- a/src/core/ext/resolver/dns/native/dns_resolver.c +++ b/src/core/ext/resolver/dns/native/dns_resolver.c @@ -61,8 +61,6 @@ typedef struct { char *name_to_resolve; /** default port to use */ char *default_port; - /** load balancing policy name */ - char *lb_policy_name; /** mutex guarding the rest of the state */ gpr_mu mu; @@ -181,7 +179,7 @@ static void dns_on_resolved(grpc_exec_ctx *exec_ctx, void *arg, } grpc_resolved_addresses_destroy(r->addresses); result = grpc_resolver_result_create(r->target_name, addresses, - r->lb_policy_name, NULL); + NULL /* lb_policy_name */, NULL); } else { gpr_timespec now = gpr_now(GPR_CLOCK_MONOTONIC); gpr_timespec next_try = gpr_backoff_step(&r->backoff_state, now); @@ -245,13 +243,11 @@ static void dns_destroy(grpc_exec_ctx *exec_ctx, grpc_resolver *gr) { gpr_free(r->target_name); gpr_free(r->name_to_resolve); gpr_free(r->default_port); - gpr_free(r->lb_policy_name); gpr_free(r); } static grpc_resolver *dns_create(grpc_resolver_args *args, - const char *default_port, - const char *lb_policy_name) { + const char *default_port) { if (0 != strcmp(args->uri->authority, "")) { gpr_log(GPR_ERROR, "authority based dns uri's not supported"); return NULL; @@ -272,7 +268,6 @@ static grpc_resolver *dns_create(grpc_resolver_args *args, r->default_port = gpr_strdup(default_port); gpr_backoff_init(&r->backoff_state, BACKOFF_MULTIPLIER, BACKOFF_JITTER, BACKOFF_MIN_SECONDS * 1000, BACKOFF_MAX_SECONDS * 1000); - r->lb_policy_name = gpr_strdup(lb_policy_name); return &r->base; } @@ -286,7 +281,7 @@ static void dns_factory_unref(grpc_resolver_factory *factory) {} static grpc_resolver *dns_factory_create_resolver( grpc_resolver_factory *factory, grpc_resolver_args *args) { - return dns_create(args, "https", "pick_first"); + return dns_create(args, "https"); } static char *dns_factory_get_default_host_name(grpc_resolver_factory *factory, diff --git a/src/core/ext/resolver/sockaddr/sockaddr_resolver.c b/src/core/ext/resolver/sockaddr/sockaddr_resolver.c index f232e0460b6..d8c18057f78 100644 --- a/src/core/ext/resolver/sockaddr/sockaddr_resolver.c +++ b/src/core/ext/resolver/sockaddr/sockaddr_resolver.c @@ -164,7 +164,7 @@ char *unix_get_default_authority(grpc_resolver_factory *factory, static void do_nothing(void *ignored) {} static grpc_resolver *sockaddr_create( - grpc_resolver_args *args, const char *default_lb_policy_name, + grpc_resolver_args *args, int parse(grpc_uri *uri, struct sockaddr_storage *dst, size_t *len)) { bool errors_found = false; sockaddr_resolver *r; @@ -198,10 +198,6 @@ static grpc_resolver *sockaddr_create( abort(); } - if (r->lb_policy_name == NULL) { - r->lb_policy_name = gpr_strdup(default_lb_policy_name); - } - path_slice = gpr_slice_new(args->uri->path, strlen(args->uri->path), do_nothing); gpr_slice_buffer_init(&path_parts); @@ -251,7 +247,7 @@ static void sockaddr_factory_unref(grpc_resolver_factory *factory) {} #define DECL_FACTORY(name) \ static grpc_resolver *name##_factory_create_resolver( \ grpc_resolver_factory *factory, grpc_resolver_args *args) { \ - return sockaddr_create(args, "pick_first", parse_##name); \ + return sockaddr_create(args, parse_##name); \ } \ static const grpc_resolver_factory_vtable name##_factory_vtable = { \ sockaddr_factory_ref, sockaddr_factory_unref, \ From 8739e806fcc0358a404ff7e752fddb49898be409 Mon Sep 17 00:00:00 2001 From: Muxi Yan <mxyan@google.com> Date: Mon, 3 Oct 2016 09:41:31 -0700 Subject: [PATCH 39/83] Update test --- src/objective-c/tests/GRPCClientTests.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/objective-c/tests/GRPCClientTests.m b/src/objective-c/tests/GRPCClientTests.m index ce6ceee586a..77640525d58 100644 --- a/src/objective-c/tests/GRPCClientTests.m +++ b/src/objective-c/tests/GRPCClientTests.m @@ -330,7 +330,7 @@ static GRPCProtoMethod *kUnaryCallMethod; GRPCCall *call = [[GRPCCall alloc] initWithHost:kHostAddress path:kUnaryCallMethod.HTTPPath requestsWriter:requestsWriter]; - [GRPCCall setCallAttribute:GRPCCallAttrIdempotentRequest host:kHostAddress path:kUnaryCallMethod.HTTPPath]; + [GRPCCall setCallSafety:GRPCCallSafetyIdempotentRequest host:kHostAddress path:kUnaryCallMethod.HTTPPath]; id<GRXWriteable> responsesWriteable = [[GRXWriteable alloc] initWithValueHandler:^(NSData *value) { XCTAssertNotNil(value, @"nil value received as response."); From 92795c405c3961441182f29a9356cf589bb70190 Mon Sep 17 00:00:00 2001 From: Muxi Yan <mxyan@google.com> Date: Mon, 3 Oct 2016 09:53:38 -0700 Subject: [PATCH 40/83] Update free_read_buffer with a function --- .../cronet/transport/cronet_transport.c | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.c b/src/core/ext/transport/cronet/transport/cronet_transport.c index 90f18e413d6..b95aba14e9a 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.c +++ b/src/core/ext/transport/cronet/transport/cronet_transport.c @@ -57,14 +57,6 @@ if (grpc_cronet_trace) gpr_log(__VA_ARGS__); \ } while (0) -#define free_read_buffer(state_rs) \ - if ((state_rs).read_buffer && \ - (state_rs).read_buffer != (state_rs).grpc_header_bytes) { \ - gpr_free((state_rs).read_buffer); \ - (state_rs).read_buffer = NULL; \ - } - - /* TODO (makdharma): Hook up into the wider tracing mechanism */ int grpc_cronet_trace = 0; @@ -247,6 +239,14 @@ static const char *op_id_string(enum e_op_id i) { return "UNKNOWN"; } +static void free_read_buffer(stream_obj *s) { + if (s->state.rs.read_buffer && + s->state.rs.read_buffer != s->state.rs.grpc_header_bytes) { + gpr_free(s->state.rs.read_buffer); + s->state.rs.read_buffer = NULL; + } +} + /* Add a new stream op to op storage. */ @@ -349,7 +349,7 @@ static void on_failed(cronet_bidirectional_stream *stream, int net_error) { gpr_free(s->state.ws.write_buffer); s->state.ws.write_buffer = NULL; } - free_read_buffer(s->state.rs); + free_read_buffer(s); gpr_mu_unlock(&s->mu); execute_from_storage(s); } @@ -372,7 +372,7 @@ static void on_canceled(cronet_bidirectional_stream *stream) { gpr_free(s->state.ws.write_buffer); s->state.ws.write_buffer = NULL; } - free_read_buffer(s->state.rs); + free_read_buffer(s); gpr_mu_unlock(&s->mu); execute_from_storage(s); } @@ -387,7 +387,7 @@ static void on_succeeded(cronet_bidirectional_stream *stream) { cronet_bidirectional_stream_destroy(s->cbs); s->state.state_callback_received[OP_SUCCEEDED] = true; s->cbs = NULL; - free_read_buffer(s->state.rs); + free_read_buffer(s); gpr_mu_unlock(&s->mu); execute_from_storage(s); } @@ -912,7 +912,7 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, uint8_t *dst_p = GPR_SLICE_START_PTR(read_data_slice); memcpy(dst_p, stream_state->rs.read_buffer, (size_t)stream_state->rs.length_field); - free_read_buffer(stream_state->rs); + free_read_buffer(s); gpr_slice_buffer_init(&stream_state->rs.read_slice_buffer); gpr_slice_buffer_add(&stream_state->rs.read_slice_buffer, read_data_slice); From 6909922fdf504167dcf89c521eaf374a62705e20 Mon Sep 17 00:00:00 2001 From: David Garcia Quintas <dgq@google.com> Date: Mon, 3 Oct 2016 11:28:37 -0700 Subject: [PATCH 41/83] Change to use the max deadline across calls --- src/core/ext/lb_policy/grpclb/grpclb.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/core/ext/lb_policy/grpclb/grpclb.c b/src/core/ext/lb_policy/grpclb/grpclb.c index 1eafd2c961f..76cc08ecc9a 100644 --- a/src/core/ext/lb_policy/grpclb/grpclb.c +++ b/src/core/ext/lb_policy/grpclb/grpclb.c @@ -105,6 +105,7 @@ #include <grpc/support/alloc.h> #include <grpc/support/host_port.h> #include <grpc/support/string_util.h> +#include <grpc/support/time.h> #include "src/core/ext/client_config/client_channel_factory.h" #include "src/core/ext/client_config/lb_policy_factory.h" @@ -765,7 +766,10 @@ static int glb_pick(grpc_exec_ctx *exec_ctx, grpc_lb_policy *pol, glb_lb_policy *glb_policy = (glb_lb_policy *)pol; gpr_mu_lock(&glb_policy->mu); - glb_policy->deadline = pick_args->deadline; + /* use the longest deadline across incoming calls for the communication with + * the LB server */ + glb_policy->deadline = + gpr_time_max(pick_args->deadline, glb_policy->deadline); bool pick_done; if (glb_policy->rr_policy != NULL) { @@ -802,9 +806,9 @@ static int glb_pick(grpc_exec_ctx *exec_ctx, grpc_lb_policy *pol, pick_args->initial_metadata, pick_args->lb_token_mdelem_storage, GRPC_MDELEM_REF(glb_policy->wc_arg.lb_token)); } + } else { /* else, the pending pick will be registered and taken care of by the * pending pick list inside the RR policy (glb_policy->rr_policy) */ - } else { grpc_polling_entity_add_to_pollset_set(exec_ctx, pick_args->pollent, glb_policy->base.interested_parties); add_pending_pick(&glb_policy->pending_picks, pick_args, target, @@ -926,6 +930,7 @@ static lb_client_data *lb_client_data_create(glb_lb_policy *glb_policy) { grpc_closure_init(&lb_client->close_sent, close_sent_cb, lb_client); grpc_closure_init(&lb_client->srv_status_rcvd, srv_status_rcvd_cb, lb_client); + /* the longest deadline across incoming calls */ lb_client->deadline = glb_policy->deadline; /* Note the following LB call progresses every time there's activity in \a From da0ec8222e89f4d72e57a63b919cbfbc7a6de2d5 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" <roth@google.com> Date: Mon, 3 Oct 2016 11:32:04 -0700 Subject: [PATCH 42/83] Add fake resolver for tests. --- Makefile | 2 + build.yaml | 2 + .../ext/resolver/sockaddr/sockaddr_resolver.c | 25 +- test/core/client_config/lb_policies_test.c | 6 +- test/core/end2end/fake_resolver.c | 229 ++++++++++++++++++ test/core/end2end/fake_resolver.h | 39 +++ test/cpp/grpclb/grpclb_test.cc | 4 +- tools/run_tests/sources_and_headers.json | 3 + .../grpc_test_util/grpc_test_util.vcxproj | 3 + .../grpc_test_util.vcxproj.filters | 6 + .../grpc_test_util_unsecure.vcxproj | 3 + .../grpc_test_util_unsecure.vcxproj.filters | 6 + 12 files changed, 301 insertions(+), 27 deletions(-) create mode 100644 test/core/end2end/fake_resolver.c create mode 100644 test/core/end2end/fake_resolver.h diff --git a/Makefile b/Makefile index b98380be48b..fc0c38f3c7f 100644 --- a/Makefile +++ b/Makefile @@ -3058,6 +3058,7 @@ LIBGRPC_TEST_UTIL_SRC = \ test/core/end2end/data/test_root_cert.c \ test/core/security/oauth2_utils.c \ test/core/end2end/cq_verifier.c \ + test/core/end2end/fake_resolver.c \ test/core/end2end/fixtures/http_proxy.c \ test/core/end2end/fixtures/proxy.c \ test/core/iomgr/endpoint_tests.c \ @@ -3225,6 +3226,7 @@ endif LIBGRPC_TEST_UTIL_UNSECURE_SRC = \ test/core/end2end/cq_verifier.c \ + test/core/end2end/fake_resolver.c \ test/core/end2end/fixtures/http_proxy.c \ test/core/end2end/fixtures/proxy.c \ test/core/iomgr/endpoint_tests.c \ diff --git a/build.yaml b/build.yaml index 4746cc1a489..9e8b4c33043 100644 --- a/build.yaml +++ b/build.yaml @@ -507,6 +507,7 @@ filegroups: build: test headers: - test/core/end2end/cq_verifier.h + - test/core/end2end/fake_resolver.h - test/core/end2end/fixtures/http_proxy.h - test/core/end2end/fixtures/proxy.h - test/core/iomgr/endpoint_tests.h @@ -520,6 +521,7 @@ filegroups: - test/core/util/slice_splitter.h src: - test/core/end2end/cq_verifier.c + - test/core/end2end/fake_resolver.c - test/core/end2end/fixtures/http_proxy.c - test/core/end2end/fixtures/proxy.c - test/core/iomgr/endpoint_tests.c diff --git a/src/core/ext/resolver/sockaddr/sockaddr_resolver.c b/src/core/ext/resolver/sockaddr/sockaddr_resolver.c index d8c18057f78..d17a166850d 100644 --- a/src/core/ext/resolver/sockaddr/sockaddr_resolver.c +++ b/src/core/ext/resolver/sockaddr/sockaddr_resolver.c @@ -51,8 +51,6 @@ typedef struct { grpc_resolver base; /** refcount */ gpr_refcount refs; - /** load balancing policy name */ - char *lb_policy_name; /** the path component of the uri passed in */ char *target_name; /** the addresses that we've 'resolved' */ @@ -123,7 +121,7 @@ static void sockaddr_maybe_finish_next_locked(grpc_exec_ctx *exec_ctx, *r->target_result = grpc_resolver_result_create( r->target_name, grpc_lb_addresses_copy(r->addresses, NULL /* user_data_copy */), - r->lb_policy_name, NULL); + NULL /* lb_policy_name */, NULL); grpc_exec_ctx_sched(exec_ctx, r->next_completion, GRPC_ERROR_NONE, NULL); r->next_completion = NULL; } @@ -133,7 +131,6 @@ static void sockaddr_destroy(grpc_exec_ctx *exec_ctx, grpc_resolver *gr) { sockaddr_resolver *r = (sockaddr_resolver *)gr; gpr_mu_destroy(&r->mu); grpc_lb_addresses_destroy(r->addresses, NULL /* user_data_destroy */); - gpr_free(r->lb_policy_name); gpr_free(r->target_name); gpr_free(r); } @@ -180,24 +177,6 @@ static grpc_resolver *sockaddr_create( r = gpr_malloc(sizeof(sockaddr_resolver)); memset(r, 0, sizeof(*r)); - r->lb_policy_name = - gpr_strdup(grpc_uri_get_query_arg(args->uri, "lb_policy")); - const char *lb_enabled_qpart = - grpc_uri_get_query_arg(args->uri, "lb_enabled"); - /* anything other than "0" is interpreted as true */ - const bool lb_enabled = - (lb_enabled_qpart != NULL && (strcmp("0", lb_enabled_qpart) != 0)); - - if (r->lb_policy_name != NULL && strcmp("grpclb", r->lb_policy_name) == 0 && - !lb_enabled) { - /* we want grpclb but the "resolved" addresses aren't LB enabled. Bail - * out, as this is meant mostly for tests. */ - gpr_log(GPR_ERROR, - "Requested 'grpclb' LB policy but resolved addresses don't " - "support load balancing."); - abort(); - } - path_slice = gpr_slice_new(args->uri->path, strlen(args->uri->path), do_nothing); gpr_slice_buffer_init(&path_parts); @@ -214,7 +193,6 @@ static grpc_resolver *sockaddr_create( errors_found = true; } gpr_free(part_str); - r->addresses->addresses[i].is_balancer = lb_enabled; if (errors_found) break; } @@ -222,7 +200,6 @@ static grpc_resolver *sockaddr_create( gpr_slice_buffer_destroy(&path_parts); gpr_slice_unref(path_slice); if (errors_found) { - gpr_free(r->lb_policy_name); gpr_free(r->target_name); grpc_lb_addresses_destroy(r->addresses, NULL /* user_data_destroy */); gpr_free(r); diff --git a/test/core/client_config/lb_policies_test.c b/test/core/client_config/lb_policies_test.c index 0b9648b7e1c..fafff7bd69f 100644 --- a/test/core/client_config/lb_policies_test.c +++ b/test/core/client_config/lb_policies_test.c @@ -48,6 +48,7 @@ #include "src/core/lib/surface/channel.h" #include "src/core/lib/surface/server.h" #include "test/core/end2end/cq_verifier.h" +#include "test/core/end2end/fake_resolver.h" #include "test/core/util/port.h" #include "test/core/util/test_config.h" @@ -508,7 +509,7 @@ void run_spec(const test_spec *spec) { /* Create client. */ servers_hostports_str = gpr_strjoin_sep((const char **)f->servers_hostports, f->num_servers, ",", NULL); - gpr_asprintf(&client_hostport, "ipv4:%s?lb_policy=round_robin", + gpr_asprintf(&client_hostport, "test:%s?lb_policy=round_robin", servers_hostports_str); arg.type = GRPC_ARG_INTEGER; @@ -544,7 +545,7 @@ static grpc_channel *create_client(const servers_fixture *f) { servers_hostports_str = gpr_strjoin_sep((const char **)f->servers_hostports, f->num_servers, ",", NULL); - gpr_asprintf(&client_hostport, "ipv4:%s?lb_policy=round_robin", + gpr_asprintf(&client_hostport, "test:%s?lb_policy=round_robin", servers_hostports_str); arg.type = GRPC_ARG_INTEGER; @@ -874,6 +875,7 @@ int main(int argc, char **argv) { const size_t NUM_SERVERS = 4; grpc_test_init(argc, argv); + grpc_fake_resolver_init(); grpc_init(); grpc_tracer_set_enabled("round_robin", 1); diff --git a/test/core/end2end/fake_resolver.c b/test/core/end2end/fake_resolver.c new file mode 100644 index 00000000000..9b5e01b2a88 --- /dev/null +++ b/test/core/end2end/fake_resolver.c @@ -0,0 +1,229 @@ +// +// Copyright 2016, Google Inc. +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +// + +// This is similar to the sockaddr resolver, except that it supports a +// bunch of query args that are useful for dependency injection in tests. + +#include <stdbool.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> + +#include <grpc/support/alloc.h> +#include <grpc/support/host_port.h> +#include <grpc/support/port_platform.h> +#include <grpc/support/string_util.h> + +#include "src/core/ext/client_config/parse_address.h" +#include "src/core/ext/client_config/resolver_registry.h" +#include "src/core/lib/channel/channel_args.h" +#include "src/core/lib/iomgr/resolve_address.h" +#include "src/core/lib/iomgr/unix_sockets_posix.h" +#include "src/core/lib/support/string.h" + +// +// fake_resolver +// + +typedef struct { + // base class -- must be first + grpc_resolver base; + + gpr_refcount refs; + + // passed-in parameters + char* target_name; // the path component of the uri passed in + grpc_lb_addresses* addresses; + char* lb_policy_name; + + // mutex guarding the rest of the state + gpr_mu mu; + // have we published? + bool published; + // pending next completion, or NULL + grpc_closure* next_completion; + // target result address for next completion + grpc_resolver_result** target_result; +} fake_resolver; + +static void fake_resolver_destroy(grpc_exec_ctx* exec_ctx, grpc_resolver* gr) { + fake_resolver* r = (fake_resolver*)gr; + gpr_mu_destroy(&r->mu); + gpr_free(r->target_name); + grpc_lb_addresses_destroy(r->addresses, NULL /* user_data_destroy */); + gpr_free(r->lb_policy_name); + gpr_free(r); +} + +static void fake_resolver_shutdown(grpc_exec_ctx* exec_ctx, + grpc_resolver* resolver) { + fake_resolver* r = (fake_resolver*)resolver; + gpr_mu_lock(&r->mu); + if (r->next_completion != NULL) { + *r->target_result = NULL; + grpc_exec_ctx_sched(exec_ctx, r->next_completion, GRPC_ERROR_NONE, NULL); + r->next_completion = NULL; + } + gpr_mu_unlock(&r->mu); +} + +static void fake_resolver_maybe_finish_next_locked(grpc_exec_ctx* exec_ctx, + fake_resolver* r) { + if (r->next_completion != NULL && !r->published) { + r->published = true; + *r->target_result = grpc_resolver_result_create( + r->target_name, + grpc_lb_addresses_copy(r->addresses, NULL /* user_data_copy */), + r->lb_policy_name, NULL /* lb_policy_args */); + grpc_exec_ctx_sched(exec_ctx, r->next_completion, GRPC_ERROR_NONE, NULL); + r->next_completion = NULL; + } +} + +static void fake_resolver_channel_saw_error(grpc_exec_ctx* exec_ctx, + grpc_resolver* resolver) { + fake_resolver* r = (fake_resolver*)resolver; + gpr_mu_lock(&r->mu); + r->published = false; + fake_resolver_maybe_finish_next_locked(exec_ctx, r); + gpr_mu_unlock(&r->mu); +} + +static void fake_resolver_next(grpc_exec_ctx* exec_ctx, grpc_resolver* resolver, + grpc_resolver_result** target_result, + grpc_closure* on_complete) { + fake_resolver* r = (fake_resolver*)resolver; + gpr_mu_lock(&r->mu); + GPR_ASSERT(!r->next_completion); + r->next_completion = on_complete; + r->target_result = target_result; + fake_resolver_maybe_finish_next_locked(exec_ctx, r); + gpr_mu_unlock(&r->mu); +} + +static const grpc_resolver_vtable fake_resolver_vtable = { + fake_resolver_destroy, fake_resolver_shutdown, + fake_resolver_channel_saw_error, fake_resolver_next}; + +// +// fake_resolver_factory +// + +static void fake_resolver_factory_ref(grpc_resolver_factory* factory) {} + +static void fake_resolver_factory_unref(grpc_resolver_factory* factory) {} + +static void do_nothing(void* ignored) {} + +static grpc_resolver* fake_resolver_create(grpc_resolver_factory* factory, + grpc_resolver_args* args) { + if (0 != strcmp(args->uri->authority, "")) { + gpr_log(GPR_ERROR, "authority based uri's not supported by the %s scheme", + args->uri->scheme); + return NULL; + } + fake_resolver* r = gpr_malloc(sizeof(fake_resolver)); + memset(r, 0, sizeof(*r)); + r->target_name = gpr_strdup(args->uri->path); + // Initialize LB policy name. + r->lb_policy_name = + gpr_strdup(grpc_uri_get_query_arg(args->uri, "lb_policy")); + if (r->lb_policy_name == NULL) { + r->lb_policy_name = gpr_strdup("pick_first"); + } + // Get lb_enabled arg. + const char* lb_enabled_qpart = + grpc_uri_get_query_arg(args->uri, "lb_enabled"); + // Anything other than "0" is interpreted as true. + const bool lb_enabled = + lb_enabled_qpart != NULL && strcmp("0", lb_enabled_qpart) != 0; + if (strcmp("grpclb", r->lb_policy_name) == 0 && !lb_enabled) { + // we want grpclb but the "resolved" addresses aren't LB enabled. Bail + // out, as this is meant mostly for tests. + gpr_log(GPR_ERROR, + "Requested 'grpclb' LB policy but resolved addresses don't " + "support load balancing."); + abort(); + } + // Construct addresses. + gpr_slice path_slice = + gpr_slice_new(args->uri->path, strlen(args->uri->path), do_nothing); + gpr_slice_buffer path_parts; + gpr_slice_buffer_init(&path_parts); + gpr_slice_split(path_slice, ",", &path_parts); + r->addresses = grpc_lb_addresses_create(path_parts.count); + bool errors_found = false; + for (size_t i = 0; i < r->addresses->num_addresses; i++) { + grpc_uri ith_uri = *args->uri; + char* part_str = gpr_dump_slice(path_parts.slices[i], GPR_DUMP_ASCII); + ith_uri.path = part_str; + if (!parse_ipv4(&ith_uri, + (struct sockaddr_storage*)(&r->addresses->addresses[i] + .address.addr), + &r->addresses->addresses[i].address.len)) { + errors_found = true; + } + gpr_free(part_str); + r->addresses->addresses[i].is_balancer = lb_enabled; + if (errors_found) break; + } + gpr_slice_buffer_destroy(&path_parts); + gpr_slice_unref(path_slice); + if (errors_found) { + gpr_free(r->lb_policy_name); + gpr_free(r->target_name); + grpc_lb_addresses_destroy(r->addresses, NULL /* user_data_destroy */); + gpr_free(r); + return NULL; + } + gpr_ref_init(&r->refs, 1); + gpr_mu_init(&r->mu); + grpc_resolver_init(&r->base, &fake_resolver_vtable); + return &r->base; +} + +static char* fake_resolver_get_default_authority(grpc_resolver_factory* factory, + grpc_uri* uri) { + const char* path = uri->path; + if (path[0] == '/') ++path; + return gpr_strdup(path); +} + +static const grpc_resolver_factory_vtable fake_resolver_factory_vtable = { + fake_resolver_factory_ref, fake_resolver_factory_unref, + fake_resolver_create, fake_resolver_get_default_authority, "test"}; + +static grpc_resolver_factory fake_resolver_factory = { + &fake_resolver_factory_vtable}; + +void grpc_fake_resolver_init(void) { + grpc_register_resolver_type(&fake_resolver_factory); +} diff --git a/test/core/end2end/fake_resolver.h b/test/core/end2end/fake_resolver.h new file mode 100644 index 00000000000..7a30347f301 --- /dev/null +++ b/test/core/end2end/fake_resolver.h @@ -0,0 +1,39 @@ +// +// Copyright 2016, Google Inc. +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +// + +#ifndef GRPC_TEST_CORE_END2END_FAKE_RESOLVER_H +#define GRPC_TEST_CORE_END2END_FAKE_RESOLVER_H + +#include "test/core/util/test_config.h" + +void grpc_fake_resolver_init(); + +#endif /* GRPC_TEST_CORE_END2END_FAKE_RESOLVER_H */ diff --git a/test/cpp/grpclb/grpclb_test.cc b/test/cpp/grpclb/grpclb_test.cc index bbb983fc09c..41fa36880a6 100644 --- a/test/cpp/grpclb/grpclb_test.cc +++ b/test/cpp/grpclb/grpclb_test.cc @@ -59,6 +59,7 @@ extern "C" { #include "src/core/lib/surface/channel.h" #include "src/core/lib/surface/server.h" #include "test/core/end2end/cq_verifier.h" +#include "test/core/end2end/fake_resolver.h" #include "test/core/util/port.h" #include "test/core/util/test_config.h" } @@ -633,7 +634,7 @@ static test_fixture setup_test_fixture(int lb_server_update_delay_ms) { gpr_thd_new(&tf.lb_server.tid, fork_lb_server, &tf.lb_server, &options); char *server_uri; - gpr_asprintf(&server_uri, "ipv4:%s?lb_policy=grpclb&lb_enabled=1", + gpr_asprintf(&server_uri, "test:%s?lb_policy=grpclb&lb_enabled=1", tf.lb_server.servers_hostport); setup_client(server_uri, &tf.client); gpr_free(server_uri); @@ -716,6 +717,7 @@ TEST(GrpclbTest, InvalidAddressInServerlist) {} int main(int argc, char **argv) { ::testing::InitGoogleTest(&argc, argv); grpc_test_init(argc, argv); + grpc_fake_resolver_init(); grpc_init(); const auto result = RUN_ALL_TESTS(); grpc_shutdown(); diff --git a/tools/run_tests/sources_and_headers.json b/tools/run_tests/sources_and_headers.json index 03bdcd42645..bbdb998df39 100644 --- a/tools/run_tests/sources_and_headers.json +++ b/tools/run_tests/sources_and_headers.json @@ -6881,6 +6881,7 @@ ], "headers": [ "test/core/end2end/cq_verifier.h", + "test/core/end2end/fake_resolver.h", "test/core/end2end/fixtures/http_proxy.h", "test/core/end2end/fixtures/proxy.h", "test/core/iomgr/endpoint_tests.h", @@ -6899,6 +6900,8 @@ "src": [ "test/core/end2end/cq_verifier.c", "test/core/end2end/cq_verifier.h", + "test/core/end2end/fake_resolver.c", + "test/core/end2end/fake_resolver.h", "test/core/end2end/fixtures/http_proxy.c", "test/core/end2end/fixtures/http_proxy.h", "test/core/end2end/fixtures/proxy.c", diff --git a/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj b/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj index 21d9fc95296..b724c217ed4 100644 --- a/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj +++ b/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj @@ -176,6 +176,7 @@ <ClInclude Include="$(SolutionDir)\..\test\core\end2end\data\ssl_test_data.h" /> <ClInclude Include="$(SolutionDir)\..\test\core\security\oauth2_utils.h" /> <ClInclude Include="$(SolutionDir)\..\test\core\end2end\cq_verifier.h" /> + <ClInclude Include="$(SolutionDir)\..\test\core\end2end\fake_resolver.h" /> <ClInclude Include="$(SolutionDir)\..\test\core\end2end\fixtures\http_proxy.h" /> <ClInclude Include="$(SolutionDir)\..\test\core\end2end\fixtures\proxy.h" /> <ClInclude Include="$(SolutionDir)\..\test\core\iomgr\endpoint_tests.h" /> @@ -284,6 +285,8 @@ </ClCompile> <ClCompile Include="$(SolutionDir)\..\test\core\end2end\cq_verifier.c"> </ClCompile> + <ClCompile Include="$(SolutionDir)\..\test\core\end2end\fake_resolver.c"> + </ClCompile> <ClCompile Include="$(SolutionDir)\..\test\core\end2end\fixtures\http_proxy.c"> </ClCompile> <ClCompile Include="$(SolutionDir)\..\test\core\end2end\fixtures\proxy.c"> diff --git a/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj.filters b/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj.filters index 09a32482cd6..92806fa04a2 100644 --- a/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj.filters +++ b/vsprojects/vcxproj/grpc_test_util/grpc_test_util.vcxproj.filters @@ -19,6 +19,9 @@ <ClCompile Include="$(SolutionDir)\..\test\core\end2end\cq_verifier.c"> <Filter>test\core\end2end</Filter> </ClCompile> + <ClCompile Include="$(SolutionDir)\..\test\core\end2end\fake_resolver.c"> + <Filter>test\core\end2end</Filter> + </ClCompile> <ClCompile Include="$(SolutionDir)\..\test\core\end2end\fixtures\http_proxy.c"> <Filter>test\core\end2end\fixtures</Filter> </ClCompile> @@ -416,6 +419,9 @@ <ClInclude Include="$(SolutionDir)\..\test\core\end2end\cq_verifier.h"> <Filter>test\core\end2end</Filter> </ClInclude> + <ClInclude Include="$(SolutionDir)\..\test\core\end2end\fake_resolver.h"> + <Filter>test\core\end2end</Filter> + </ClInclude> <ClInclude Include="$(SolutionDir)\..\test\core\end2end\fixtures\http_proxy.h"> <Filter>test\core\end2end\fixtures</Filter> </ClInclude> diff --git a/vsprojects/vcxproj/grpc_test_util_unsecure/grpc_test_util_unsecure.vcxproj b/vsprojects/vcxproj/grpc_test_util_unsecure/grpc_test_util_unsecure.vcxproj index 04d1e584b5c..7878683f9eb 100644 --- a/vsprojects/vcxproj/grpc_test_util_unsecure/grpc_test_util_unsecure.vcxproj +++ b/vsprojects/vcxproj/grpc_test_util_unsecure/grpc_test_util_unsecure.vcxproj @@ -148,6 +148,7 @@ <ItemGroup> <ClInclude Include="$(SolutionDir)\..\test\core\end2end\cq_verifier.h" /> + <ClInclude Include="$(SolutionDir)\..\test\core\end2end\fake_resolver.h" /> <ClInclude Include="$(SolutionDir)\..\test\core\end2end\fixtures\http_proxy.h" /> <ClInclude Include="$(SolutionDir)\..\test\core\end2end\fixtures\proxy.h" /> <ClInclude Include="$(SolutionDir)\..\test\core\iomgr\endpoint_tests.h" /> @@ -163,6 +164,8 @@ <ItemGroup> <ClCompile Include="$(SolutionDir)\..\test\core\end2end\cq_verifier.c"> </ClCompile> + <ClCompile Include="$(SolutionDir)\..\test\core\end2end\fake_resolver.c"> + </ClCompile> <ClCompile Include="$(SolutionDir)\..\test\core\end2end\fixtures\http_proxy.c"> </ClCompile> <ClCompile Include="$(SolutionDir)\..\test\core\end2end\fixtures\proxy.c"> diff --git a/vsprojects/vcxproj/grpc_test_util_unsecure/grpc_test_util_unsecure.vcxproj.filters b/vsprojects/vcxproj/grpc_test_util_unsecure/grpc_test_util_unsecure.vcxproj.filters index 0f7072aa61d..2b20ab32fee 100644 --- a/vsprojects/vcxproj/grpc_test_util_unsecure/grpc_test_util_unsecure.vcxproj.filters +++ b/vsprojects/vcxproj/grpc_test_util_unsecure/grpc_test_util_unsecure.vcxproj.filters @@ -4,6 +4,9 @@ <ClCompile Include="$(SolutionDir)\..\test\core\end2end\cq_verifier.c"> <Filter>test\core\end2end</Filter> </ClCompile> + <ClCompile Include="$(SolutionDir)\..\test\core\end2end\fake_resolver.c"> + <Filter>test\core\end2end</Filter> + </ClCompile> <ClCompile Include="$(SolutionDir)\..\test\core\end2end\fixtures\http_proxy.c"> <Filter>test\core\end2end\fixtures</Filter> </ClCompile> @@ -45,6 +48,9 @@ <ClInclude Include="$(SolutionDir)\..\test\core\end2end\cq_verifier.h"> <Filter>test\core\end2end</Filter> </ClInclude> + <ClInclude Include="$(SolutionDir)\..\test\core\end2end\fake_resolver.h"> + <Filter>test\core\end2end</Filter> + </ClInclude> <ClInclude Include="$(SolutionDir)\..\test\core\end2end\fixtures\http_proxy.h"> <Filter>test\core\end2end\fixtures</Filter> </ClInclude> From b34b055a40e66c625dc6271081c4b1bf5efa25cb Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" <roth@google.com> Date: Mon, 3 Oct 2016 13:34:55 -0700 Subject: [PATCH 43/83] Code cleanup. --- .../ext/resolver/dns/native/dns_resolver.c | 3 -- .../ext/resolver/sockaddr/sockaddr_resolver.c | 39 ++++++---------- test/core/end2end/fake_resolver.c | 45 ++++++------------- 3 files changed, 28 insertions(+), 59 deletions(-) diff --git a/src/core/ext/resolver/dns/native/dns_resolver.c b/src/core/ext/resolver/dns/native/dns_resolver.c index 3908547893b..fa33ffd7bd5 100644 --- a/src/core/ext/resolver/dns/native/dns_resolver.c +++ b/src/core/ext/resolver/dns/native/dns_resolver.c @@ -53,8 +53,6 @@ typedef struct { /** base class: must be first */ grpc_resolver base; - /** refcount */ - gpr_refcount refs; /** target name */ char *target_name; /** name to resolve (usually the same as target_name) */ @@ -260,7 +258,6 @@ static grpc_resolver *dns_create(grpc_resolver_args *args, // Create resolver. dns_resolver *r = gpr_malloc(sizeof(dns_resolver)); memset(r, 0, sizeof(*r)); - gpr_ref_init(&r->refs, 1); gpr_mu_init(&r->mu); grpc_resolver_init(&r->base, &dns_resolver_vtable); r->target_name = gpr_strdup(path); diff --git a/src/core/ext/resolver/sockaddr/sockaddr_resolver.c b/src/core/ext/resolver/sockaddr/sockaddr_resolver.c index d17a166850d..5bb665e4163 100644 --- a/src/core/ext/resolver/sockaddr/sockaddr_resolver.c +++ b/src/core/ext/resolver/sockaddr/sockaddr_resolver.c @@ -49,8 +49,6 @@ typedef struct { /** base class: must be first */ grpc_resolver base; - /** refcount */ - gpr_refcount refs; /** the path component of the uri passed in */ char *target_name; /** the addresses that we've 'resolved' */ @@ -163,53 +161,44 @@ static void do_nothing(void *ignored) {} static grpc_resolver *sockaddr_create( grpc_resolver_args *args, int parse(grpc_uri *uri, struct sockaddr_storage *dst, size_t *len)) { - bool errors_found = false; - sockaddr_resolver *r; - gpr_slice path_slice; - gpr_slice_buffer path_parts; - if (0 != strcmp(args->uri->authority, "")) { gpr_log(GPR_ERROR, "authority based uri's not supported by the %s scheme", args->uri->scheme); return NULL; } - - r = gpr_malloc(sizeof(sockaddr_resolver)); - memset(r, 0, sizeof(*r)); - - path_slice = + /* Construct addresses. */ + gpr_slice path_slice = gpr_slice_new(args->uri->path, strlen(args->uri->path), do_nothing); + gpr_slice_buffer path_parts; gpr_slice_buffer_init(&path_parts); - gpr_slice_split(path_slice, ",", &path_parts); - r->addresses = grpc_lb_addresses_create(path_parts.count); - for (size_t i = 0; i < r->addresses->num_addresses; i++) { + grpc_lb_addresses *addresses = grpc_lb_addresses_create(path_parts.count); + bool errors_found = false; + for (size_t i = 0; i < addresses->num_addresses; i++) { grpc_uri ith_uri = *args->uri; char *part_str = gpr_dump_slice(path_parts.slices[i], GPR_DUMP_ASCII); ith_uri.path = part_str; - if (!parse(&ith_uri, (struct sockaddr_storage *)(&r->addresses->addresses[i] + if (!parse(&ith_uri, (struct sockaddr_storage *)(&addresses->addresses[i] .address.addr), - &r->addresses->addresses[i].address.len)) { + &addresses->addresses[i].address.len)) { errors_found = true; } gpr_free(part_str); if (errors_found) break; } - - r->target_name = gpr_strdup(args->uri->path); gpr_slice_buffer_destroy(&path_parts); gpr_slice_unref(path_slice); if (errors_found) { - gpr_free(r->target_name); - grpc_lb_addresses_destroy(r->addresses, NULL /* user_data_destroy */); - gpr_free(r); + grpc_lb_addresses_destroy(addresses, NULL /* user_data_destroy */); return NULL; } - - gpr_ref_init(&r->refs, 1); + /* Instantiate resolver. */ + sockaddr_resolver *r = gpr_malloc(sizeof(sockaddr_resolver)); + memset(r, 0, sizeof(*r)); + r->target_name = gpr_strdup(args->uri->path); + r->addresses = addresses; gpr_mu_init(&r->mu); grpc_resolver_init(&r->base, &sockaddr_resolver_vtable); - return &r->base; } diff --git a/test/core/end2end/fake_resolver.c b/test/core/end2end/fake_resolver.c index 9b5e01b2a88..1f7b4b60f57 100644 --- a/test/core/end2end/fake_resolver.c +++ b/test/core/end2end/fake_resolver.c @@ -57,8 +57,6 @@ typedef struct { // base class -- must be first grpc_resolver base; - gpr_refcount refs; - // passed-in parameters char* target_name; // the path component of the uri passed in grpc_lb_addresses* addresses; @@ -150,61 +148,46 @@ static grpc_resolver* fake_resolver_create(grpc_resolver_factory* factory, args->uri->scheme); return NULL; } - fake_resolver* r = gpr_malloc(sizeof(fake_resolver)); - memset(r, 0, sizeof(*r)); - r->target_name = gpr_strdup(args->uri->path); - // Initialize LB policy name. - r->lb_policy_name = - gpr_strdup(grpc_uri_get_query_arg(args->uri, "lb_policy")); - if (r->lb_policy_name == NULL) { - r->lb_policy_name = gpr_strdup("pick_first"); - } - // Get lb_enabled arg. + // Get lb_enabled arg. Anything other than "0" is interpreted as true. const char* lb_enabled_qpart = grpc_uri_get_query_arg(args->uri, "lb_enabled"); - // Anything other than "0" is interpreted as true. const bool lb_enabled = lb_enabled_qpart != NULL && strcmp("0", lb_enabled_qpart) != 0; - if (strcmp("grpclb", r->lb_policy_name) == 0 && !lb_enabled) { - // we want grpclb but the "resolved" addresses aren't LB enabled. Bail - // out, as this is meant mostly for tests. - gpr_log(GPR_ERROR, - "Requested 'grpclb' LB policy but resolved addresses don't " - "support load balancing."); - abort(); - } // Construct addresses. gpr_slice path_slice = gpr_slice_new(args->uri->path, strlen(args->uri->path), do_nothing); gpr_slice_buffer path_parts; gpr_slice_buffer_init(&path_parts); gpr_slice_split(path_slice, ",", &path_parts); - r->addresses = grpc_lb_addresses_create(path_parts.count); + grpc_lb_addresses* addresses = grpc_lb_addresses_create(path_parts.count); bool errors_found = false; - for (size_t i = 0; i < r->addresses->num_addresses; i++) { + for (size_t i = 0; i < addresses->num_addresses; i++) { grpc_uri ith_uri = *args->uri; char* part_str = gpr_dump_slice(path_parts.slices[i], GPR_DUMP_ASCII); ith_uri.path = part_str; if (!parse_ipv4(&ith_uri, - (struct sockaddr_storage*)(&r->addresses->addresses[i] + (struct sockaddr_storage*)(&addresses->addresses[i] .address.addr), - &r->addresses->addresses[i].address.len)) { + &addresses->addresses[i].address.len)) { errors_found = true; } gpr_free(part_str); - r->addresses->addresses[i].is_balancer = lb_enabled; + addresses->addresses[i].is_balancer = lb_enabled; if (errors_found) break; } gpr_slice_buffer_destroy(&path_parts); gpr_slice_unref(path_slice); if (errors_found) { - gpr_free(r->lb_policy_name); - gpr_free(r->target_name); - grpc_lb_addresses_destroy(r->addresses, NULL /* user_data_destroy */); - gpr_free(r); + grpc_lb_addresses_destroy(addresses, NULL /* user_data_destroy */); return NULL; } - gpr_ref_init(&r->refs, 1); + // Instantiate resolver. + fake_resolver* r = gpr_malloc(sizeof(fake_resolver)); + memset(r, 0, sizeof(*r)); + r->target_name = gpr_strdup(args->uri->path); + r->addresses = addresses; + r->lb_policy_name = + gpr_strdup(grpc_uri_get_query_arg(args->uri, "lb_policy")); gpr_mu_init(&r->mu); grpc_resolver_init(&r->base, &fake_resolver_vtable); return &r->base; From 5cf3c372ddafee6ee907e2b709e907d1dcd17f51 Mon Sep 17 00:00:00 2001 From: David Garcia Quintas <dgq@google.com> Date: Mon, 3 Oct 2016 14:30:03 -0700 Subject: [PATCH 44/83] Back to using inf future lor LB call deadline --- src/core/ext/client_config/client_channel.c | 4 +++- src/core/ext/client_config/lb_policy.h | 2 +- src/core/ext/lb_policy/grpclb/grpclb.c | 8 ++------ 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/core/ext/client_config/client_channel.c b/src/core/ext/client_config/client_channel.c index 0c1b2445bfb..3133869e22c 100644 --- a/src/core/ext/client_config/client_channel.c +++ b/src/core/ext/client_config/client_channel.c @@ -602,9 +602,11 @@ static bool pick_subchannel(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, int r; GRPC_LB_POLICY_REF(lb_policy, "pick_subchannel"); gpr_mu_unlock(&chand->mu); + // TODO(dgq): use deadline for service config instead of inf_future for the + // pick's deadline. const grpc_lb_policy_pick_args inputs = { calld->pollent, initial_metadata, initial_metadata_flags, - &calld->lb_token_mdelem, calld->deadline}; + &calld->lb_token_mdelem, gpr_inf_future(GPR_CLOCK_MONOTONIC)}; r = grpc_lb_policy_pick(exec_ctx, lb_policy, &inputs, connected_subchannel, NULL, on_ready); GRPC_LB_POLICY_UNREF(exec_ctx, lb_policy, "pick_subchannel"); diff --git a/src/core/ext/client_config/lb_policy.h b/src/core/ext/client_config/lb_policy.h index 376bb2da631..6cc3e1ebd38 100644 --- a/src/core/ext/client_config/lb_policy.h +++ b/src/core/ext/client_config/lb_policy.h @@ -65,7 +65,7 @@ typedef struct grpc_lb_policy_pick_args { uint32_t initial_metadata_flags; /** Storage for LB token in \a initial_metadata, or NULL if not used */ grpc_linked_mdelem *lb_token_mdelem_storage; - /** Deadline associated with the picking call. */ + /** Deadline for the call to the LB server */ gpr_timespec deadline; } grpc_lb_policy_pick_args; diff --git a/src/core/ext/lb_policy/grpclb/grpclb.c b/src/core/ext/lb_policy/grpclb/grpclb.c index 76cc08ecc9a..63af774ea6d 100644 --- a/src/core/ext/lb_policy/grpclb/grpclb.c +++ b/src/core/ext/lb_policy/grpclb/grpclb.c @@ -275,7 +275,7 @@ typedef struct glb_lb_policy { const char *server_name; grpc_client_channel_factory *cc_factory; - /** deadline for the original client's call */ + /** deadline for the LB's call */ gpr_timespec deadline; /** for communicating with the LB server */ @@ -766,10 +766,7 @@ static int glb_pick(grpc_exec_ctx *exec_ctx, grpc_lb_policy *pol, glb_lb_policy *glb_policy = (glb_lb_policy *)pol; gpr_mu_lock(&glb_policy->mu); - /* use the longest deadline across incoming calls for the communication with - * the LB server */ - glb_policy->deadline = - gpr_time_max(pick_args->deadline, glb_policy->deadline); + glb_policy->deadline = pick_args->deadline; bool pick_done; if (glb_policy->rr_policy != NULL) { @@ -930,7 +927,6 @@ static lb_client_data *lb_client_data_create(glb_lb_policy *glb_policy) { grpc_closure_init(&lb_client->close_sent, close_sent_cb, lb_client); grpc_closure_init(&lb_client->srv_status_rcvd, srv_status_rcvd_cb, lb_client); - /* the longest deadline across incoming calls */ lb_client->deadline = glb_policy->deadline; /* Note the following LB call progresses every time there's activity in \a From 61c5801465b4b81fe075e3282785d861d3f717c0 Mon Sep 17 00:00:00 2001 From: David Garcia Quintas <dgq@google.com> Date: Mon, 3 Oct 2016 14:44:20 -0700 Subject: [PATCH 45/83] improved todo --- src/core/ext/client_config/client_channel.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/core/ext/client_config/client_channel.c b/src/core/ext/client_config/client_channel.c index 3133869e22c..3b4747bebcf 100644 --- a/src/core/ext/client_config/client_channel.c +++ b/src/core/ext/client_config/client_channel.c @@ -602,8 +602,7 @@ static bool pick_subchannel(grpc_exec_ctx *exec_ctx, grpc_call_element *elem, int r; GRPC_LB_POLICY_REF(lb_policy, "pick_subchannel"); gpr_mu_unlock(&chand->mu); - // TODO(dgq): use deadline for service config instead of inf_future for the - // pick's deadline. + // TODO(dgq): make this deadline configurable somehow. const grpc_lb_policy_pick_args inputs = { calld->pollent, initial_metadata, initial_metadata_flags, &calld->lb_token_mdelem, gpr_inf_future(GPR_CLOCK_MONOTONIC)}; From af42900b3dc26672dc393c2eb3ea56fec9e0d960 Mon Sep 17 00:00:00 2001 From: Stanley Cheung <stanleycheung@google.com> Date: Tue, 4 Oct 2016 00:49:05 -0700 Subject: [PATCH 46/83] generate_projects.sh out of sync on master --- tools/run_tests/sources_and_headers.json | 1 + tools/run_tests/tests.json | 42 ++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/tools/run_tests/sources_and_headers.json b/tools/run_tests/sources_and_headers.json index 9492bcffda0..a760e0986aa 100644 --- a/tools/run_tests/sources_and_headers.json +++ b/tools/run_tests/sources_and_headers.json @@ -361,6 +361,7 @@ "grpc_test_util" ], "headers": [], + "is_filegroup": false, "language": "c", "name": "dns_resolver_connectivity_test", "src": [ diff --git a/tools/run_tests/tests.json b/tools/run_tests/tests.json index 65b227e2b82..c3395067c94 100644 --- a/tools/run_tests/tests.json +++ b/tools/run_tests/tests.json @@ -31479,6 +31479,27 @@ "posix" ] }, + { + "args": [ + "--scenarios_json", + "{\"scenarios\": [{\"name\": \"cpp_generic_async_streaming_ping_pong_secure\", \"warmup_seconds\": 0, \"benchmark_seconds\": 1, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 1, \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"server_type\": \"ASYNC_GENERIC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"client_channels\": 1, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 1, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}, \"histogram_params\": {\"max_possible\": 60000000000.0, \"resolution\": 0.01}}, \"num_clients\": 1}]}" + ], + "boringssl": true, + "ci_platforms": [ + "linux" + ], + "cpu_cost": 2, + "defaults": "boringssl", + "exclude_configs": [], + "flaky": false, + "language": "c++", + "name": "json_run_localhost", + "platforms": [ + "linux" + ], + "shortname": "json_run_localhost:cpp_generic_async_streaming_ping_pong_secure", + "timeout_seconds": 180 + }, { "args": [ "--scenarios_json", @@ -31689,6 +31710,27 @@ "shortname": "json_run_localhost:cpp_protobuf_async_streaming_qps_unconstrained_secure", "timeout_seconds": 180 }, + { + "args": [ + "--scenarios_json", + "{\"scenarios\": [{\"name\": \"cpp_generic_async_streaming_ping_pong_insecure\", \"warmup_seconds\": 0, \"benchmark_seconds\": 1, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 1, \"security_params\": null, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"server_type\": \"ASYNC_GENERIC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": null, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"client_channels\": 1, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 1, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}, \"histogram_params\": {\"max_possible\": 60000000000.0, \"resolution\": 0.01}}, \"num_clients\": 1}]}" + ], + "boringssl": true, + "ci_platforms": [ + "linux" + ], + "cpu_cost": 2, + "defaults": "boringssl", + "exclude_configs": [], + "flaky": false, + "language": "c++", + "name": "json_run_localhost", + "platforms": [ + "linux" + ], + "shortname": "json_run_localhost:cpp_generic_async_streaming_ping_pong_insecure", + "timeout_seconds": 180 + }, { "args": [ "--scenarios_json", From 6bc086102e4b681672070ecff9cc0b769604b6a8 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch <jtattermusch@google.com> Date: Tue, 4 Oct 2016 11:52:32 +0200 Subject: [PATCH 47/83] fixes for run_tests_in_workspace.sh --- tools/run_tests/run_tests_in_workspace.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/run_tests/run_tests_in_workspace.sh b/tools/run_tests/run_tests_in_workspace.sh index b0c9ad05f76..98ef3566db1 100755 --- a/tools/run_tests/run_tests_in_workspace.sh +++ b/tools/run_tests/run_tests_in_workspace.sh @@ -42,5 +42,5 @@ rm -rf "${WORKSPACE_NAME}" git clone --recursive . "${WORKSPACE_NAME}" echo "Running run_tests.py in workspace ${WORKSPACE_NAME}" -"${WORKSPACE_NAME}/tools/run_tests/run_tests.py" $@ +python "${WORKSPACE_NAME}/tools/run_tests/run_tests.py" $@ From 0b6d72c2961af504d7e40f3ac0a4238cd2ba82b5 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch <jtattermusch@google.com> Date: Tue, 4 Oct 2016 15:08:36 +0200 Subject: [PATCH 48/83] remove node portability targets --- tools/run_tests/run_tests_matrix.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/tools/run_tests/run_tests_matrix.py b/tools/run_tests/run_tests_matrix.py index d5f90478257..a94f9cfef5e 100755 --- a/tools/run_tests/run_tests_matrix.py +++ b/tools/run_tests/run_tests_matrix.py @@ -195,15 +195,6 @@ def _create_portability_test_jobs(extra_args=[]): compiler='coreclr', labels=['portability'], extra_args=extra_args) - - for compiler in ['node5', 'node0.12']: - test_jobs += _generate_jobs(languages=['node'], - configs=['dbg'], - platforms=['linux'], - arch='default', - compiler=compiler, - labels=['portability'], - extra_args=extra_args) return test_jobs From aa44062271a0009c4148ec32b92109263b2a7549 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch <jtattermusch@google.com> Date: Tue, 4 Oct 2016 17:52:15 +0200 Subject: [PATCH 49/83] add proxy script for run_tests_matrix.py --- tools/jenkins/run_jenkins_matrix.sh | 42 +++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100755 tools/jenkins/run_jenkins_matrix.sh diff --git a/tools/jenkins/run_jenkins_matrix.sh b/tools/jenkins/run_jenkins_matrix.sh new file mode 100755 index 00000000000..b3783e69584 --- /dev/null +++ b/tools/jenkins/run_jenkins_matrix.sh @@ -0,0 +1,42 @@ +#!/usr/bin/env bash +# Copyright 2015, Google Inc. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +# +# This script is invoked by Jenkins and triggers a test run, bypassing +# all args to the test script. +# +# Setting up rvm environment BEFORE we set -ex. +[[ -s /etc/profile.d/rvm.sh ]] && . /etc/profile.d/rvm.sh +# To prevent cygwin bash complaining about empty lines ending with \r +# we set the igncr option. The option doesn't exist on Linux, so we fallback +# to just 'set -ex' there. +# NOTE: No empty lines should appear in this file before igncr is set! +set -ex -o igncr || set -ex + +python tools/run_tests/run_tests_matrix.py $@ From 274c8ed001aca0dc563ee7e04ecd2f9a486b77ec Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" <roth@google.com> Date: Tue, 4 Oct 2016 09:21:42 -0700 Subject: [PATCH 50/83] Fix handling of max receive message size on client side. --- src/core/lib/channel/message_size_filter.c | 14 +- src/core/lib/surface/call.c | 15 +- test/core/end2end/tests/max_message_length.c | 165 ++++++++++++++++++- 3 files changed, 179 insertions(+), 15 deletions(-) diff --git a/src/core/lib/channel/message_size_filter.c b/src/core/lib/channel/message_size_filter.c index 02fc68fc3ae..f067a3a51c9 100644 --- a/src/core/lib/channel/message_size_filter.c +++ b/src/core/lib/channel/message_size_filter.c @@ -73,16 +73,22 @@ static void recv_message_ready(grpc_exec_ctx* exec_ctx, void* user_data, gpr_asprintf(&message_string, "Received message larger than max (%u vs. %d)", (*calld->recv_message)->length, chand->max_recv_size); - gpr_slice message = gpr_slice_from_copied_string(message_string); + grpc_error* new_error = grpc_error_set_int( + GRPC_ERROR_CREATE(message_string), GRPC_ERROR_INT_GRPC_STATUS, + GRPC_STATUS_INVALID_ARGUMENT); + if (error == GRPC_ERROR_NONE) { + error = new_error; + } else { + error = grpc_error_add_child(error, new_error); + GRPC_ERROR_UNREF(new_error); + } gpr_free(message_string); - grpc_call_element_send_close_with_message( - exec_ctx, elem, GRPC_STATUS_INVALID_ARGUMENT, &message); } // Invoke the next callback. grpc_exec_ctx_sched(exec_ctx, calld->next_recv_message_ready, error, NULL); } -// Start transport op. +// Start transport stream op. static void start_transport_stream_op(grpc_exec_ctx* exec_ctx, grpc_call_element* elem, grpc_transport_stream_op* op) { diff --git a/src/core/lib/surface/call.c b/src/core/lib/surface/call.c index 5690bcab1e4..b0f66f4f612 100644 --- a/src/core/lib/surface/call.c +++ b/src/core/lib/surface/call.c @@ -1103,8 +1103,8 @@ static void receiving_slice_ready(grpc_exec_ctx *exec_ctx, void *bctlp, } } -static void process_data_after_md(grpc_exec_ctx *exec_ctx, batch_control *bctl, - bool success) { +static void process_data_after_md(grpc_exec_ctx *exec_ctx, + batch_control *bctl) { grpc_call *call = bctl->call; if (call->receiving_stream == NULL) { *call->receiving_buffer = NULL; @@ -1124,8 +1124,6 @@ static void process_data_after_md(grpc_exec_ctx *exec_ctx, batch_control *bctl, grpc_closure_init(&call->receiving_slice_ready, receiving_slice_ready, bctl); continue_receiving_slices(exec_ctx, bctl); - /* early out */ - return; } } @@ -1133,12 +1131,17 @@ static void receiving_stream_ready(grpc_exec_ctx *exec_ctx, void *bctlp, grpc_error *error) { batch_control *bctl = bctlp; grpc_call *call = bctl->call; - + if (error != GRPC_ERROR_NONE) { + grpc_status_code status; + const char *msg; + grpc_error_get_status(error, &status, &msg); + close_with_status(exec_ctx, call, status, msg); + } gpr_mu_lock(&bctl->call->mu); if (bctl->call->has_initial_md_been_received || error != GRPC_ERROR_NONE || call->receiving_stream == NULL) { gpr_mu_unlock(&bctl->call->mu); - process_data_after_md(exec_ctx, bctlp, error); + process_data_after_md(exec_ctx, bctlp); } else { call->saved_receiving_stream_ready_bctlp = bctlp; gpr_mu_unlock(&bctl->call->mu); diff --git a/test/core/end2end/tests/max_message_length.c b/test/core/end2end/tests/max_message_length.c index cdca3e67487..d27ccedb4e3 100644 --- a/test/core/end2end/tests/max_message_length.c +++ b/test/core/end2end/tests/max_message_length.c @@ -98,9 +98,12 @@ static void end_test(grpc_end2end_test_fixture *f) { grpc_completion_queue_destroy(f->cq); } -static void test_max_message_length(grpc_end2end_test_config config, - bool send_limit) { - gpr_log(GPR_INFO, "testing with send_limit=%d", send_limit); +// Test with request larger than the limit. +// If send_limit is true, applies send limit on client; otherwise, applies +// recv limit on server. +static void test_max_message_length_on_request(grpc_end2end_test_config config, + bool send_limit) { + gpr_log(GPR_INFO, "testing request with send_limit=%d", send_limit); grpc_end2end_test_fixture f; grpc_arg channel_arg; @@ -239,9 +242,161 @@ done: config.tear_down_data(&f); } +// Test with response larger than the limit. +// If send_limit is true, applies send limit on server; otherwise, applies +// recv limit on client. +static void test_max_message_length_on_response(grpc_end2end_test_config config, + bool send_limit) { + gpr_log(GPR_INFO, "testing response with send_limit=%d", send_limit); + + grpc_end2end_test_fixture f; + grpc_arg channel_arg; + grpc_channel_args channel_args; + grpc_call *c = NULL; + grpc_call *s = NULL; + cq_verifier *cqv; + grpc_op ops[6]; + grpc_op *op; + gpr_slice response_payload_slice = + gpr_slice_from_copied_string("hello world"); + grpc_byte_buffer *response_payload = + grpc_raw_byte_buffer_create(&response_payload_slice, 1); + grpc_byte_buffer *recv_payload = NULL; + grpc_metadata_array initial_metadata_recv; + grpc_metadata_array trailing_metadata_recv; + grpc_metadata_array request_metadata_recv; + grpc_call_details call_details; + grpc_status_code status; + grpc_call_error error; + char *details = NULL; + size_t details_capacity = 0; + int was_cancelled = 2; + + channel_arg.key = send_limit ? GRPC_ARG_MAX_SEND_MESSAGE_LENGTH + : GRPC_ARG_MAX_RECEIVE_MESSAGE_LENGTH; + channel_arg.type = GRPC_ARG_INTEGER; + channel_arg.value.integer = 5; + + channel_args.num_args = 1; + channel_args.args = &channel_arg; + + f = begin_test(config, "test_max_message_length", + send_limit ? NULL : &channel_args, + send_limit ? &channel_args : NULL); + cqv = cq_verifier_create(f.cq); + + c = grpc_channel_create_call(f.client, NULL, GRPC_PROPAGATE_DEFAULTS, f.cq, + "/foo", "foo.test.google.fr:1234", + gpr_inf_future(GPR_CLOCK_REALTIME), NULL); + GPR_ASSERT(c); + + grpc_metadata_array_init(&initial_metadata_recv); + grpc_metadata_array_init(&trailing_metadata_recv); + grpc_metadata_array_init(&request_metadata_recv); + grpc_call_details_init(&call_details); + + memset(ops, 0, sizeof(ops)); + op = ops; + op->op = GRPC_OP_SEND_INITIAL_METADATA; + op->data.send_initial_metadata.count = 0; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_SEND_CLOSE_FROM_CLIENT; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_RECV_INITIAL_METADATA; + op->data.recv_initial_metadata = &initial_metadata_recv; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_RECV_MESSAGE; + op->data.recv_message = &recv_payload; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_RECV_STATUS_ON_CLIENT; + op->data.recv_status_on_client.trailing_metadata = &trailing_metadata_recv; + op->data.recv_status_on_client.status = &status; + op->data.recv_status_on_client.status_details = &details; + op->data.recv_status_on_client.status_details_capacity = &details_capacity; + op->flags = 0; + op->reserved = NULL; + op++; + error = grpc_call_start_batch(c, ops, (size_t)(op - ops), tag(1), NULL); + GPR_ASSERT(GRPC_CALL_OK == error); + + error = + grpc_server_request_call(f.server, &s, &call_details, + &request_metadata_recv, f.cq, f.cq, tag(101)); + GPR_ASSERT(GRPC_CALL_OK == error); + CQ_EXPECT_COMPLETION(cqv, tag(101), 1); + cq_verify(cqv); + + memset(ops, 0, sizeof(ops)); + op = ops; + op->op = GRPC_OP_SEND_INITIAL_METADATA; + op->data.send_initial_metadata.count = 0; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_RECV_CLOSE_ON_SERVER; + op->data.recv_close_on_server.cancelled = &was_cancelled; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_SEND_MESSAGE; + op->data.send_message = response_payload; + op->flags = 0; + op->reserved = NULL; + op++; + op->op = GRPC_OP_SEND_STATUS_FROM_SERVER; + op->data.send_status_from_server.trailing_metadata_count = 0; + op->data.send_status_from_server.status = GRPC_STATUS_OK; + op->data.send_status_from_server.status_details = "xyz"; + op->flags = 0; + op->reserved = NULL; + op++; + error = grpc_call_start_batch(s, ops, (size_t)(op - ops), tag(102), NULL); + GPR_ASSERT(GRPC_CALL_OK == error); + + CQ_EXPECT_COMPLETION(cqv, tag(102), 1); + CQ_EXPECT_COMPLETION(cqv, tag(1), 1); + cq_verify(cqv); + + GPR_ASSERT(0 == strcmp(call_details.method, "/foo")); + GPR_ASSERT(0 == strcmp(call_details.host, "foo.test.google.fr:1234")); + GPR_ASSERT(was_cancelled == 0); + + GPR_ASSERT(status == GRPC_STATUS_INVALID_ARGUMENT); + GPR_ASSERT(strcmp(details, + send_limit + ? "Sent message larger than max (11 vs. 5)" + : "Received message larger than max (11 vs. 5)") == 0); + + gpr_free(details); + grpc_metadata_array_destroy(&initial_metadata_recv); + grpc_metadata_array_destroy(&trailing_metadata_recv); + grpc_metadata_array_destroy(&request_metadata_recv); + grpc_call_details_destroy(&call_details); + grpc_byte_buffer_destroy(response_payload); + grpc_byte_buffer_destroy(recv_payload); + + grpc_call_destroy(c); + if (s != NULL) grpc_call_destroy(s); + + cq_verifier_destroy(cqv); + + end_test(&f); + config.tear_down_data(&f); +} + void max_message_length(grpc_end2end_test_config config) { - test_max_message_length(config, true); - test_max_message_length(config, false); + test_max_message_length_on_request(config, false /* send_limit */); + test_max_message_length_on_request(config, true /* send_limit */); + test_max_message_length_on_response(config, false /* send_limit */); + test_max_message_length_on_response(config, true /* send_limit */); } void max_message_length_pre_init(void) {} From 07aab59da9454c30b079849119562c58a40caa94 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" <roth@google.com> Date: Tue, 4 Oct 2016 10:02:00 -0700 Subject: [PATCH 51/83] clang-format --- src/core/ext/client_config/client_channel.c | 5 +++-- src/core/ext/resolver/sockaddr/sockaddr_resolver.c | 14 ++++++++------ test/core/end2end/fake_resolver.c | 8 ++++---- 3 files changed, 15 insertions(+), 12 deletions(-) diff --git a/src/core/ext/client_config/client_channel.c b/src/core/ext/client_config/client_channel.c index f3befe9b478..51f6f8e4078 100644 --- a/src/core/ext/client_config/client_channel.c +++ b/src/core/ext/client_config/client_channel.c @@ -189,7 +189,7 @@ static void on_resolver_result_changed(grpc_exec_ctx *exec_ctx, void *arg, // Special case: If all of the addresses are balancer addresses, // assume that we should use the grpclb policy, regardless of what the // resolver actually specified. - const char* lb_policy_name = + const char *lb_policy_name = grpc_resolver_result_get_lb_policy_name(chand->resolver_result); bool found_backend_address = false; for (size_t i = 0; i < lb_policy_args.addresses->num_addresses; ++i) { @@ -203,7 +203,8 @@ static void on_resolver_result_changed(grpc_exec_ctx *exec_ctx, void *arg, gpr_log(GPR_INFO, "resolver requested LB policy %s but provided only balancer " "addresses, no backend addresses -- forcing use of grpclb LB " - "policy", (lb_policy_name == NULL ? "(none)" : lb_policy_name)); + "policy", + (lb_policy_name == NULL ? "(none)" : lb_policy_name)); } lb_policy_name = "grpclb"; } diff --git a/src/core/ext/resolver/sockaddr/sockaddr_resolver.c b/src/core/ext/resolver/sockaddr/sockaddr_resolver.c index 5bb665e4163..5a7a32d7cb7 100644 --- a/src/core/ext/resolver/sockaddr/sockaddr_resolver.c +++ b/src/core/ext/resolver/sockaddr/sockaddr_resolver.c @@ -158,9 +158,10 @@ char *unix_get_default_authority(grpc_resolver_factory *factory, static void do_nothing(void *ignored) {} -static grpc_resolver *sockaddr_create( - grpc_resolver_args *args, - int parse(grpc_uri *uri, struct sockaddr_storage *dst, size_t *len)) { +static grpc_resolver *sockaddr_create(grpc_resolver_args *args, + int parse(grpc_uri *uri, + struct sockaddr_storage *dst, + size_t *len)) { if (0 != strcmp(args->uri->authority, "")) { gpr_log(GPR_ERROR, "authority based uri's not supported by the %s scheme", args->uri->scheme); @@ -178,9 +179,10 @@ static grpc_resolver *sockaddr_create( grpc_uri ith_uri = *args->uri; char *part_str = gpr_dump_slice(path_parts.slices[i], GPR_DUMP_ASCII); ith_uri.path = part_str; - if (!parse(&ith_uri, (struct sockaddr_storage *)(&addresses->addresses[i] - .address.addr), - &addresses->addresses[i].address.len)) { + if (!parse( + &ith_uri, + (struct sockaddr_storage *)(&addresses->addresses[i].address.addr), + &addresses->addresses[i].address.len)) { errors_found = true; } gpr_free(part_str); diff --git a/test/core/end2end/fake_resolver.c b/test/core/end2end/fake_resolver.c index 1f7b4b60f57..8a6624a49ab 100644 --- a/test/core/end2end/fake_resolver.c +++ b/test/core/end2end/fake_resolver.c @@ -165,10 +165,10 @@ static grpc_resolver* fake_resolver_create(grpc_resolver_factory* factory, grpc_uri ith_uri = *args->uri; char* part_str = gpr_dump_slice(path_parts.slices[i], GPR_DUMP_ASCII); ith_uri.path = part_str; - if (!parse_ipv4(&ith_uri, - (struct sockaddr_storage*)(&addresses->addresses[i] - .address.addr), - &addresses->addresses[i].address.len)) { + if (!parse_ipv4( + &ith_uri, + (struct sockaddr_storage*)(&addresses->addresses[i].address.addr), + &addresses->addresses[i].address.len)) { errors_found = true; } gpr_free(part_str); From 95c7143704794912094e7f19e6c595af076a543a Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" <roth@google.com> Date: Tue, 4 Oct 2016 10:03:05 -0700 Subject: [PATCH 52/83] Ran generate_projects.sh. --- tools/run_tests/sources_and_headers.json | 1 + tools/run_tests/tests.json | 42 ++++++++++++++++++++++++ 2 files changed, 43 insertions(+) diff --git a/tools/run_tests/sources_and_headers.json b/tools/run_tests/sources_and_headers.json index 1f6eaccb94c..ae5a9382c0b 100644 --- a/tools/run_tests/sources_and_headers.json +++ b/tools/run_tests/sources_and_headers.json @@ -361,6 +361,7 @@ "grpc_test_util" ], "headers": [], + "is_filegroup": false, "language": "c", "name": "dns_resolver_connectivity_test", "src": [ diff --git a/tools/run_tests/tests.json b/tools/run_tests/tests.json index 65b227e2b82..c3395067c94 100644 --- a/tools/run_tests/tests.json +++ b/tools/run_tests/tests.json @@ -31479,6 +31479,27 @@ "posix" ] }, + { + "args": [ + "--scenarios_json", + "{\"scenarios\": [{\"name\": \"cpp_generic_async_streaming_ping_pong_secure\", \"warmup_seconds\": 0, \"benchmark_seconds\": 1, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 1, \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"server_type\": \"ASYNC_GENERIC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": {\"use_test_ca\": true, \"server_host_override\": \"foo.test.google.fr\"}, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"client_channels\": 1, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 1, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}, \"histogram_params\": {\"max_possible\": 60000000000.0, \"resolution\": 0.01}}, \"num_clients\": 1}]}" + ], + "boringssl": true, + "ci_platforms": [ + "linux" + ], + "cpu_cost": 2, + "defaults": "boringssl", + "exclude_configs": [], + "flaky": false, + "language": "c++", + "name": "json_run_localhost", + "platforms": [ + "linux" + ], + "shortname": "json_run_localhost:cpp_generic_async_streaming_ping_pong_secure", + "timeout_seconds": 180 + }, { "args": [ "--scenarios_json", @@ -31689,6 +31710,27 @@ "shortname": "json_run_localhost:cpp_protobuf_async_streaming_qps_unconstrained_secure", "timeout_seconds": 180 }, + { + "args": [ + "--scenarios_json", + "{\"scenarios\": [{\"name\": \"cpp_generic_async_streaming_ping_pong_insecure\", \"warmup_seconds\": 0, \"benchmark_seconds\": 1, \"num_servers\": 1, \"server_config\": {\"async_server_threads\": 1, \"core_limit\": 1, \"security_params\": null, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"server_type\": \"ASYNC_GENERIC_SERVER\"}, \"client_config\": {\"client_type\": \"ASYNC_CLIENT\", \"security_params\": null, \"payload_config\": {\"bytebuf_params\": {\"resp_size\": 0, \"req_size\": 0}}, \"client_channels\": 1, \"async_client_threads\": 1, \"outstanding_rpcs_per_channel\": 1, \"rpc_type\": \"STREAMING\", \"load_params\": {\"closed_loop\": {}}, \"histogram_params\": {\"max_possible\": 60000000000.0, \"resolution\": 0.01}}, \"num_clients\": 1}]}" + ], + "boringssl": true, + "ci_platforms": [ + "linux" + ], + "cpu_cost": 2, + "defaults": "boringssl", + "exclude_configs": [], + "flaky": false, + "language": "c++", + "name": "json_run_localhost", + "platforms": [ + "linux" + ], + "shortname": "json_run_localhost:cpp_generic_async_streaming_ping_pong_insecure", + "timeout_seconds": 180 + }, { "args": [ "--scenarios_json", From 53ab32be86d0747af636070674f5830123c5a45c Mon Sep 17 00:00:00 2001 From: David Garcia Quintas <dgq@google.com> Date: Tue, 4 Oct 2016 13:09:36 -0700 Subject: [PATCH 53/83] added missing line to UDP server --- src/core/lib/iomgr/udp_server.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/core/lib/iomgr/udp_server.c b/src/core/lib/iomgr/udp_server.c index 12e929fa6a8..6650ef022df 100644 --- a/src/core/lib/iomgr/udp_server.c +++ b/src/core/lib/iomgr/udp_server.c @@ -199,6 +199,7 @@ void grpc_udp_server_destroy(grpc_exec_ctx *exec_ctx, grpc_udp_server *s, /* shutdown all fd's */ if (s->active_ports) { for (i = 0; i < s->nports; i++) { + server_port *sp = &s->ports[i]; /* Call the orphan_cb to signal that the FD is about to be closed and * should no longer be used. */ GPR_ASSERT(sp->orphan_cb); From 84350e167d4bc4f7c96af71baf28b5a5a0807856 Mon Sep 17 00:00:00 2001 From: Nathaniel Manista <nathaniel@google.com> Date: Tue, 4 Oct 2016 20:25:53 +0000 Subject: [PATCH 54/83] Drop unnecessary return statement --- src/python/grpcio/grpc/_server.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/python/grpcio/grpc/_server.py b/src/python/grpcio/grpc/_server.py index 94a13bfb2fb..f70cd2afa55 100644 --- a/src/python/grpcio/grpc/_server.py +++ b/src/python/grpcio/grpc/_server.py @@ -462,7 +462,6 @@ def _unary_response_in_pool( rpc_event, state, response, response_serializer) if serialized_response is not None: _status(rpc_event, state, serialized_response) - return def _stream_response_in_pool( From f042481403cd35669f5efc7aff9b42036ab7f862 Mon Sep 17 00:00:00 2001 From: David Garcia Quintas <dgq@google.com> Date: Tue, 4 Oct 2016 13:29:52 -0700 Subject: [PATCH 55/83] s/std::string/grpc::string --- test/cpp/util/proto_file_parser.cc | 2 +- test/cpp/util/proto_reflection_descriptor_database.cc | 4 ++-- test/cpp/util/proto_reflection_descriptor_database.h | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/test/cpp/util/proto_file_parser.cc b/test/cpp/util/proto_file_parser.cc index 01acb015325..98dd3f14ad4 100644 --- a/test/cpp/util/proto_file_parser.cc +++ b/test/cpp/util/proto_file_parser.cc @@ -82,7 +82,7 @@ ProtoFileParser::ProtoFileParser(std::shared_ptr<grpc::Channel> channel, const grpc::string& proto_path, const grpc::string& protofiles) : has_error_(false) { - std::vector<std::string> service_list; + std::vector<grpc::string> service_list; if (channel) { reflection_db_.reset(new grpc::ProtoReflectionDescriptorDatabase(channel)); reflection_db_->GetServices(&service_list); diff --git a/test/cpp/util/proto_reflection_descriptor_database.cc b/test/cpp/util/proto_reflection_descriptor_database.cc index f0d14c686a3..b60f447e377 100644 --- a/test/cpp/util/proto_reflection_descriptor_database.cc +++ b/test/cpp/util/proto_reflection_descriptor_database.cc @@ -255,7 +255,7 @@ bool ProtoReflectionDescriptorDatabase::FindAllExtensionNumbers( } bool ProtoReflectionDescriptorDatabase::GetServices( - std::vector<std::string>* output) { + std::vector<grpc::string>* output) { ServerReflectionRequest request; request.set_list_services(""); ServerReflectionResponse response; @@ -288,7 +288,7 @@ bool ProtoReflectionDescriptorDatabase::GetServices( const protobuf::FileDescriptorProto ProtoReflectionDescriptorDatabase::ParseFileDescriptorProtoResponse( - const std::string& byte_fd_proto) { + const grpc::string& byte_fd_proto) { protobuf::FileDescriptorProto file_desc_proto; file_desc_proto.ParseFromString(byte_fd_proto); return file_desc_proto; diff --git a/test/cpp/util/proto_reflection_descriptor_database.h b/test/cpp/util/proto_reflection_descriptor_database.h index 0e69696d5fb..471c9618dad 100644 --- a/test/cpp/util/proto_reflection_descriptor_database.h +++ b/test/cpp/util/proto_reflection_descriptor_database.h @@ -95,7 +95,7 @@ class ProtoReflectionDescriptorDatabase : public protobuf::DescriptorDatabase { std::vector<int>* output) GRPC_OVERRIDE; // Provide a list of full names of registered services - bool GetServices(std::vector<std::string>* output); + bool GetServices(std::vector<grpc::string>* output); private: typedef ClientReaderWriter< @@ -104,7 +104,7 @@ class ProtoReflectionDescriptorDatabase : public protobuf::DescriptorDatabase { ClientStream; const protobuf::FileDescriptorProto ParseFileDescriptorProtoResponse( - const std::string& byte_fd_proto); + const grpc::string& byte_fd_proto); void AddFileFromResponse( const grpc::reflection::v1alpha::FileDescriptorResponse& response); From e5b4c26ef7d1a695f8d925b1d4d481853c98f188 Mon Sep 17 00:00:00 2001 From: David Garcia Quintas <dgq@google.com> Date: Tue, 4 Oct 2016 13:44:18 -0700 Subject: [PATCH 56/83] more usage of std::string --- test/cpp/end2end/proto_server_reflection_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cpp/end2end/proto_server_reflection_test.cc b/test/cpp/end2end/proto_server_reflection_test.cc index efbb0e1f8e5..75efd01f066 100644 --- a/test/cpp/end2end/proto_server_reflection_test.cc +++ b/test/cpp/end2end/proto_server_reflection_test.cc @@ -144,7 +144,7 @@ class ProtoServerReflectionTest : public ::testing::Test { TEST_F(ProtoServerReflectionTest, CheckResponseWithLocalDescriptorPool) { ResetStub(); - std::vector<std::string> services; + std::vector<grpc::string> services; desc_db_->GetServices(&services); // The service list has at least one service (reflection servcie). EXPECT_TRUE(services.size() > 0); From 3b0d092e22a30d449952be210b174cb9d080b462 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch <jtattermusch@google.com> Date: Mon, 19 Sep 2016 14:33:18 +0200 Subject: [PATCH 57/83] add matrix run script --- tools/run_tests/run_tests_in_workspace.sh | 44 ++++ tools/run_tests/run_tests_matrix.py | 239 ++++++++++++++++++++++ 2 files changed, 283 insertions(+) create mode 100755 tools/run_tests/run_tests_in_workspace.sh create mode 100755 tools/run_tests/run_tests_matrix.py diff --git a/tools/run_tests/run_tests_in_workspace.sh b/tools/run_tests/run_tests_in_workspace.sh new file mode 100755 index 00000000000..0e7604dbc55 --- /dev/null +++ b/tools/run_tests/run_tests_in_workspace.sh @@ -0,0 +1,44 @@ +#!/bin/bash +# Copyright 2015, Google Inc. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +# +# Create a workspace in a subdirectory to allow running multiple builds in isolation. +# WORKSPACE_NAME env variable needs to contain name of the workspace to create. +# All cmdline args will be passed to run_tests.py script (executed in the +# newly created workspace) +set -ex + +cd $(dirname $0)/../.. + +rm -rf "${WORKSPACE_NAME}" +git clone --recursive . "${WORKSPACE_NAME}" + +echo "Running run_tests.py in workspace ${WORKSPACE_NAME}" +"${WORKSPACE_NAME}/tools/run_tests/run_tests.py" $@ + diff --git a/tools/run_tests/run_tests_matrix.py b/tools/run_tests/run_tests_matrix.py new file mode 100755 index 00000000000..2344e2f9fd8 --- /dev/null +++ b/tools/run_tests/run_tests_matrix.py @@ -0,0 +1,239 @@ +#!/usr/bin/env python2.7 +# Copyright 2015, Google Inc. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +"""Run test matrix.""" + +import argparse +import jobset +import os +import report_utils +import sys + +_ROOT = os.path.abspath(os.path.join(os.path.dirname(sys.argv[0]), '../..')) +os.chdir(_ROOT) + +# TODO(jtattermusch): this is not going to be enough for sanitizers. +_RUNTESTS_TIMEOUT = 30*60 + + +def _docker_jobspec(name, runtests_args=[]): + """Run a single instance of run_tests.py in a docker container""" + # TODO: fix copying report files from inside docker.... + test_job = jobset.JobSpec( + cmdline=['python', 'tools/run_tests/run_tests.py', + '--use_docker', + '-t', + '-j', '3', + '-x', 'report_%s.xml' % name] + runtests_args, + shortname='run_tests_%s' % name, + timeout_seconds=_RUNTESTS_TIMEOUT) + return test_job + + +def _workspace_jobspec(name, runtests_args=[], workspace_name=None): + """Run a single instance of run_tests.py in a separate workspace""" + env = {'WORKSPACE_NAME': workspace_name} + test_job = jobset.JobSpec( + cmdline=['tools/run_tests/run_tests_in_workspace.sh', + '-t', + '-j', '3', + '-x', '../report_%s.xml' % name] + runtests_args, + environ=env, + shortname='run_tests_%s' % name, + timeout_seconds=_RUNTESTS_TIMEOUT) + return test_job + + +def _generate_jobs(languages, configs, platforms, + arch=None, compiler=None, + labels=[]): + result = [] + for language in languages: + for platform in platforms: + for config in configs: + name = '%s_%s_%s' % (language, platform, config) + runtests_args = ['-l', language, + '-c', config] + if arch or compiler: + name += '_%s_%s' % (arch, compiler) + runtests_args += ['--arch', arch, + '--compiler', compiler] + + if platform == 'linux': + job = _docker_jobspec(name=name, runtests_args=runtests_args) + else: + job = _workspace_jobspec(name=name, runtests_args=runtests_args) + + job.labels = [platform, config, language] + labels + result.append(job) + return result + + +def _create_test_jobs(): + test_jobs = [] + # supported on linux only + test_jobs += _generate_jobs(languages=['sanity', 'php7'], + configs=['dbg', 'opt'], + platforms=['linux'], + labels=['basictests']) + + # supported on all platforms. + test_jobs += _generate_jobs(languages=['c', 'csharp', 'node', 'python'], + configs=['dbg', 'opt'], + platforms=['linux', 'macos', 'windows'], + labels=['basictests']) + + # supported on linux and mac. + test_jobs += _generate_jobs(languages=['c++', 'ruby', 'php'], + configs=['dbg', 'opt'], + platforms=['linux', 'macos'], + labels=['basictests']) + + # supported on mac only. + test_jobs += _generate_jobs(languages=['objc'], + configs=['dbg', 'opt'], + platforms=['macos'], + labels=['basictests']) + + # sanitizers + test_jobs += _generate_jobs(languages=['c'], + configs=['msan', 'asan', 'tsan'], + platforms=['linux'], + labels=['sanitizers']) + test_jobs += _generate_jobs(languages=['c++'], + configs=['asan', 'tsan'], + platforms=['linux'], + labels=['sanitizers']) + return test_jobs + + +def _create_portability_test_jobs(): + test_jobs = [] + # portability C x86 + test_jobs += _generate_jobs(languages=['c'], + configs=['dbg'], + platforms=['linux'], + arch='x86', + compiler='default', + labels=['portability']) + + # portability C and C++ on x64 + for compiler in ['gcc4.4', 'gcc4.6', 'gcc5.3', + 'clang3.5', 'clang3.6', 'clang3.7']: + test_jobs += _generate_jobs(languages=['c', 'c++'], + configs=['dbg'], + platforms=['linux'], + arch='x64', + compiler=compiler, + labels=['portability']) + + # portability C on Windows + for arch in ['x86', 'x64']: + for compiler in ['vs2013', 'vs2015']: + test_jobs += _generate_jobs(languages=['c'], + configs=['dbg'], + platforms=['windows'], + arch=arch, + compiler=compiler, + labels=['portability']) + + test_jobs += _generate_jobs(languages=['python'], + configs=['dbg'], + platforms=['linux'], + arch='default', + compiler='python3.4', + labels=['portability']) + + test_jobs += _generate_jobs(languages=['csharp'], + configs=['dbg'], + platforms=['linux'], + arch='default', + compiler='coreclr', + labels=['portability']) + + for compiler in ['node5', 'node6', 'node0.12']: + test_jobs += _generate_jobs(languages=['node'], + configs=['dbg'], + platforms=['linux'], + arch='default', + compiler=compiler, + labels=['portability']) + return test_jobs + + +all_jobs = _create_test_jobs() + _create_portability_test_jobs() + +all_labels = set() +for job in all_jobs: + for label in job.labels: + all_labels.add(label) + +argp = argparse.ArgumentParser(description='Run a matrix of run_tests.py tests.') +argp.add_argument('-f', '--filter', + choices=sorted(all_labels), + nargs='+', + default=[], + help='Filter targets to run by label with AND semantics.') +args = argp.parse_args() + +jobs = [] +for job in all_jobs: + if not args.filter or all(filter in job.labels for filter in args.filter): + jobs.append(job) + +if not jobs: + jobset.message('FAILED', 'No test suites match given criteria.', + do_newline=True) + sys.exit(1) + +print('IMPORTANT: The changes you are testing need to be locally committed') +print('because only the committed changes in the current branch will be') +print('copied to the docker environment or into subworkspaces.') + +print +print 'Will run these tests:' +for job in jobs: + print ' %s' % job.shortname +print + +jobset.message('START', 'Running test matrix.', do_newline=True) +num_failures, resultset = jobset.run(jobs, + newline_on_success=True, + travis=True, + maxjobs=2) +report_utils.render_junit_xml_report(resultset, 'report.xml') + +if num_failures == 0: + jobset.message('SUCCESS', 'All run_tests.py instance finished successfully.', + do_newline=True) +else: + jobset.message('FAILED', 'Some run_tests.py instance have failed.', + do_newline=True) + sys.exit(1) From 0b19470d2b6ec821aa504ce615ba9f9d68596d28 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch <jtattermusch@google.com> Date: Mon, 19 Sep 2016 18:20:37 +0200 Subject: [PATCH 58/83] improve report collection --- tools/run_tests/dockerize/docker_run_tests.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/run_tests/dockerize/docker_run_tests.sh b/tools/run_tests/dockerize/docker_run_tests.sh index 8c6143d24f9..ef02d266253 100755 --- a/tools/run_tests/dockerize/docker_run_tests.sh +++ b/tools/run_tests/dockerize/docker_run_tests.sh @@ -63,6 +63,7 @@ echo '</body></html>' >> index.html cd .. zip -r reports.zip reports -find . -name report.xml | xargs zip reports.zip +find . -name report.xml | xargs -r zip reports.zip +find . -name 'report_*.xml' | xargs -r zip reports.zip exit $exit_code From 7a7792a9de680c9d6925b092d5efbf17984feec5 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch <jtattermusch@google.com> Date: Mon, 19 Sep 2016 18:37:17 +0200 Subject: [PATCH 59/83] run matrix improvements --- .../dockerize/build_docker_and_run_tests.sh | 12 ++++++------ tools/run_tests/run_tests_in_workspace.sh | 2 ++ tools/run_tests/run_tests_matrix.py | 17 ++++++++++++++--- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/tools/run_tests/dockerize/build_docker_and_run_tests.sh b/tools/run_tests/dockerize/build_docker_and_run_tests.sh index c2ea6f2c6eb..b4b172ddef6 100755 --- a/tools/run_tests/dockerize/build_docker_and_run_tests.sh +++ b/tools/run_tests/dockerize/build_docker_and_run_tests.sh @@ -44,9 +44,6 @@ mkdir -p /tmp/ccache # its cache location now that --download-cache is deprecated). mkdir -p /tmp/xdg-cache-home -# Create a local branch so the child Docker script won't complain -git branch -f jenkins-docker - # Inputs # DOCKERFILE_DIR - Directory in which Dockerfile file is located. # DOCKER_RUN_SCRIPT - Script to run under docker (relative to grpc repo root) @@ -86,9 +83,12 @@ docker run \ $DOCKER_IMAGE_NAME \ bash -l "/var/local/jenkins/grpc/$DOCKER_RUN_SCRIPT" || DOCKER_FAILED="true" -docker cp "$CONTAINER_NAME:/var/local/git/grpc/reports.zip" $git_root || true -unzip -o $git_root/reports.zip -d $git_root || true -rm -f reports.zip +# use unique name for reports.zip to prevent clash between concurrent +# run_tests.py runs +TEMP_REPORTS_ZIP=`mktemp` +docker cp "$CONTAINER_NAME:/var/local/git/grpc/reports.zip" ${TEMP_REPORTS_ZIP} || true +unzip -o ${TEMP_REPORTS_ZIP} -d $git_root || true +rm -f ${TEMP_REPORTS_ZIP} # remove the container, possibly killing it first docker rm -f $CONTAINER_NAME || true diff --git a/tools/run_tests/run_tests_in_workspace.sh b/tools/run_tests/run_tests_in_workspace.sh index 0e7604dbc55..b0c9ad05f76 100755 --- a/tools/run_tests/run_tests_in_workspace.sh +++ b/tools/run_tests/run_tests_in_workspace.sh @@ -37,6 +37,8 @@ set -ex cd $(dirname $0)/../.. rm -rf "${WORKSPACE_NAME}" +# TODO(jtattermusch): clone --recursive fetches the submodules from github. +# Try avoiding that to save time and network capacity. git clone --recursive . "${WORKSPACE_NAME}" echo "Running run_tests.py in workspace ${WORKSPACE_NAME}" diff --git a/tools/run_tests/run_tests_matrix.py b/tools/run_tests/run_tests_matrix.py index 2344e2f9fd8..bc2e37abd89 100755 --- a/tools/run_tests/run_tests_matrix.py +++ b/tools/run_tests/run_tests_matrix.py @@ -32,6 +32,7 @@ import argparse import jobset +import multiprocessing import os import report_utils import sys @@ -40,8 +41,12 @@ _ROOT = os.path.abspath(os.path.join(os.path.dirname(sys.argv[0]), '../..')) os.chdir(_ROOT) # TODO(jtattermusch): this is not going to be enough for sanitizers. +# TODO(jtattermusch): this is not going to be enough for rebuilding clang docker. _RUNTESTS_TIMEOUT = 30*60 +# Number of jobs assigned to each run_tests.py instance +_INNER_JOBS = 2 + def _docker_jobspec(name, runtests_args=[]): """Run a single instance of run_tests.py in a docker container""" @@ -50,7 +55,7 @@ def _docker_jobspec(name, runtests_args=[]): cmdline=['python', 'tools/run_tests/run_tests.py', '--use_docker', '-t', - '-j', '3', + '-j', str(_INNER_JOBS), '-x', 'report_%s.xml' % name] + runtests_args, shortname='run_tests_%s' % name, timeout_seconds=_RUNTESTS_TIMEOUT) @@ -59,11 +64,13 @@ def _docker_jobspec(name, runtests_args=[]): def _workspace_jobspec(name, runtests_args=[], workspace_name=None): """Run a single instance of run_tests.py in a separate workspace""" + if not workspace_name: + workspace_name = 'workspace_%s' % name env = {'WORKSPACE_NAME': workspace_name} test_job = jobset.JobSpec( cmdline=['tools/run_tests/run_tests_in_workspace.sh', '-t', - '-j', '3', + '-j', str(_INNER_JOBS), '-x', '../report_%s.xml' % name] + runtests_args, environ=env, shortname='run_tests_%s' % name, @@ -196,6 +203,10 @@ for job in all_jobs: all_labels.add(label) argp = argparse.ArgumentParser(description='Run a matrix of run_tests.py tests.') +argp.add_argument('-j', '--jobs', + default=multiprocessing.cpu_count()/_INNER_JOBS, + type=int, + help='Number of concurrent run_tests.py instances.') argp.add_argument('-f', '--filter', choices=sorted(all_labels), nargs='+', @@ -227,7 +238,7 @@ jobset.message('START', 'Running test matrix.', do_newline=True) num_failures, resultset = jobset.run(jobs, newline_on_success=True, travis=True, - maxjobs=2) + maxjobs=args.jobs) report_utils.render_junit_xml_report(resultset, 'report.xml') if num_failures == 0: From 5b34c4fc099d8b8df0696c1255009c144bf03cd4 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch <jtattermusch@google.com> Date: Tue, 20 Sep 2016 16:06:13 +0200 Subject: [PATCH 60/83] address some TODOs --- tools/run_tests/run_tests_matrix.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tools/run_tests/run_tests_matrix.py b/tools/run_tests/run_tests_matrix.py index bc2e37abd89..06ab02e32fb 100755 --- a/tools/run_tests/run_tests_matrix.py +++ b/tools/run_tests/run_tests_matrix.py @@ -40,9 +40,9 @@ import sys _ROOT = os.path.abspath(os.path.join(os.path.dirname(sys.argv[0]), '../..')) os.chdir(_ROOT) -# TODO(jtattermusch): this is not going to be enough for sanitizers. -# TODO(jtattermusch): this is not going to be enough for rebuilding clang docker. -_RUNTESTS_TIMEOUT = 30*60 +# Set the timeout high to allow enough time for sanitizers and pre-building +# clang docker. +_RUNTESTS_TIMEOUT = 2*60*60 # Number of jobs assigned to each run_tests.py instance _INNER_JOBS = 2 @@ -50,7 +50,6 @@ _INNER_JOBS = 2 def _docker_jobspec(name, runtests_args=[]): """Run a single instance of run_tests.py in a docker container""" - # TODO: fix copying report files from inside docker.... test_job = jobset.JobSpec( cmdline=['python', 'tools/run_tests/run_tests.py', '--use_docker', @@ -203,6 +202,8 @@ for job in all_jobs: all_labels.add(label) argp = argparse.ArgumentParser(description='Run a matrix of run_tests.py tests.') +# TODO(jtattermusch): allow running tests with --build_only flag +# TODO(jtattermusch): allow running tests with --force_default_poller flag. argp.add_argument('-j', '--jobs', default=multiprocessing.cpu_count()/_INNER_JOBS, type=int, From 7a6c2235d522b67e9dfe77311e2239463abc8cd5 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch <jtattermusch@google.com> Date: Thu, 22 Sep 2016 13:40:48 +0200 Subject: [PATCH 61/83] support passing extra args to run_tests.py --- tools/run_tests/run_tests_matrix.py | 90 +++++++++++++++++++---------- 1 file changed, 59 insertions(+), 31 deletions(-) diff --git a/tools/run_tests/run_tests_matrix.py b/tools/run_tests/run_tests_matrix.py index 06ab02e32fb..ae7fdd84f9b 100755 --- a/tools/run_tests/run_tests_matrix.py +++ b/tools/run_tests/run_tests_matrix.py @@ -79,7 +79,7 @@ def _workspace_jobspec(name, runtests_args=[], workspace_name=None): def _generate_jobs(languages, configs, platforms, arch=None, compiler=None, - labels=[]): + labels=[], extra_args=[]): result = [] for language in languages: for platform in platforms: @@ -92,6 +92,7 @@ def _generate_jobs(languages, configs, platforms, runtests_args += ['--arch', arch, '--compiler', compiler] + runtests_args += extra_args if platform == 'linux': job = _docker_jobspec(name=name, runtests_args=runtests_args) else: @@ -102,45 +103,51 @@ def _generate_jobs(languages, configs, platforms, return result -def _create_test_jobs(): +def _create_test_jobs(extra_args=[]): test_jobs = [] # supported on linux only test_jobs += _generate_jobs(languages=['sanity', 'php7'], configs=['dbg', 'opt'], platforms=['linux'], - labels=['basictests']) + labels=['basictests'], + extra_args=extra_args) # supported on all platforms. test_jobs += _generate_jobs(languages=['c', 'csharp', 'node', 'python'], - configs=['dbg', 'opt'], - platforms=['linux', 'macos', 'windows'], - labels=['basictests']) + configs=['dbg', 'opt'], + platforms=['linux', 'macos', 'windows'], + labels=['basictests'], + extra_args=extra_args) # supported on linux and mac. test_jobs += _generate_jobs(languages=['c++', 'ruby', 'php'], - configs=['dbg', 'opt'], - platforms=['linux', 'macos'], - labels=['basictests']) + configs=['dbg', 'opt'], + platforms=['linux', 'macos'], + labels=['basictests'], + extra_args=extra_args) # supported on mac only. test_jobs += _generate_jobs(languages=['objc'], configs=['dbg', 'opt'], platforms=['macos'], - labels=['basictests']) + labels=['basictests'], + extra_args=extra_args) # sanitizers test_jobs += _generate_jobs(languages=['c'], configs=['msan', 'asan', 'tsan'], platforms=['linux'], - labels=['sanitizers']) + labels=['sanitizers'], + extra_args=extra_args) test_jobs += _generate_jobs(languages=['c++'], configs=['asan', 'tsan'], platforms=['linux'], - labels=['sanitizers']) + labels=['sanitizers'], + extra_args=extra_args) return test_jobs -def _create_portability_test_jobs(): +def _create_portability_test_jobs(extra_args=[]): test_jobs = [] # portability C x86 test_jobs += _generate_jobs(languages=['c'], @@ -148,7 +155,8 @@ def _create_portability_test_jobs(): platforms=['linux'], arch='x86', compiler='default', - labels=['portability']) + labels=['portability'], + extra_args=extra_args) # portability C and C++ on x64 for compiler in ['gcc4.4', 'gcc4.6', 'gcc5.3', @@ -158,7 +166,8 @@ def _create_portability_test_jobs(): platforms=['linux'], arch='x64', compiler=compiler, - labels=['portability']) + labels=['portability'], + extra_args=extra_args) # portability C on Windows for arch in ['x86', 'x64']: @@ -168,53 +177,72 @@ def _create_portability_test_jobs(): platforms=['windows'], arch=arch, compiler=compiler, - labels=['portability']) + labels=['portability'], + extra_args=extra_args) test_jobs += _generate_jobs(languages=['python'], configs=['dbg'], platforms=['linux'], arch='default', compiler='python3.4', - labels=['portability']) + labels=['portability'], + extra_args=extra_args) test_jobs += _generate_jobs(languages=['csharp'], configs=['dbg'], platforms=['linux'], arch='default', compiler='coreclr', - labels=['portability']) + labels=['portability'], + extra_args=extra_args) for compiler in ['node5', 'node6', 'node0.12']: test_jobs += _generate_jobs(languages=['node'], - configs=['dbg'], - platforms=['linux'], - arch='default', - compiler=compiler, - labels=['portability']) + configs=['dbg'], + platforms=['linux'], + arch='default', + compiler=compiler, + labels=['portability'], + extra_args=extra_args) return test_jobs -all_jobs = _create_test_jobs() + _create_portability_test_jobs() +def _allowed_labels(): + """Returns a list of existing job labels.""" + all_labels = set() + for job in _create_test_jobs() + _create_portability_test_jobs(): + for label in job.labels: + all_labels.add(label) + return sorted(all_labels) -all_labels = set() -for job in all_jobs: - for label in job.labels: - all_labels.add(label) argp = argparse.ArgumentParser(description='Run a matrix of run_tests.py tests.') -# TODO(jtattermusch): allow running tests with --build_only flag -# TODO(jtattermusch): allow running tests with --force_default_poller flag. argp.add_argument('-j', '--jobs', default=multiprocessing.cpu_count()/_INNER_JOBS, type=int, help='Number of concurrent run_tests.py instances.') argp.add_argument('-f', '--filter', - choices=sorted(all_labels), + choices=_allowed_labels(), nargs='+', default=[], help='Filter targets to run by label with AND semantics.') +argp.add_argument('--build_only', + default=False, + action='store_const', + const=True, + help='Pass --build_only flag to run_tests.py instances.') +argp.add_argument('--force_default_poller', default=False, action='store_const', const=True, + help='Pass --force_default_poller to run_tests.py instances.') args = argp.parse_args() +extra_args = [] +if args.build_only: + extra_args.append('--build_only') +if args.force_default_poller: + extra_args.append('--force_default_poller') + +all_jobs = _create_test_jobs(extra_args=extra_args) + _create_portability_test_jobs(extra_args=extra_args) + jobs = [] for job in all_jobs: if not args.filter or all(filter in job.labels for filter in args.filter): From f098c753baf059b5c01699b8f56c194180cbf3e1 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch <jtattermusch@google.com> Date: Thu, 22 Sep 2016 14:28:07 +0200 Subject: [PATCH 62/83] node6 portability test not yet implemented --- tools/run_tests/run_tests_matrix.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/run_tests/run_tests_matrix.py b/tools/run_tests/run_tests_matrix.py index ae7fdd84f9b..d87632d8891 100755 --- a/tools/run_tests/run_tests_matrix.py +++ b/tools/run_tests/run_tests_matrix.py @@ -196,7 +196,7 @@ def _create_portability_test_jobs(extra_args=[]): labels=['portability'], extra_args=extra_args) - for compiler in ['node5', 'node6', 'node0.12']: + for compiler in ['node5', 'node0.12']: test_jobs += _generate_jobs(languages=['node'], configs=['dbg'], platforms=['linux'], From e922d7702eb8ead6acfa23fc44823b699bc6a72f Mon Sep 17 00:00:00 2001 From: Jan Tattermusch <jtattermusch@google.com> Date: Thu, 22 Sep 2016 14:31:10 +0200 Subject: [PATCH 63/83] support --dry_run flag --- tools/run_tests/run_tests_matrix.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tools/run_tests/run_tests_matrix.py b/tools/run_tests/run_tests_matrix.py index d87632d8891..a31b0dd9177 100755 --- a/tools/run_tests/run_tests_matrix.py +++ b/tools/run_tests/run_tests_matrix.py @@ -233,6 +233,11 @@ argp.add_argument('--build_only', help='Pass --build_only flag to run_tests.py instances.') argp.add_argument('--force_default_poller', default=False, action='store_const', const=True, help='Pass --force_default_poller to run_tests.py instances.') +argp.add_argument('--dry_run', + default=False, + action='store_const', + const=True, + help='Only print what would be run.') args = argp.parse_args() extra_args = [] @@ -263,6 +268,10 @@ for job in jobs: print ' %s' % job.shortname print +if args.dry_run: + print '--dry_run was used, exiting' + sys.exit(1) + jobset.message('START', 'Running test matrix.', do_newline=True) num_failures, resultset = jobset.run(jobs, newline_on_success=True, From b6b4f8759b641aee0d737d16d8375560d9024e4d Mon Sep 17 00:00:00 2001 From: Jan Tattermusch <jtattermusch@google.com> Date: Thu, 22 Sep 2016 14:44:27 +0200 Subject: [PATCH 64/83] better --dry_run printouts --- tools/run_tests/run_tests_matrix.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/run_tests/run_tests_matrix.py b/tools/run_tests/run_tests_matrix.py index a31b0dd9177..d5f90478257 100755 --- a/tools/run_tests/run_tests_matrix.py +++ b/tools/run_tests/run_tests_matrix.py @@ -265,7 +265,10 @@ print('copied to the docker environment or into subworkspaces.') print print 'Will run these tests:' for job in jobs: - print ' %s' % job.shortname + if args.dry_run: + print ' %s: "%s"' % (job.shortname, ' '.join(job.cmdline)) + else: + print ' %s' % job.shortname print if args.dry_run: From 1b0dd825cc6e56fa79c5de46452069aee87193e5 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch <jtattermusch@google.com> Date: Tue, 4 Oct 2016 11:52:32 +0200 Subject: [PATCH 65/83] fixes for run_tests_in_workspace.sh --- tools/run_tests/run_tests_in_workspace.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/run_tests/run_tests_in_workspace.sh b/tools/run_tests/run_tests_in_workspace.sh index b0c9ad05f76..98ef3566db1 100755 --- a/tools/run_tests/run_tests_in_workspace.sh +++ b/tools/run_tests/run_tests_in_workspace.sh @@ -42,5 +42,5 @@ rm -rf "${WORKSPACE_NAME}" git clone --recursive . "${WORKSPACE_NAME}" echo "Running run_tests.py in workspace ${WORKSPACE_NAME}" -"${WORKSPACE_NAME}/tools/run_tests/run_tests.py" $@ +python "${WORKSPACE_NAME}/tools/run_tests/run_tests.py" $@ From ad666a94142db43593456f4cd88645ff80e66786 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch <jtattermusch@google.com> Date: Tue, 4 Oct 2016 15:08:36 +0200 Subject: [PATCH 66/83] remove node portability targets --- tools/run_tests/run_tests_matrix.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/tools/run_tests/run_tests_matrix.py b/tools/run_tests/run_tests_matrix.py index d5f90478257..a94f9cfef5e 100755 --- a/tools/run_tests/run_tests_matrix.py +++ b/tools/run_tests/run_tests_matrix.py @@ -195,15 +195,6 @@ def _create_portability_test_jobs(extra_args=[]): compiler='coreclr', labels=['portability'], extra_args=extra_args) - - for compiler in ['node5', 'node0.12']: - test_jobs += _generate_jobs(languages=['node'], - configs=['dbg'], - platforms=['linux'], - arch='default', - compiler=compiler, - labels=['portability'], - extra_args=extra_args) return test_jobs From ffdd3a3cab09235c11efc34bea2ee6746dd54f12 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch <jtattermusch@google.com> Date: Tue, 4 Oct 2016 17:52:15 +0200 Subject: [PATCH 67/83] add proxy script for run_tests_matrix.py --- tools/jenkins/run_jenkins_matrix.sh | 42 +++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) create mode 100755 tools/jenkins/run_jenkins_matrix.sh diff --git a/tools/jenkins/run_jenkins_matrix.sh b/tools/jenkins/run_jenkins_matrix.sh new file mode 100755 index 00000000000..b3783e69584 --- /dev/null +++ b/tools/jenkins/run_jenkins_matrix.sh @@ -0,0 +1,42 @@ +#!/usr/bin/env bash +# Copyright 2015, Google Inc. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +# +# This script is invoked by Jenkins and triggers a test run, bypassing +# all args to the test script. +# +# Setting up rvm environment BEFORE we set -ex. +[[ -s /etc/profile.d/rvm.sh ]] && . /etc/profile.d/rvm.sh +# To prevent cygwin bash complaining about empty lines ending with \r +# we set the igncr option. The option doesn't exist on Linux, so we fallback +# to just 'set -ex' there. +# NOTE: No empty lines should appear in this file before igncr is set! +set -ex -o igncr || set -ex + +python tools/run_tests/run_tests_matrix.py $@ From 5f33e0b1d4c6c6ec394967afba703fdb76c73738 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch <jtattermusch@google.com> Date: Mon, 3 Oct 2016 19:47:37 +0200 Subject: [PATCH 68/83] fix building go interop image --- .../interoptest/grpc_interop_go/build_interop.sh | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/tools/dockerfile/interoptest/grpc_interop_go/build_interop.sh b/tools/dockerfile/interoptest/grpc_interop_go/build_interop.sh index 1fd088322cd..858ee0a68cd 100755 --- a/tools/dockerfile/interoptest/grpc_interop_go/build_interop.sh +++ b/tools/dockerfile/interoptest/grpc_interop_go/build_interop.sh @@ -36,19 +36,12 @@ set -e # to test instead of using "go get" to download from Github directly. git clone --recursive /var/local/jenkins/grpc-go src/google.golang.org/grpc +# Get all gRPC Go dependencies +(cd src/google.golang.org/grpc && go get -t .) + # copy service account keys if available cp -r /var/local/jenkins/service_account $HOME || true -# Get dependencies from GitHub -# NOTE: once grpc-go dependencies change, this needs to be updated manually -# but we don't expect this to happen any time soon. -go get github.com/golang/protobuf/proto -go get golang.org/x/net/context -go get golang.org/x/net/trace -go get golang.org/x/oauth2 -go get golang.org/x/oauth2/google -go get google.golang.org/cloud - # Build the interop client and server (cd src/google.golang.org/grpc/interop/client && go install) (cd src/google.golang.org/grpc/interop/server && go install) From 8e5a2eab00ba0a14d4352fcf49ff12e6e870ba72 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch <jtattermusch@google.com> Date: Wed, 5 Oct 2016 15:26:03 +0200 Subject: [PATCH 69/83] add script to reboot a jenkins slave --- tools/jenkins/reboot_worker.sh | 38 ++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100755 tools/jenkins/reboot_worker.sh diff --git a/tools/jenkins/reboot_worker.sh b/tools/jenkins/reboot_worker.sh new file mode 100755 index 00000000000..285e699b9b9 --- /dev/null +++ b/tools/jenkins/reboot_worker.sh @@ -0,0 +1,38 @@ +#!/usr/bin/env bash +# Copyright 2016, Google Inc. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions are +# met: +# +# * Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# * Redistributions in binary form must reproduce the above +# copyright notice, this list of conditions and the following disclaimer +# in the documentation and/or other materials provided with the +# distribution. +# * Neither the name of Google Inc. nor the names of its +# contributors may be used to endorse or promote products derived from +# this software without specific prior written permission. +# +# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +# +# Reboots Jenkins worker +# +# NOTE: No empty lines should appear in this file before igncr is set! +set -ex -o igncr || set -ex + +# Give 5 seconds to finish the current job, then kill the jenkins slave process +# to avoid running any other jobs on the worker and restart the worker. +nohup sh -c 'sleep 5; killall java; sudo reboot' & From a3570f22d06ce2aa3bc6bf20784b79243db935e8 Mon Sep 17 00:00:00 2001 From: Lizan Zhou <zlizan@google.com> Date: Mon, 5 Sep 2016 13:29:17 +0900 Subject: [PATCH 70/83] Fix grpc_byte_buffer_copy to copy compression algorithm --- src/core/lib/surface/byte_buffer.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/core/lib/surface/byte_buffer.c b/src/core/lib/surface/byte_buffer.c index a093a37af32..054a6e6c586 100644 --- a/src/core/lib/surface/byte_buffer.c +++ b/src/core/lib/surface/byte_buffer.c @@ -72,8 +72,9 @@ grpc_byte_buffer *grpc_raw_byte_buffer_from_reader( grpc_byte_buffer *grpc_byte_buffer_copy(grpc_byte_buffer *bb) { switch (bb->type) { case GRPC_BB_RAW: - return grpc_raw_byte_buffer_create(bb->data.raw.slice_buffer.slices, - bb->data.raw.slice_buffer.count); + return grpc_raw_compressed_byte_buffer_create( + bb->data.raw.slice_buffer.slices, bb->data.raw.slice_buffer.count, + bb->data.raw.compression); } GPR_UNREACHABLE_CODE(return NULL); } From c7cf9a69a4299fffd071c0436df43a0a187a60fa Mon Sep 17 00:00:00 2001 From: Yuchen Zeng <zyc@google.com> Date: Wed, 5 Oct 2016 17:36:14 -0700 Subject: [PATCH 71/83] Fix maybe-uninitialized variable --- src/core/ext/transport/cronet/transport/cronet_transport.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.c b/src/core/ext/transport/cronet/transport/cronet_transport.c index 19e43673b99..984a8bb5551 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.c +++ b/src/core/ext/transport/cronet/transport/cronet_transport.c @@ -781,7 +781,7 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, &cronet_callbacks); CRONET_LOG(GPR_DEBUG, "%p = cronet_bidirectional_stream_create()", s->cbs); char *url; - const char *method; + const char *method = NULL; s->header_array.headers = NULL; convert_metadata_to_cronet_headers( stream_op->send_initial_metadata->list.head, s->curr_ct.host, &url, From 4552f6aad888878fb15b292c134b5d90233d5491 Mon Sep 17 00:00:00 2001 From: Yuchen Zeng <zyc@google.com> Date: Wed, 5 Oct 2016 17:54:00 -0700 Subject: [PATCH 72/83] Init method as POST --- src/core/ext/transport/cronet/transport/cronet_transport.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.c b/src/core/ext/transport/cronet/transport/cronet_transport.c index 984a8bb5551..4431d58351e 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.c +++ b/src/core/ext/transport/cronet/transport/cronet_transport.c @@ -781,7 +781,7 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, &cronet_callbacks); CRONET_LOG(GPR_DEBUG, "%p = cronet_bidirectional_stream_create()", s->cbs); char *url; - const char *method = NULL; + const char *method = "POST"; s->header_array.headers = NULL; convert_metadata_to_cronet_headers( stream_op->send_initial_metadata->list.head, s->curr_ct.host, &url, From bc4ea6d727692c1720df7a3480f8d86c73d68df0 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch <jtattermusch@google.com> Date: Thu, 6 Oct 2016 14:01:02 +0200 Subject: [PATCH 73/83] fix compilation error --- src/core/ext/transport/cronet/transport/cronet_transport.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.c b/src/core/ext/transport/cronet/transport/cronet_transport.c index 19e43673b99..f7f57ac825c 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.c +++ b/src/core/ext/transport/cronet/transport/cronet_transport.c @@ -780,8 +780,8 @@ static enum e_op_result execute_stream_op(grpc_exec_ctx *exec_ctx, s->cbs = cronet_bidirectional_stream_create(s->curr_ct.engine, s->curr_gs, &cronet_callbacks); CRONET_LOG(GPR_DEBUG, "%p = cronet_bidirectional_stream_create()", s->cbs); - char *url; - const char *method; + char *url = NULL; + const char *method = NULL; s->header_array.headers = NULL; convert_metadata_to_cronet_headers( stream_op->send_initial_metadata->list.head, s->curr_ct.host, &url, From 8bc8a83656eaf1bb3b1362edbb590013bd62a4a7 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" <roth@google.com> Date: Thu, 6 Oct 2016 08:11:29 -0700 Subject: [PATCH 74/83] Update tests. --- test/core/bad_ssl/bad_ssl_test.c | 2 +- test/core/end2end/connection_refused_test.c | 14 +++++++------- test/core/end2end/dualstack_socket_test.c | 2 +- test/core/end2end/tests/simple_delayed_request.c | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) diff --git a/test/core/bad_ssl/bad_ssl_test.c b/test/core/bad_ssl/bad_ssl_test.c index c9cdb169b61..f8a9fe6caca 100644 --- a/test/core/bad_ssl/bad_ssl_test.c +++ b/test/core/bad_ssl/bad_ssl_test.c @@ -88,7 +88,7 @@ static void run_test(const char *target, size_t nops) { op = ops; op->op = GRPC_OP_SEND_INITIAL_METADATA; op->data.send_initial_metadata.count = 0; - op->flags = GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY; + op->flags = GRPC_INITIAL_METADATA_WAIT_FOR_READY; op->reserved = NULL; op++; op->op = GRPC_OP_RECV_STATUS_ON_CLIENT; diff --git a/test/core/end2end/connection_refused_test.c b/test/core/end2end/connection_refused_test.c index 4149159a378..62278d63c5a 100644 --- a/test/core/end2end/connection_refused_test.c +++ b/test/core/end2end/connection_refused_test.c @@ -44,7 +44,7 @@ static void *tag(intptr_t i) { return (void *)i; } -static void run_test(bool fail_fast) { +static void run_test(bool wait_for_ready) { grpc_channel *chan; grpc_call *call; gpr_timespec deadline = GRPC_TIMEOUT_SECONDS_TO_DEADLINE(2); @@ -57,7 +57,7 @@ static void run_test(bool fail_fast) { char *details = NULL; size_t details_capacity = 0; - gpr_log(GPR_INFO, "TEST: fail_fast=%d", fail_fast); + gpr_log(GPR_INFO, "TEST: wait_for_ready=%d", wait_for_ready); grpc_init(); @@ -81,7 +81,7 @@ static void run_test(bool fail_fast) { op = ops; op->op = GRPC_OP_SEND_INITIAL_METADATA; op->data.send_initial_metadata.count = 0; - op->flags = fail_fast ? 0 : GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY; + op->flags = wait_for_ready ? GRPC_INITIAL_METADATA_WAIT_FOR_READY : 0; op->reserved = NULL; op++; op->op = GRPC_OP_RECV_STATUS_ON_CLIENT; @@ -98,10 +98,10 @@ static void run_test(bool fail_fast) { CQ_EXPECT_COMPLETION(cqv, tag(1), 1); cq_verify(cqv); - if (fail_fast) { - GPR_ASSERT(status == GRPC_STATUS_UNAVAILABLE); - } else { + if (wait_for_ready) { GPR_ASSERT(status == GRPC_STATUS_DEADLINE_EXCEEDED); + } else { + GPR_ASSERT(status == GRPC_STATUS_UNAVAILABLE); } grpc_completion_queue_shutdown(cq); @@ -122,7 +122,7 @@ static void run_test(bool fail_fast) { int main(int argc, char **argv) { grpc_test_init(argc, argv); - run_test(true); run_test(false); + run_test(true); return 0; } diff --git a/test/core/end2end/dualstack_socket_test.c b/test/core/end2end/dualstack_socket_test.c index 8abb81c803a..cb07ca535b3 100644 --- a/test/core/end2end/dualstack_socket_test.c +++ b/test/core/end2end/dualstack_socket_test.c @@ -171,7 +171,7 @@ void test_connect(const char *server_host, const char *client_host, int port, op = ops; op->op = GRPC_OP_SEND_INITIAL_METADATA; op->data.send_initial_metadata.count = 0; - op->flags = expect_ok ? GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY : 0; + op->flags = expect_ok ? GRPC_INITIAL_METADATA_WAIT_FOR_READY : 0; op->reserved = NULL; op++; op->op = GRPC_OP_SEND_CLOSE_FROM_CLIENT; diff --git a/test/core/end2end/tests/simple_delayed_request.c b/test/core/end2end/tests/simple_delayed_request.c index 74f1232d786..50d1975c8d0 100644 --- a/test/core/end2end/tests/simple_delayed_request.c +++ b/test/core/end2end/tests/simple_delayed_request.c @@ -119,7 +119,7 @@ static void simple_delayed_request_body(grpc_end2end_test_config config, op = ops; op->op = GRPC_OP_SEND_INITIAL_METADATA; op->data.send_initial_metadata.count = 0; - op->flags = GRPC_INITIAL_METADATA_IGNORE_CONNECTIVITY; + op->flags = GRPC_INITIAL_METADATA_WAIT_FOR_READY; op->reserved = NULL; op++; op->op = GRPC_OP_SEND_CLOSE_FROM_CLIENT; From 624f4ba76d99b14d9e90fee03cee8d0866e1bca6 Mon Sep 17 00:00:00 2001 From: Craig Tiller <ctiller@google.com> Date: Thu, 6 Oct 2016 11:56:54 -0700 Subject: [PATCH 75/83] Add missing delete --- test/cpp/qps/client_async.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/test/cpp/qps/client_async.cc b/test/cpp/qps/client_async.cc index 5d9cb4bd0cf..081114859ce 100644 --- a/test/cpp/qps/client_async.cc +++ b/test/cpp/qps/client_async.cc @@ -243,6 +243,7 @@ class AsyncClient : public ClientImpl<StubType, RequestType> { // this thread isn't supposed to shut down std::lock_guard<std::mutex> l(shutdown_state_[thread_idx]->mutex); if (shutdown_state_[thread_idx]->shutdown) { + delete ctx; return true; } else if (!ctx->RunNextState(ok, entry)) { // The RPC and callback are done, so clone the ctx From 6f6f94fc92517b1487034e8d72be0b6779eb51ff Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" <roth@google.com> Date: Thu, 6 Oct 2016 12:16:31 -0700 Subject: [PATCH 76/83] Fix header. --- src/cpp/common/channel_filter.h | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/cpp/common/channel_filter.h b/src/cpp/common/channel_filter.h index 6f5af3dec31..ae32e02f699 100644 --- a/src/cpp/common/channel_filter.h +++ b/src/cpp/common/channel_filter.h @@ -42,6 +42,7 @@ #include <vector> #include "src/core/lib/channel/channel_stack.h" +#include "src/core/lib/security/context/security_context.h" #include "src/core/lib/surface/channel_init.h" #include "src/core/lib/transport/metadata_batch.h" @@ -54,11 +55,6 @@ /// "name-of-filter", GRPC_SERVER_CHANNEL, INT_MAX, nullptr); /// \endcode -/// Forward declaration to avoid including the file -/// "src/core/lib/security/context/security_context.h" -struct grpc_client_security_context; -struct grpc_server_security_context; - namespace grpc { /// A C++ wrapper for the \c grpc_metadata_batch struct. From 619c034fefab80def5ede91f6c28ef4013410035 Mon Sep 17 00:00:00 2001 From: Stanley Cheung <stanleycheung@google.com> Date: Thu, 6 Oct 2016 12:22:02 -0700 Subject: [PATCH 77/83] php: require grpc extension to be installed before composer package --- composer.json | 1 + examples/php/composer.json | 1 + src/php/composer.json | 1 + templates/composer.json.template | 1 + templates/src/php/composer.json.template | 1 + tools/dockerfile/interoptest/grpc_interop_php/build_interop.sh | 2 +- tools/dockerfile/interoptest/grpc_interop_php7/build_interop.sh | 2 +- .../stress_test/grpc_interop_stress_php/build_interop_stress.sh | 2 +- 8 files changed, 8 insertions(+), 3 deletions(-) diff --git a/composer.json b/composer.json index c5c7ae81d88..711ee82b79a 100644 --- a/composer.json +++ b/composer.json @@ -7,6 +7,7 @@ "license": "BSD-3-Clause", "require": { "php": ">=5.5.0", + "ext-grpc": "*", "google/protobuf": "v3.1.0-alpha-1" }, "require-dev": { diff --git a/examples/php/composer.json b/examples/php/composer.json index e6409f87b43..3d1a95d0045 100644 --- a/examples/php/composer.json +++ b/examples/php/composer.json @@ -2,6 +2,7 @@ "name": "grpc/grpc-demo", "description": "gRPC example for PHP", "require": { + "ext-grpc": "*", "grpc/grpc": "v1.0.0" } } diff --git a/src/php/composer.json b/src/php/composer.json index 60420940321..2d5d555bc29 100644 --- a/src/php/composer.json +++ b/src/php/composer.json @@ -8,6 +8,7 @@ "version": "1.1.0", "require": { "php": ">=5.5.0", + "ext-grpc": "*", "google/protobuf": "v3.1.0-alpha-1" }, "require-dev": { diff --git a/templates/composer.json.template b/templates/composer.json.template index accfb382a99..3b4d62f24db 100644 --- a/templates/composer.json.template +++ b/templates/composer.json.template @@ -9,6 +9,7 @@ "license": "BSD-3-Clause", "require": { "php": ">=5.5.0", + "ext-grpc": "*", "google/protobuf": "v3.1.0-alpha-1" }, "require-dev": { diff --git a/templates/src/php/composer.json.template b/templates/src/php/composer.json.template index 7feeae976d8..12a4ce8f83f 100644 --- a/templates/src/php/composer.json.template +++ b/templates/src/php/composer.json.template @@ -10,6 +10,7 @@ "version": "${settings.php_version.php_composer()}", "require": { "php": ">=5.5.0", + "ext-grpc": "*", "google/protobuf": "v3.1.0-alpha-1" }, "require-dev": { diff --git a/tools/dockerfile/interoptest/grpc_interop_php/build_interop.sh b/tools/dockerfile/interoptest/grpc_interop_php/build_interop.sh index cf5e888effb..624d5877862 100755 --- a/tools/dockerfile/interoptest/grpc_interop_php/build_interop.sh +++ b/tools/dockerfile/interoptest/grpc_interop_php/build_interop.sh @@ -46,6 +46,6 @@ make install (cd third_party/protobuf && make install) -(cd src/php && composer install) +(cd src/php && php -d extension=ext/grpc/modules/grpc.so /usr/local/bin/composer install) (cd src/php && ./bin/generate_proto_php.sh) diff --git a/tools/dockerfile/interoptest/grpc_interop_php7/build_interop.sh b/tools/dockerfile/interoptest/grpc_interop_php7/build_interop.sh index e486e5276a8..87cb0fe4b21 100755 --- a/tools/dockerfile/interoptest/grpc_interop_php7/build_interop.sh +++ b/tools/dockerfile/interoptest/grpc_interop_php7/build_interop.sh @@ -46,6 +46,6 @@ make install (cd third_party/protobuf && make install) -(cd src/php && composer install) +(cd src/php && php -d extension=ext/grpc/modules/grpc.so /usr/local/bin/composer install) (cd src/php && ./bin/generate_proto_php.sh) diff --git a/tools/dockerfile/stress_test/grpc_interop_stress_php/build_interop_stress.sh b/tools/dockerfile/stress_test/grpc_interop_stress_php/build_interop_stress.sh index 34fd09f78b0..a671d1501f8 100755 --- a/tools/dockerfile/stress_test/grpc_interop_stress_php/build_interop_stress.sh +++ b/tools/dockerfile/stress_test/grpc_interop_stress_php/build_interop_stress.sh @@ -48,6 +48,6 @@ make install (cd third_party/protobuf && make install) -(cd src/php && composer install) +(cd src/php && php -d extension=ext/grpc/modules/grpc.so /usr/local/bin/composer install) (cd src/php && ./bin/generate_proto_php.sh) From 757e84ef1cfd50661cb8242debd7a7990448dde1 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" <roth@google.com> Date: Thu, 6 Oct 2016 13:07:53 -0700 Subject: [PATCH 78/83] Add 'extern "C"' to error.h. --- src/core/lib/iomgr/error.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/core/lib/iomgr/error.h b/src/core/lib/iomgr/error.h index 7e2fd7a3bd6..00ace8a7a91 100644 --- a/src/core/lib/iomgr/error.h +++ b/src/core/lib/iomgr/error.h @@ -40,6 +40,10 @@ #include <grpc/status.h> #include <grpc/support/time.h> +#ifdef __cplusplus +extern "C" { +#endif + /// Opaque representation of an error. /// Errors are refcounted objects that represent the result of an operation. /// Ownership laws: @@ -204,4 +208,8 @@ bool grpc_log_if_error(const char *what, grpc_error *error, const char *file, #define GRPC_LOG_IF_ERROR(what, error) \ grpc_log_if_error((what), (error), __FILE__, __LINE__) +#ifdef __cplusplus +} +#endif + #endif /* GRPC_CORE_LIB_IOMGR_ERROR_H */ From 82b64d1565318afb40d72d46e890766d24368dc8 Mon Sep 17 00:00:00 2001 From: Alexander Polcyn <apolcyn@google.com> Date: Thu, 6 Oct 2016 17:33:12 -0700 Subject: [PATCH 79/83] change back slashes to forward slashes in grpc.tool nuspec --- src/csharp/Grpc.Tools.nuspec | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/csharp/Grpc.Tools.nuspec b/src/csharp/Grpc.Tools.nuspec index 0c937ab9cbf..ba4e1d674cd 100644 --- a/src/csharp/Grpc.Tools.nuspec +++ b/src/csharp/Grpc.Tools.nuspec @@ -17,17 +17,17 @@ </metadata> <files> <!-- forward slashes in src path enable building on Linux --> - <file src="protoc_plugins/windows_x86/protoc.exe" target="tools\windows_x86\protoc.exe" /> - <file src="protoc_plugins/windows_x86/grpc_csharp_plugin.exe" target="tools\windows_x86\grpc_csharp_plugin.exe" /> - <file src="protoc_plugins/windows_x64/protoc.exe" target="tools\windows_x64\protoc.exe" /> - <file src="protoc_plugins/windows_x64/grpc_csharp_plugin.exe" target="tools\windows_x64\grpc_csharp_plugin.exe" /> - <file src="protoc_plugins/linux_x86/protoc" target="tools\linux_x86\protoc" /> - <file src="protoc_plugins/linux_x86/grpc_csharp_plugin" target="tools\linux_x86\grpc_csharp_plugin" /> - <file src="protoc_plugins/linux_x64/protoc" target="tools\linux_x64\protoc" /> - <file src="protoc_plugins/linux_x64/grpc_csharp_plugin" target="tools\linux_x64\grpc_csharp_plugin" /> - <file src="protoc_plugins/macosx_x86/protoc" target="tools\macosx_x86\protoc" /> - <file src="protoc_plugins/macosx_x86/grpc_csharp_plugin" target="tools\macosx_x86\grpc_csharp_plugin" /> - <file src="protoc_plugins/macosx_x64/protoc" target="tools\macosx_x64\protoc" /> - <file src="protoc_plugins/macosx_x64/grpc_csharp_plugin" target="tools\macosx_x64\grpc_csharp_plugin" /> + <file src="protoc_plugins/windows_x86/protoc.exe" target="tools/windows_x86/protoc.exe" /> + <file src="protoc_plugins/windows_x86/grpc_csharp_plugin.exe" target="tools/windows_x86/grpc_csharp_plugin.exe" /> + <file src="protoc_plugins/windows_x64/protoc.exe" target="tools/windows_x64/protoc.exe" /> + <file src="protoc_plugins/windows_x64/grpc_csharp_plugin.exe" target="tools/windows_x64/grpc_csharp_plugin.exe" /> + <file src="protoc_plugins/linux_x86/protoc" target="tools/linux_x86/protoc" /> + <file src="protoc_plugins/linux_x86/grpc_csharp_plugin" target="tools/linux_x86/grpc_csharp_plugin" /> + <file src="protoc_plugins/linux_x64/protoc" target="tools/linux_x64/protoc" /> + <file src="protoc_plugins/linux_x64/grpc_csharp_plugin" target="tools/linux_x64/grpc_csharp_plugin" /> + <file src="protoc_plugins/macosx_x86/protoc" target="tools/macosx_x86/protoc" /> + <file src="protoc_plugins/macosx_x86/grpc_csharp_plugin" target="tools/macosx_x86/grpc_csharp_plugin" /> + <file src="protoc_plugins/macosx_x64/protoc" target="tools/macosx_x64/protoc" /> + <file src="protoc_plugins/macosx_x64/grpc_csharp_plugin" target="tools/macosx_x64/grpc_csharp_plugin" /> </files> </package> From e480a6a1f91e1167c33d08a208e3fabfee940584 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch <jtattermusch@google.com> Date: Fri, 7 Oct 2016 12:59:08 +0200 Subject: [PATCH 80/83] dont eat run_tests.py errors on test failure --- tools/run_tests/dockerize/build_docker_and_run_tests.sh | 8 +++----- tools/run_tests/run_tests.py | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/tools/run_tests/dockerize/build_docker_and_run_tests.sh b/tools/run_tests/dockerize/build_docker_and_run_tests.sh index b4b172ddef6..c3219c533d5 100755 --- a/tools/run_tests/dockerize/build_docker_and_run_tests.sh +++ b/tools/run_tests/dockerize/build_docker_and_run_tests.sh @@ -61,6 +61,7 @@ CONTAINER_NAME="run_tests_$(uuidgen)" docker_instance_git_root=/var/local/jenkins/grpc # Run tests inside docker +DOCKER_EXIT_CODE=0 docker run \ -e "RUN_TESTS_COMMAND=$RUN_TESTS_COMMAND" \ -e "config=$config" \ @@ -81,7 +82,7 @@ docker run \ -w /var/local/git/grpc \ --name=$CONTAINER_NAME \ $DOCKER_IMAGE_NAME \ - bash -l "/var/local/jenkins/grpc/$DOCKER_RUN_SCRIPT" || DOCKER_FAILED="true" + bash -l "/var/local/jenkins/grpc/$DOCKER_RUN_SCRIPT" || DOCKER_EXIT_CODE=$? # use unique name for reports.zip to prevent clash between concurrent # run_tests.py runs @@ -93,7 +94,4 @@ rm -f ${TEMP_REPORTS_ZIP} # remove the container, possibly killing it first docker rm -f $CONTAINER_NAME || true -if [ "$DOCKER_FAILED" != "" ] && [ "$XML_REPORT" == "" ] -then - exit 1 -fi +exit $DOCKER_EXIT_CODE diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index 168331602ca..a6f3d405dc2 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -1423,7 +1423,7 @@ else: exit_code = 0 if BuildAndRunError.BUILD in errors: exit_code |= 1 - if BuildAndRunError.TEST in errors and not args.travis: + if BuildAndRunError.TEST in errors: exit_code |= 2 if BuildAndRunError.POST_TEST in errors: exit_code |= 4 From ab7abdb968d0a455e3b0ed2f33722243283e9bb9 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch <jtattermusch@google.com> Date: Fri, 7 Oct 2016 12:59:08 +0200 Subject: [PATCH 81/83] dont eat run_tests.py errors on test failure --- tools/run_tests/dockerize/build_docker_and_run_tests.sh | 8 +++----- tools/run_tests/run_tests.py | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/tools/run_tests/dockerize/build_docker_and_run_tests.sh b/tools/run_tests/dockerize/build_docker_and_run_tests.sh index b4b172ddef6..c3219c533d5 100755 --- a/tools/run_tests/dockerize/build_docker_and_run_tests.sh +++ b/tools/run_tests/dockerize/build_docker_and_run_tests.sh @@ -61,6 +61,7 @@ CONTAINER_NAME="run_tests_$(uuidgen)" docker_instance_git_root=/var/local/jenkins/grpc # Run tests inside docker +DOCKER_EXIT_CODE=0 docker run \ -e "RUN_TESTS_COMMAND=$RUN_TESTS_COMMAND" \ -e "config=$config" \ @@ -81,7 +82,7 @@ docker run \ -w /var/local/git/grpc \ --name=$CONTAINER_NAME \ $DOCKER_IMAGE_NAME \ - bash -l "/var/local/jenkins/grpc/$DOCKER_RUN_SCRIPT" || DOCKER_FAILED="true" + bash -l "/var/local/jenkins/grpc/$DOCKER_RUN_SCRIPT" || DOCKER_EXIT_CODE=$? # use unique name for reports.zip to prevent clash between concurrent # run_tests.py runs @@ -93,7 +94,4 @@ rm -f ${TEMP_REPORTS_ZIP} # remove the container, possibly killing it first docker rm -f $CONTAINER_NAME || true -if [ "$DOCKER_FAILED" != "" ] && [ "$XML_REPORT" == "" ] -then - exit 1 -fi +exit $DOCKER_EXIT_CODE diff --git a/tools/run_tests/run_tests.py b/tools/run_tests/run_tests.py index 3ccba877c99..c7d10e057f5 100755 --- a/tools/run_tests/run_tests.py +++ b/tools/run_tests/run_tests.py @@ -1369,7 +1369,7 @@ else: exit_code = 0 if BuildAndRunError.BUILD in errors: exit_code |= 1 - if BuildAndRunError.TEST in errors and not args.travis: + if BuildAndRunError.TEST in errors: exit_code |= 2 if BuildAndRunError.POST_TEST in errors: exit_code |= 4 From 1e5f6af0c7deda7a101d9842fba78682f5c7aa0c Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" <roth@google.com> Date: Fri, 7 Oct 2016 08:32:58 -0700 Subject: [PATCH 82/83] Fix grpclb LB policy pick method to return 0 upon error. --- src/core/ext/client_config/lb_policy.h | 21 ++++++++++++--------- src/core/ext/lb_policy/grpclb/grpclb.c | 2 +- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/core/ext/client_config/lb_policy.h b/src/core/ext/client_config/lb_policy.h index 6cc3e1ebd38..110d08fcac1 100644 --- a/src/core/ext/client_config/lb_policy.h +++ b/src/core/ext/client_config/lb_policy.h @@ -142,15 +142,18 @@ void grpc_lb_policy_weak_unref(grpc_exec_ctx *exec_ctx, grpc_lb_policy *policy); void grpc_lb_policy_init(grpc_lb_policy *policy, const grpc_lb_policy_vtable *vtable); -/** Find an appropriate target for this call, based on \a pick_args. - Picking can be synchronous or asynchronous. In the synchronous case, when a - pick is readily available, it'll be returned in \a target and a non-zero - value will be returned. - In the asynchronous case, zero is returned and \a on_complete will be called - once \a target and \a user_data have been set. Any IO should be done under - \a pick_args->pollent. The opaque \a user_data output argument corresponds - to information that may need be propagated from the LB policy. It may be - NULL. Errors are signaled by receiving a NULL \a *target. */ +/** Finds an appropriate subchannel for a call, based on \a pick_args. + + \a target will be set to the selected subchannel, or NULL on failure. + Upon success, \a user_data will be set to whatever opaque information + may need to be propagated from the LB policy, or NULL if not needed. + + If the pick succeeds and a result is known immediately, a non-zero + value will be returned. Otherwise, \a on_complete will be invoked + once the pick is complete with its error argument set to indicate + success or failure. + + Any I/O should be done under \a pick_args->pollent. */ int grpc_lb_policy_pick(grpc_exec_ctx *exec_ctx, grpc_lb_policy *policy, const grpc_lb_policy_pick_args *pick_args, grpc_connected_subchannel **target, void **user_data, diff --git a/src/core/ext/lb_policy/grpclb/grpclb.c b/src/core/ext/lb_policy/grpclb/grpclb.c index 63af774ea6d..ae1f2a3b4c6 100644 --- a/src/core/ext/lb_policy/grpclb/grpclb.c +++ b/src/core/ext/lb_policy/grpclb/grpclb.c @@ -761,7 +761,7 @@ static int glb_pick(grpc_exec_ctx *exec_ctx, grpc_lb_policy *pol, GRPC_ERROR_CREATE("No mdelem storage for the LB token. Load reporting " "won't work without it. Failing"), NULL); - return 1; + return 0; } glb_lb_policy *glb_policy = (glb_lb_policy *)pol; From cf16b76b9a85c801c51aab3b3fe7a1735a6839a4 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" <roth@google.com> Date: Fri, 7 Oct 2016 09:51:03 -0700 Subject: [PATCH 83/83] clang-format --- src/core/ext/transport/cronet/transport/cronet_transport.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/ext/transport/cronet/transport/cronet_transport.c b/src/core/ext/transport/cronet/transport/cronet_transport.c index 00a5be419ef..25ad40b935a 100644 --- a/src/core/ext/transport/cronet/transport/cronet_transport.c +++ b/src/core/ext/transport/cronet/transport/cronet_transport.c @@ -543,7 +543,7 @@ static void create_grpc_frame(gpr_slice_buffer *write_slice_buffer, static void convert_metadata_to_cronet_headers( grpc_linked_mdelem *head, const char *host, char **pp_url, cronet_bidirectional_stream_header **pp_headers, size_t *p_num_headers, - const char ** method) { + const char **method) { grpc_linked_mdelem *curr = head; /* Walk the linked list and get number of header fields */ size_t num_headers_available = 0;