From 7fc6f3be56a285bea98c9c2e980fa2720edf87b9 Mon Sep 17 00:00:00 2001 From: Makarand Dharmapurikar Date: Mon, 9 Jan 2017 10:55:51 -0800 Subject: [PATCH 1/5] changes to http2 test server Not kill the server on disconnect Spawn one server per test case --- test/http2_test/http2_base_server.py | 1 - test/http2_test/http2_test_server.py | 34 ++++++++++++++++++++-------- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/test/http2_test/http2_base_server.py b/test/http2_test/http2_base_server.py index ee7719b1a80..8de028ceb1b 100644 --- a/test/http2_test/http2_base_server.py +++ b/test/http2_test/http2_base_server.py @@ -73,7 +73,6 @@ class H2ProtocolBaseServer(twisted.internet.protocol.Protocol): def on_connection_lost(self, reason): logging.info('Disconnected %s' % reason) - twisted.internet.reactor.callFromThread(twisted.internet.reactor.stop) def dataReceived(self, data): try: diff --git a/test/http2_test/http2_test_server.py b/test/http2_test/http2_test_server.py index 44e36d34b64..abde3433ad2 100644 --- a/test/http2_test/http2_test_server.py +++ b/test/http2_test/http2_test_server.py @@ -73,18 +73,32 @@ class H2Factory(twisted.internet.protocol.Factory): else: return t().get_base_server() +def parse_arguments(): + parser = argparse.ArgumentParser() + parser.add_argument('--base_port', type=int, default=8080, + help='base port to run the servers (default: 8080). One test server is ' + 'started on each incrementing port, beginning with base_port, in the ' + 'following order: goaway,max_streams,ping,rst_after_data,rst_after_header,' + 'rst_during_data' + ) + return parser.parse_args() + +def start_test_servers(base_port): + """ Start one server per test case on incrementing port numbers + beginning with base_port """ + index = 0 + for test_case in sorted(_TEST_CASE_MAPPING.keys()): + portnum = base_port + index + logging.warning('serving on port %d : %s'%(portnum, test_case)) + endpoint = twisted.internet.endpoints.TCP4ServerEndpoint( + twisted.internet.reactor, portnum, backlog=128) + endpoint.listen(H2Factory(test_case)) + index += 1 + if __name__ == '__main__': logging.basicConfig( format='%(levelname) -10s %(asctime)s %(module)s:%(lineno)s | %(message)s', level=logging.INFO) - parser = argparse.ArgumentParser() - parser.add_argument('--test_case', choices=sorted(_TEST_CASE_MAPPING.keys()), - help='test case to run', required=True) - parser.add_argument('--port', type=int, default=8080, - help='port to run the server (default: 8080)') - args = parser.parse_args() - logging.info('Running test case %s on port %d' % (args.test_case, args.port)) - endpoint = twisted.internet.endpoints.TCP4ServerEndpoint( - twisted.internet.reactor, args.port, backlog=128) - endpoint.listen(H2Factory(args.test_case)) + args = parse_arguments() + start_test_servers(args.base_port) twisted.internet.reactor.run() From 49f02d3697ed109b514edc7e746088b8e5d7b819 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Mon, 9 Jan 2017 14:54:46 -0800 Subject: [PATCH 2/5] Add support for sending custom headers in HTTP CONNECT request. --- .../client_channel/http_connect_handshaker.c | 24 +++++++++++++++++-- .../client_channel/http_connect_handshaker.h | 5 +++- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/src/core/ext/client_channel/http_connect_handshaker.c b/src/core/ext/client_channel/http_connect_handshaker.c index ace804c47f0..0561c81aa5a 100644 --- a/src/core/ext/client_channel/http_connect_handshaker.c +++ b/src/core/ext/client_channel/http_connect_handshaker.c @@ -55,6 +55,8 @@ typedef struct http_connect_handshaker { grpc_handshaker base; char* proxy_server; + grpc_http_header* headers; + size_t num_headers; gpr_refcount refcount; gpr_mu mu; @@ -90,6 +92,11 @@ static void http_connect_handshaker_unref(grpc_exec_ctx* exec_ctx, gpr_free(handshaker->read_buffer_to_destroy); } gpr_free(handshaker->proxy_server); + for (size_t i = 0; i < handshaker->num_headers; ++i) { + gpr_free(handshaker->headers[i].key); + gpr_free(handshaker->headers[i].value); + } + gpr_free(handshaker->headers); grpc_slice_buffer_destroy_internal(exec_ctx, &handshaker->write_buffer); grpc_http_parser_destroy(&handshaker->http_parser); grpc_http_response_destroy(&handshaker->http_response); @@ -290,6 +297,8 @@ static void http_connect_handshaker_do_handshake( request.host = server_name; request.http.method = "CONNECT"; request.http.path = server_name; + request.http.hdrs = handshaker->headers; + request.http.hdr_count = handshaker->num_headers; request.handshaker = &grpc_httpcli_plaintext; grpc_slice request_slice = grpc_httpcli_format_connect_request(&request); grpc_slice_buffer_add(&handshaker->write_buffer, request_slice); @@ -307,7 +316,9 @@ static const grpc_handshaker_vtable http_connect_handshaker_vtable = { http_connect_handshaker_destroy, http_connect_handshaker_shutdown, http_connect_handshaker_do_handshake}; -grpc_handshaker* grpc_http_connect_handshaker_create(const char* proxy_server) { +grpc_handshaker* grpc_http_connect_handshaker_create(const char* proxy_server, + grpc_http_header* headers, + size_t num_headers) { GPR_ASSERT(proxy_server != NULL); http_connect_handshaker* handshaker = gpr_malloc(sizeof(*handshaker)); memset(handshaker, 0, sizeof(*handshaker)); @@ -315,6 +326,14 @@ grpc_handshaker* grpc_http_connect_handshaker_create(const char* proxy_server) { gpr_mu_init(&handshaker->mu); gpr_ref_init(&handshaker->refcount, 1); handshaker->proxy_server = gpr_strdup(proxy_server); + if (num_headers > 0) { + handshaker->headers = gpr_malloc(sizeof(grpc_http_header) * num_headers); + for (size_t i = 0; i < num_headers; ++i) { + handshaker->headers[i].key = gpr_strdup(headers[i].key); + handshaker->headers[i].value = gpr_strdup(headers[i].value); + } + handshaker->num_headers = num_headers; + } grpc_slice_buffer_init(&handshaker->write_buffer); grpc_closure_init(&handshaker->request_done_closure, on_write_done, handshaker, grpc_schedule_on_exec_ctx); @@ -359,7 +378,8 @@ static void handshaker_factory_add_handshakers( char* proxy_name = grpc_get_http_proxy_server(); if (proxy_name != NULL) { grpc_handshake_manager_add(handshake_mgr, - grpc_http_connect_handshaker_create(proxy_name)); + grpc_http_connect_handshaker_create(proxy_name, + NULL, 0)); gpr_free(proxy_name); } } diff --git a/src/core/ext/client_channel/http_connect_handshaker.h b/src/core/ext/client_channel/http_connect_handshaker.h index 56485f13735..c2e68de7167 100644 --- a/src/core/ext/client_channel/http_connect_handshaker.h +++ b/src/core/ext/client_channel/http_connect_handshaker.h @@ -35,9 +35,12 @@ #define GRPC_CORE_EXT_CLIENT_CHANNEL_HTTP_CONNECT_HANDSHAKER_H #include "src/core/lib/channel/handshaker.h" +#include "src/core/lib/http/parser.h" /// Creates a new HTTP CONNECT handshaker. -grpc_handshaker* grpc_http_connect_handshaker_create(const char* proxy_server); +grpc_handshaker* grpc_http_connect_handshaker_create(const char* proxy_server, + grpc_http_header* headers, + size_t num_headers); /// Returns the name of the proxy to use, or NULL if no proxy is configured. /// Caller takes ownership of result. From 466589606a933bfcc453c51daef8fc7f689001e0 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Mon, 9 Jan 2017 14:56:18 -0800 Subject: [PATCH 3/5] clang-format --- src/core/ext/client_channel/http_connect_handshaker.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/core/ext/client_channel/http_connect_handshaker.c b/src/core/ext/client_channel/http_connect_handshaker.c index 0561c81aa5a..fba32561acf 100644 --- a/src/core/ext/client_channel/http_connect_handshaker.c +++ b/src/core/ext/client_channel/http_connect_handshaker.c @@ -377,9 +377,9 @@ static void handshaker_factory_add_handshakers( const grpc_channel_args* args, grpc_handshake_manager* handshake_mgr) { char* proxy_name = grpc_get_http_proxy_server(); if (proxy_name != NULL) { - grpc_handshake_manager_add(handshake_mgr, - grpc_http_connect_handshaker_create(proxy_name, - NULL, 0)); + grpc_handshake_manager_add( + handshake_mgr, + grpc_http_connect_handshaker_create(proxy_name, NULL, 0)); gpr_free(proxy_name); } } From bf07d75311b5b5a988913572853ec6dc75cd07c0 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Tue, 10 Jan 2017 11:03:46 -0800 Subject: [PATCH 4/5] Ran generate_projects.sh --- tools/doxygen/Doxyfile.c++ | 38 ++++++++++++------------ tools/doxygen/Doxyfile.c++.internal | 36 +++++++++++------------ tools/doxygen/Doxyfile.core | 34 ++++++++++----------- tools/doxygen/Doxyfile.core.internal | 44 ++++++++++++++-------------- 4 files changed, 76 insertions(+), 76 deletions(-) diff --git a/tools/doxygen/Doxyfile.c++ b/tools/doxygen/Doxyfile.c++ index 6539f965658..eef7ff4d5db 100644 --- a/tools/doxygen/Doxyfile.c++ +++ b/tools/doxygen/Doxyfile.c++ @@ -848,34 +848,34 @@ include/grpc/impl/codegen/sync.h \ include/grpc/impl/codegen/sync_generic.h \ include/grpc/impl/codegen/sync_posix.h \ include/grpc/impl/codegen/sync_windows.h \ +doc/fail_fast.md \ +doc/g_stands_for.md \ +doc/wait-for-ready.md \ doc/binary-logging.md \ -doc/c-style-guide.md \ -doc/command_line_tool.md \ doc/compression.md \ +doc/c-style-guide.md \ +doc/PROTOCOL-WEB.md \ +doc/cpp-style-guide.md \ +doc/server_reflection_tutorial.md \ +doc/server-reflection.md \ +doc/interop-test-descriptions.md \ +doc/PROTOCOL-HTTP2.md \ doc/compression_cookbook.md \ -doc/connection-backoff-interop-test-description.md \ doc/connection-backoff.md \ -doc/connectivity-semantics-and-api.md \ -doc/cpp-style-guide.md \ -doc/environment_variables.md \ doc/epoll-polling-engine.md \ -doc/fail_fast.md \ -doc/g_stands_for.md \ -doc/health-checking.md \ -doc/http-grpc-status-mapping.md \ -doc/interop-test-descriptions.md \ +doc/connectivity-semantics-and-api.md \ doc/load-balancing.md \ +doc/environment_variables.md \ doc/naming.md \ +doc/stress_test_framework.md \ +doc/http-grpc-status-mapping.md \ +doc/health-checking.md \ +doc/connection-backoff-interop-test-description.md \ doc/negative-http2-interop-test-descriptions.md \ -doc/PROTOCOL-HTTP2.md \ -doc/PROTOCOL-WEB.md \ -doc/server-reflection.md \ -doc/server_reflection_tutorial.md \ +doc/command_line_tool.md \ doc/statuscodes.md \ -doc/stress_test_framework.md \ -doc/wait-for-ready.md \ -doc/cpp/pending_api_cleanups.md \ -doc/cpp/perf_notes.md +doc/cpp/perf_notes.md \ +doc/cpp/pending_api_cleanups.md # This tag can be used to specify the character encoding of the source files # that doxygen parses. Internally doxygen uses the UTF-8 encoding. Doxygen uses diff --git a/tools/doxygen/Doxyfile.c++.internal b/tools/doxygen/Doxyfile.c++.internal index ffef534fe27..e8c47237ca4 100644 --- a/tools/doxygen/Doxyfile.c++.internal +++ b/tools/doxygen/Doxyfile.c++.internal @@ -894,34 +894,34 @@ src/cpp/util/status.cc \ src/cpp/util/string_ref.cc \ src/cpp/util/time_cc.cc \ src/cpp/codegen/codegen_init.cc \ +doc/fail_fast.md \ +doc/g_stands_for.md \ +doc/wait-for-ready.md \ doc/binary-logging.md \ -doc/c-style-guide.md \ -doc/command_line_tool.md \ doc/compression.md \ +doc/c-style-guide.md \ +doc/PROTOCOL-WEB.md \ +doc/cpp-style-guide.md \ +doc/server_reflection_tutorial.md \ +doc/server-reflection.md \ +doc/interop-test-descriptions.md \ +doc/PROTOCOL-HTTP2.md \ doc/compression_cookbook.md \ -doc/connection-backoff-interop-test-description.md \ doc/connection-backoff.md \ -doc/connectivity-semantics-and-api.md \ -doc/cpp-style-guide.md \ -doc/environment_variables.md \ doc/epoll-polling-engine.md \ -doc/fail_fast.md \ -doc/g_stands_for.md \ -doc/health-checking.md \ -doc/http-grpc-status-mapping.md \ -doc/interop-test-descriptions.md \ +doc/connectivity-semantics-and-api.md \ doc/load-balancing.md \ +doc/environment_variables.md \ doc/naming.md \ +doc/stress_test_framework.md \ +doc/http-grpc-status-mapping.md \ +doc/health-checking.md \ +doc/connection-backoff-interop-test-description.md \ doc/negative-http2-interop-test-descriptions.md \ -doc/PROTOCOL-HTTP2.md \ -doc/PROTOCOL-WEB.md \ -doc/server-reflection.md \ -doc/server_reflection_tutorial.md \ +doc/command_line_tool.md \ doc/statuscodes.md \ -doc/stress_test_framework.md \ -doc/wait-for-ready.md \ -doc/cpp/pending_api_cleanups.md \ doc/cpp/perf_notes.md \ +doc/cpp/pending_api_cleanups.md \ src/cpp/README.md # This tag can be used to specify the character encoding of the source files diff --git a/tools/doxygen/Doxyfile.core b/tools/doxygen/Doxyfile.core index f8a3970416d..92b57610dca 100644 --- a/tools/doxygen/Doxyfile.core +++ b/tools/doxygen/Doxyfile.core @@ -826,32 +826,32 @@ include/grpc/impl/codegen/sync.h \ include/grpc/impl/codegen/sync_generic.h \ include/grpc/impl/codegen/sync_posix.h \ include/grpc/impl/codegen/sync_windows.h \ +doc/fail_fast.md \ +doc/g_stands_for.md \ +doc/wait-for-ready.md \ doc/binary-logging.md \ -doc/c-style-guide.md \ -doc/command_line_tool.md \ doc/compression.md \ +doc/c-style-guide.md \ +doc/PROTOCOL-WEB.md \ +doc/cpp-style-guide.md \ +doc/server_reflection_tutorial.md \ +doc/server-reflection.md \ +doc/interop-test-descriptions.md \ +doc/PROTOCOL-HTTP2.md \ doc/compression_cookbook.md \ -doc/connection-backoff-interop-test-description.md \ doc/connection-backoff.md \ -doc/connectivity-semantics-and-api.md \ -doc/cpp-style-guide.md \ -doc/environment_variables.md \ doc/epoll-polling-engine.md \ -doc/fail_fast.md \ -doc/g_stands_for.md \ -doc/health-checking.md \ -doc/http-grpc-status-mapping.md \ -doc/interop-test-descriptions.md \ +doc/connectivity-semantics-and-api.md \ doc/load-balancing.md \ +doc/environment_variables.md \ doc/naming.md \ +doc/stress_test_framework.md \ +doc/http-grpc-status-mapping.md \ +doc/health-checking.md \ +doc/connection-backoff-interop-test-description.md \ doc/negative-http2-interop-test-descriptions.md \ -doc/PROTOCOL-HTTP2.md \ -doc/PROTOCOL-WEB.md \ -doc/server-reflection.md \ -doc/server_reflection_tutorial.md \ +doc/command_line_tool.md \ doc/statuscodes.md \ -doc/stress_test_framework.md \ -doc/wait-for-ready.md \ doc/core/pending_api_cleanups.md # This tag can be used to specify the character encoding of the source files diff --git a/tools/doxygen/Doxyfile.core.internal b/tools/doxygen/Doxyfile.core.internal index 1a945bec49f..04acd4ff1e7 100644 --- a/tools/doxygen/Doxyfile.core.internal +++ b/tools/doxygen/Doxyfile.core.internal @@ -1281,41 +1281,39 @@ src/core/lib/support/tmpfile_msys.c \ src/core/lib/support/tmpfile_posix.c \ src/core/lib/support/tmpfile_windows.c \ src/core/lib/support/wrap_memcpy.c \ +doc/fail_fast.md \ +doc/g_stands_for.md \ +doc/wait-for-ready.md \ doc/binary-logging.md \ -doc/c-style-guide.md \ -doc/command_line_tool.md \ doc/compression.md \ +doc/c-style-guide.md \ +doc/PROTOCOL-WEB.md \ +doc/cpp-style-guide.md \ +doc/server_reflection_tutorial.md \ +doc/server-reflection.md \ +doc/interop-test-descriptions.md \ +doc/PROTOCOL-HTTP2.md \ doc/compression_cookbook.md \ -doc/connection-backoff-interop-test-description.md \ doc/connection-backoff.md \ -doc/connectivity-semantics-and-api.md \ -doc/cpp-style-guide.md \ -doc/environment_variables.md \ doc/epoll-polling-engine.md \ -doc/fail_fast.md \ -doc/g_stands_for.md \ -doc/health-checking.md \ -doc/http-grpc-status-mapping.md \ -doc/interop-test-descriptions.md \ +doc/connectivity-semantics-and-api.md \ doc/load-balancing.md \ +doc/environment_variables.md \ doc/naming.md \ +doc/stress_test_framework.md \ +doc/http-grpc-status-mapping.md \ +doc/health-checking.md \ +doc/connection-backoff-interop-test-description.md \ doc/negative-http2-interop-test-descriptions.md \ -doc/PROTOCOL-HTTP2.md \ -doc/PROTOCOL-WEB.md \ -doc/server-reflection.md \ -doc/server_reflection_tutorial.md \ +doc/command_line_tool.md \ doc/statuscodes.md \ -doc/stress_test_framework.md \ -doc/wait-for-ready.md \ doc/core/pending_api_cleanups.md \ src/core/README.md \ src/core/ext/README.md \ -src/core/ext/census/README.md \ -src/core/ext/census/gen/README.md \ -src/core/ext/client_channel/README.md \ src/core/ext/resolver/README.md \ src/core/ext/resolver/dns/native/README.md \ src/core/ext/resolver/sockaddr/README.md \ +src/core/ext/client_channel/README.md \ src/core/ext/transport/README.md \ src/core/ext/transport/chttp2/README.md \ src/core/ext/transport/chttp2/client/insecure/README.md \ @@ -1323,12 +1321,14 @@ src/core/ext/transport/chttp2/client/secure/README.md \ src/core/ext/transport/chttp2/server/insecure/README.md \ src/core/ext/transport/chttp2/server/secure/README.md \ src/core/ext/transport/chttp2/transport/README.md \ +src/core/ext/census/README.md \ +src/core/ext/census/gen/README.md \ src/core/lib/README.md \ -src/core/lib/channel/README.md \ src/core/lib/iomgr/README.md \ +src/core/lib/tsi/README.md \ src/core/lib/surface/README.md \ src/core/lib/transport/README.md \ -src/core/lib/tsi/README.md +src/core/lib/channel/README.md # This tag can be used to specify the character encoding of the source files # that doxygen parses. Internally doxygen uses the UTF-8 encoding. Doxygen uses From d70c8bb871c68d8d8fd96660236c96e48d71c00a Mon Sep 17 00:00:00 2001 From: ncteisen Date: Tue, 10 Jan 2017 11:58:40 -0800 Subject: [PATCH 5/5] Implement wait-for-ready behavior in Python stress and qps client The clients now block until the channel is in the READY state. This fixes some test flakiness issues we have had. --- src/python/grpcio_tests/tests/qps/benchmark_client.py | 8 ++------ src/python/grpcio_tests/tests/stress/client.py | 9 ++++++--- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/python/grpcio_tests/tests/qps/benchmark_client.py b/src/python/grpcio_tests/tests/qps/benchmark_client.py index 83b46c914e7..650e4756e72 100644 --- a/src/python/grpcio_tests/tests/qps/benchmark_client.py +++ b/src/python/grpcio_tests/tests/qps/benchmark_client.py @@ -68,12 +68,8 @@ class BenchmarkClient: else: channel = grpc.insecure_channel(server) - connected_event = threading.Event() - def wait_for_ready(connectivity): - if connectivity == grpc.ChannelConnectivity.READY: - connected_event.set() - channel.subscribe(wait_for_ready, try_to_connect=True) - connected_event.wait() + # waits for the channel to be ready before we start sending messages + grpc.channel_ready_future(channel).result() if config.payload_config.WhichOneof('payload') == 'simple_params': self._generic = False diff --git a/src/python/grpcio_tests/tests/stress/client.py b/src/python/grpcio_tests/tests/stress/client.py index 390ea13021d..b8116729b57 100644 --- a/src/python/grpcio_tests/tests/stress/client.py +++ b/src/python/grpcio_tests/tests/stress/client.py @@ -110,10 +110,13 @@ def _get_channel(target, args): channel_credentials = grpc.ssl_channel_credentials( root_certificates=root_certificates) options = (('grpc.ssl_target_name_override', args.server_host_override,),) - return grpc.secure_channel( - target, channel_credentials, options=options) + channel = grpc.secure_channel(target, channel_credentials, options=options) else: - return grpc.insecure_channel(target) + channel = grpc.insecure_channel(target) + + # waits for the channel to be ready before we start sending messages + grpc.channel_ready_future(channel).result() + return channel def run_test(args): test_cases = _parse_weighted_test_cases(args.test_cases)