From aec96aa2232b3ab7f86bc08a99f82684406edf0b Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 7 Apr 2015 14:32:15 -0700 Subject: [PATCH 01/11] Fix server shutdown A previous fix to make close() occur later can cause socket reuse by servers to fail as previous sockets are left asynchronously open. This change: - adds a callback to TCP server shutdown to signal that the server is completely shutdown - wait for that callback before destroying listeners in the server (and before destroying the server) - handles fallout --- src/core/iomgr/tcp_server.h | 4 +- src/core/iomgr/tcp_server_posix.c | 58 ++++++++++++++++--- src/core/security/server_secure_chttp2.c | 4 +- src/core/surface/completion_queue.c | 7 +++ src/core/surface/completion_queue.h | 2 + src/core/surface/server.c | 44 ++++++++++++-- src/core/surface/server.h | 2 + src/core/surface/server_chttp2.c | 4 +- test/core/end2end/tests/cancel_after_invoke.c | 5 +- test/core/end2end/tests/cancel_test_helpers.h | 5 +- test/core/iomgr/tcp_server_posix_test.c | 10 ++-- 11 files changed, 117 insertions(+), 28 deletions(-) diff --git a/src/core/iomgr/tcp_server.h b/src/core/iomgr/tcp_server.h index 68ee85c5a7f..1e58901a7a6 100644 --- a/src/core/iomgr/tcp_server.h +++ b/src/core/iomgr/tcp_server.h @@ -71,6 +71,8 @@ int grpc_tcp_server_add_port(grpc_tcp_server *s, const void *addr, up when grpc_tcp_server_destroy is called. */ int grpc_tcp_server_get_fd(grpc_tcp_server *s, unsigned index); -void grpc_tcp_server_destroy(grpc_tcp_server *server); +void grpc_tcp_server_destroy(grpc_tcp_server *server, + void (*shutdown_done)(void *shutdown_done_arg), + void *shutdown_done_arg); #endif /* GRPC_INTERNAL_CORE_IOMGR_TCP_SERVER_H */ diff --git a/src/core/iomgr/tcp_server_posix.c b/src/core/iomgr/tcp_server_posix.c index 90b7eb451d4..482166e2eb6 100644 --- a/src/core/iomgr/tcp_server_posix.c +++ b/src/core/iomgr/tcp_server_posix.c @@ -102,12 +102,18 @@ struct grpc_tcp_server { gpr_cv cv; /* active port count: how many ports are actually still listening */ - int active_ports; + size_t active_ports; + /* destroyed port count: how many ports are completely destroyed */ + size_t destroyed_ports; /* all listening ports */ server_port *ports; size_t nports; size_t port_capacity; + + /* shutdown callback */ + void (*shutdown_complete)(void *); + void *shutdown_complete_arg; }; grpc_tcp_server *grpc_tcp_server_create(void) { @@ -115,6 +121,7 @@ grpc_tcp_server *grpc_tcp_server_create(void) { gpr_mu_init(&s->mu); gpr_cv_init(&s->cv); s->active_ports = 0; + s->destroyed_ports = 0; s->cb = NULL; s->cb_arg = NULL; s->ports = gpr_malloc(sizeof(server_port) * INIT_PORT_CAP); @@ -123,29 +130,62 @@ grpc_tcp_server *grpc_tcp_server_create(void) { return s; } -void grpc_tcp_server_destroy(grpc_tcp_server *s) { +static void finish_shutdown(grpc_tcp_server *s) { + s->shutdown_complete(s->shutdown_complete_arg); + + gpr_mu_destroy(&s->mu); + gpr_cv_destroy(&s->cv); + + gpr_free(s->ports); + gpr_free(s); +} + +static void destroyed_port(void *server, int success) { + grpc_tcp_server *s = server; + gpr_mu_lock(&s->mu); + s->destroyed_ports++; + if (s->destroyed_ports == s->nports) { + gpr_mu_unlock(&s->mu); + finish_shutdown(s); + } else { + gpr_mu_unlock(&s->mu); + } +} + +static void dont_care_about_shutdown_completion(void *ignored) {} + +void grpc_tcp_server_destroy(grpc_tcp_server *s, + void (*shutdown_complete)(void *shutdown_complete_arg), + void *shutdown_complete_arg) { size_t i; gpr_mu_lock(&s->mu); + + s->shutdown_complete = shutdown_complete ? shutdown_complete : dont_care_about_shutdown_completion; + s->shutdown_complete_arg = shutdown_complete_arg; + /* shutdown all fd's */ for (i = 0; i < s->nports; i++) { grpc_fd_shutdown(s->ports[i].emfd); } /* wait while that happens */ + /* TODO(ctiller): make this asynchronous also */ while (s->active_ports) { gpr_cv_wait(&s->cv, &s->mu, gpr_inf_future); } gpr_mu_unlock(&s->mu); /* delete ALL the things */ - for (i = 0; i < s->nports; i++) { - server_port *sp = &s->ports[i]; - if (sp->addr.sockaddr.sa_family == AF_UNIX) { - unlink_if_unix_domain_socket(&sp->addr.un); + if (s->nports) { + for (i = 0; i < s->nports; i++) { + server_port *sp = &s->ports[i]; + if (sp->addr.sockaddr.sa_family == AF_UNIX) { + unlink_if_unix_domain_socket(&sp->addr.un); + } + grpc_fd_orphan(sp->emfd, destroyed_port, s); } - grpc_fd_orphan(sp->emfd, NULL, NULL); + } else { + finish_shutdown(s); } - gpr_free(s->ports); - gpr_free(s); } /* get max listen queue size on linux */ diff --git a/src/core/security/server_secure_chttp2.c b/src/core/security/server_secure_chttp2.c index c155b80b7e6..d5b3a82b878 100644 --- a/src/core/security/server_secure_chttp2.c +++ b/src/core/security/server_secure_chttp2.c @@ -120,7 +120,7 @@ static void destroy(grpc_server *server, void *statep) { grpc_server_secure_state *state = statep; gpr_mu_lock(&state->mu); state->is_shutdown = 1; - grpc_tcp_server_destroy(state->tcp); + grpc_tcp_server_destroy(state->tcp, grpc_server_listener_destroy_done, server); gpr_mu_unlock(&state->mu); state_unref(state); } @@ -213,7 +213,7 @@ error: grpc_resolved_addresses_destroy(resolved); } if (tcp) { - grpc_tcp_server_destroy(tcp); + grpc_tcp_server_destroy(tcp, NULL, NULL); } if (state) { gpr_free(state); diff --git a/src/core/surface/completion_queue.c b/src/core/surface/completion_queue.c index 6a1d83ce5d4..b08bd93693e 100644 --- a/src/core/surface/completion_queue.c +++ b/src/core/surface/completion_queue.c @@ -432,3 +432,10 @@ void grpc_cq_dump_pending_ops(grpc_completion_queue *cc) { grpc_pollset *grpc_cq_pollset(grpc_completion_queue *cc) { return &cc->pollset; } + +void grpc_cq_hack_spin_pollset(grpc_completion_queue *cc) { + gpr_mu_lock(GRPC_POLLSET_MU(&cc->pollset)); + grpc_pollset_kick(&cc->pollset); + grpc_pollset_work(&cc->pollset, gpr_time_add(gpr_now(), gpr_time_from_millis(100))); + gpr_mu_unlock(GRPC_POLLSET_MU(&cc->pollset)); +} diff --git a/src/core/surface/completion_queue.h b/src/core/surface/completion_queue.h index 3054264cadf..3e9e2d186ef 100644 --- a/src/core/surface/completion_queue.h +++ b/src/core/surface/completion_queue.h @@ -114,4 +114,6 @@ void grpc_cq_dump_pending_ops(grpc_completion_queue *cc); grpc_pollset *grpc_cq_pollset(grpc_completion_queue *cc); +void grpc_cq_hack_spin_pollset(grpc_completion_queue *cc); + #endif /* GRPC_INTERNAL_CORE_SURFACE_COMPLETION_QUEUE_H */ diff --git a/src/core/surface/server.c b/src/core/surface/server.c index 424734c54ce..2e013ea7421 100644 --- a/src/core/surface/server.c +++ b/src/core/surface/server.c @@ -137,6 +137,7 @@ struct grpc_server { size_t cq_count; gpr_mu mu; + gpr_cv cv; registered_method *registered_methods; requested_call_array requested_calls; @@ -149,6 +150,7 @@ struct grpc_server { channel_data root_channel_data; listener *listeners; + int listeners_destroyed; gpr_refcount internal_refcount; }; @@ -263,6 +265,7 @@ static void server_unref(grpc_server *server) { if (gpr_unref(&server->internal_refcount)) { grpc_channel_args_destroy(server->channel_args); gpr_mu_destroy(&server->mu); + gpr_cv_destroy(&server->cv); gpr_free(server->channel_filters); requested_call_array_destroy(&server->requested_calls); while ((rm = server->registered_methods) != NULL) { @@ -620,6 +623,7 @@ grpc_server *grpc_server_create_from_filters(grpc_completion_queue *cq, if (cq) addcq(server, cq); gpr_mu_init(&server->mu); + gpr_cv_init(&server->cv); server->unregistered_cq = cq; /* decremented by grpc_server_destroy */ @@ -781,6 +785,15 @@ grpc_transport_setup_result grpc_server_setup_transport( return result; } +static int num_listeners(grpc_server *server) { + listener *l; + int n = 0; + for (l = server->listeners; l; l = l->next) { + n++; + } + return n; +} + static void shutdown_internal(grpc_server *server, gpr_uint8 have_shutdown_tag, void *shutdown_tag) { listener *l; @@ -878,11 +891,6 @@ static void shutdown_internal(grpc_server *server, gpr_uint8 have_shutdown_tag, for (l = server->listeners; l; l = l->next) { l->destroy(server, l->arg); } - while (server->listeners) { - l = server->listeners; - server->listeners = l->next; - gpr_free(l); - } } void grpc_server_shutdown(grpc_server *server) { @@ -893,8 +901,18 @@ void grpc_server_shutdown_and_notify(grpc_server *server, void *tag) { shutdown_internal(server, 1, tag); } +void grpc_server_listener_destroy_done(void *s) { + grpc_server *server = s; + gpr_mu_lock(&server->mu); + server->listeners_destroyed++; + gpr_cv_signal(&server->cv); + gpr_mu_unlock(&server->mu); +} + void grpc_server_destroy(grpc_server *server) { channel_data *c; + listener *l; + size_t i; gpr_mu_lock(&server->mu); if (!server->shutdown) { gpr_mu_unlock(&server->mu); @@ -902,6 +920,22 @@ void grpc_server_destroy(grpc_server *server) { gpr_mu_lock(&server->mu); } + while (server->listeners_destroyed != num_listeners(server)) { + for (i = 0; i < server->cq_count; i++) { + gpr_mu_unlock(&server->mu); + grpc_cq_hack_spin_pollset(server->cqs[i]); + gpr_mu_lock(&server->mu); + } + + gpr_cv_wait(&server->cv, &server->mu, gpr_time_add(gpr_now(), gpr_time_from_millis(100))); + } + + while (server->listeners) { + l = server->listeners; + server->listeners = l->next; + gpr_free(l); + } + for (c = server->root_channel_data.next; c != &server->root_channel_data; c = c->next) { shutdown_channel(c); diff --git a/src/core/surface/server.h b/src/core/surface/server.h index e33f69b8c75..548a16c6c96 100644 --- a/src/core/surface/server.h +++ b/src/core/surface/server.h @@ -51,6 +51,8 @@ void grpc_server_add_listener(grpc_server *server, void *listener, grpc_pollset **pollsets, size_t npollsets), void (*destroy)(grpc_server *server, void *arg)); +void grpc_server_listener_destroy_done(void *server); + /* Setup a transport - creates a channel stack, binds the transport to the server */ grpc_transport_setup_result grpc_server_setup_transport( diff --git a/src/core/surface/server_chttp2.c b/src/core/surface/server_chttp2.c index 27434b39e2d..9a23125752a 100644 --- a/src/core/surface/server_chttp2.c +++ b/src/core/surface/server_chttp2.c @@ -75,7 +75,7 @@ static void start(grpc_server *server, void *tcpp, grpc_pollset **pollsets, size callbacks) */ static void destroy(grpc_server *server, void *tcpp) { grpc_tcp_server *tcp = tcpp; - grpc_tcp_server_destroy(tcp); + grpc_tcp_server_destroy(tcp, grpc_server_listener_destroy_done, server); } int grpc_server_add_http2_port(grpc_server *server, const char *addr) { @@ -131,7 +131,7 @@ error: grpc_resolved_addresses_destroy(resolved); } if (tcp) { - grpc_tcp_server_destroy(tcp); + grpc_tcp_server_destroy(tcp, NULL, NULL); } return 0; } diff --git a/test/core/end2end/tests/cancel_after_invoke.c b/test/core/end2end/tests/cancel_after_invoke.c index e15fa7fcd91..326321f4e21 100644 --- a/test/core/end2end/tests/cancel_after_invoke.c +++ b/test/core/end2end/tests/cancel_after_invoke.c @@ -51,10 +51,11 @@ static void *tag(gpr_intptr t) { return (void *)t; } static grpc_end2end_test_fixture begin_test(grpc_end2end_test_config config, const char *test_name, + cancellation_mode mode, grpc_channel_args *client_args, grpc_channel_args *server_args) { grpc_end2end_test_fixture f; - gpr_log(GPR_INFO, "%s/%s", test_name, config.name); + gpr_log(GPR_INFO, "%s/%s/%s", test_name, config.name, mode.name); f = config.create_fixture(client_args, server_args); config.init_client(&f, client_args); config.init_server(&f, server_args); @@ -109,7 +110,7 @@ static void test_cancel_after_invoke(grpc_end2end_test_config config, grpc_op ops[6]; grpc_op *op; grpc_call *c; - grpc_end2end_test_fixture f = begin_test(config, __FUNCTION__, NULL, NULL); + grpc_end2end_test_fixture f = begin_test(config, __FUNCTION__, mode, NULL, NULL); gpr_timespec deadline = five_seconds_time(); cq_verifier *v_client = cq_verifier_create(f.client_cq); grpc_metadata_array initial_metadata_recv; diff --git a/test/core/end2end/tests/cancel_test_helpers.h b/test/core/end2end/tests/cancel_test_helpers.h index f2581dc32ff..3d92b64ae49 100644 --- a/test/core/end2end/tests/cancel_test_helpers.h +++ b/test/core/end2end/tests/cancel_test_helpers.h @@ -35,6 +35,7 @@ #define GRPC_TEST_CORE_END2END_TESTS_CANCEL_TEST_HELPERS_H typedef struct { + const char *name; grpc_call_error (*initiate_cancel)(grpc_call *call); grpc_status_code expect_status; const char *expect_details; @@ -45,7 +46,7 @@ static grpc_call_error wait_for_deadline(grpc_call *call) { } static const cancellation_mode cancellation_modes[] = { - {grpc_call_cancel, GRPC_STATUS_CANCELLED, ""}, - {wait_for_deadline, GRPC_STATUS_DEADLINE_EXCEEDED, "Deadline Exceeded"}, }; + {"cancel", grpc_call_cancel, GRPC_STATUS_CANCELLED, ""}, + {"deadline", wait_for_deadline, GRPC_STATUS_DEADLINE_EXCEEDED, "Deadline Exceeded"}, }; #endif /* GRPC_TEST_CORE_END2END_TESTS_CANCEL_TEST_HELPERS_H */ diff --git a/test/core/iomgr/tcp_server_posix_test.c b/test/core/iomgr/tcp_server_posix_test.c index 2689c3f38e5..6b80ee1ee8c 100644 --- a/test/core/iomgr/tcp_server_posix_test.c +++ b/test/core/iomgr/tcp_server_posix_test.c @@ -60,14 +60,14 @@ static void on_connect(void *arg, grpc_endpoint *tcp) { static void test_no_op(void) { grpc_tcp_server *s = grpc_tcp_server_create(); - grpc_tcp_server_destroy(s); + grpc_tcp_server_destroy(s, NULL, NULL); } static void test_no_op_with_start(void) { grpc_tcp_server *s = grpc_tcp_server_create(); LOG_TEST(); grpc_tcp_server_start(s, NULL, 0, on_connect, NULL); - grpc_tcp_server_destroy(s); + grpc_tcp_server_destroy(s, NULL, NULL); } static void test_no_op_with_port(void) { @@ -80,7 +80,7 @@ static void test_no_op_with_port(void) { GPR_ASSERT( grpc_tcp_server_add_port(s, (struct sockaddr *)&addr, sizeof(addr))); - grpc_tcp_server_destroy(s); + grpc_tcp_server_destroy(s, NULL, NULL); } static void test_no_op_with_port_and_start(void) { @@ -95,7 +95,7 @@ static void test_no_op_with_port_and_start(void) { grpc_tcp_server_start(s, NULL, 0, on_connect, NULL); - grpc_tcp_server_destroy(s); + grpc_tcp_server_destroy(s, NULL, NULL); } static void test_connect(int n) { @@ -144,7 +144,7 @@ static void test_connect(int n) { gpr_mu_unlock(&mu); - grpc_tcp_server_destroy(s); + grpc_tcp_server_destroy(s, NULL, NULL); } int main(int argc, char **argv) { From 6d97916b16f21c9b5a379ce5ddd35d30956f0304 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 7 Apr 2015 16:13:13 -0700 Subject: [PATCH 02/11] Update Windows build for interface changes --- src/core/iomgr/tcp_server_windows.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/core/iomgr/tcp_server_windows.c b/src/core/iomgr/tcp_server_windows.c index 0c3ab1dc911..896c9e5d08a 100644 --- a/src/core/iomgr/tcp_server_windows.c +++ b/src/core/iomgr/tcp_server_windows.c @@ -92,7 +92,9 @@ grpc_tcp_server *grpc_tcp_server_create(void) { return s; } -void grpc_tcp_server_destroy(grpc_tcp_server *s) { +void grpc_tcp_server_destroy(grpc_tcp_server *s, + void(*shutdown_done)(void *shutdown_done_arg), + void *shutdown_done_arg) { size_t i; gpr_mu_lock(&s->mu); /* shutdown all fd's */ @@ -112,6 +114,10 @@ void grpc_tcp_server_destroy(grpc_tcp_server *s) { } gpr_free(s->ports); gpr_free(s); + + if (shutdown_done) { + shutdown_done(shutdown_done_arg); + } } /* Prepare a recently-created socket for listening. */ From c02c1d883a08a2b6a646e7d736028c48baac3026 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Tue, 7 Apr 2015 16:21:55 -0700 Subject: [PATCH 03/11] clang-format --- src/core/iomgr/tcp_server.h | 6 ++-- src/core/iomgr/tcp_server_posix.c | 10 +++--- src/core/iomgr/tcp_server_windows.c | 34 +++++++++---------- src/core/security/server_secure_chttp2.c | 17 ++++++---- src/core/surface/completion_queue.c | 3 +- src/core/surface/completion_queue.h | 2 +- src/core/surface/server.c | 13 +++---- src/core/surface/server.h | 5 +-- src/core/surface/server_chttp2.c | 3 +- test/core/end2end/tests/cancel_after_invoke.c | 3 +- test/core/end2end/tests/cancel_test_helpers.h | 6 ++-- 11 files changed, 56 insertions(+), 46 deletions(-) diff --git a/src/core/iomgr/tcp_server.h b/src/core/iomgr/tcp_server.h index 1e58901a7a6..66bb3ef7018 100644 --- a/src/core/iomgr/tcp_server.h +++ b/src/core/iomgr/tcp_server.h @@ -71,8 +71,8 @@ int grpc_tcp_server_add_port(grpc_tcp_server *s, const void *addr, up when grpc_tcp_server_destroy is called. */ int grpc_tcp_server_get_fd(grpc_tcp_server *s, unsigned index); -void grpc_tcp_server_destroy(grpc_tcp_server *server, - void (*shutdown_done)(void *shutdown_done_arg), +void grpc_tcp_server_destroy(grpc_tcp_server *server, + void (*shutdown_done)(void *shutdown_done_arg), void *shutdown_done_arg); -#endif /* GRPC_INTERNAL_CORE_IOMGR_TCP_SERVER_H */ +#endif /* GRPC_INTERNAL_CORE_IOMGR_TCP_SERVER_H */ diff --git a/src/core/iomgr/tcp_server_posix.c b/src/core/iomgr/tcp_server_posix.c index 482166e2eb6..895f85fc682 100644 --- a/src/core/iomgr/tcp_server_posix.c +++ b/src/core/iomgr/tcp_server_posix.c @@ -154,13 +154,15 @@ static void destroyed_port(void *server, int success) { static void dont_care_about_shutdown_completion(void *ignored) {} -void grpc_tcp_server_destroy(grpc_tcp_server *s, - void (*shutdown_complete)(void *shutdown_complete_arg), - void *shutdown_complete_arg) { +void grpc_tcp_server_destroy( + grpc_tcp_server *s, void (*shutdown_complete)(void *shutdown_complete_arg), + void *shutdown_complete_arg) { size_t i; gpr_mu_lock(&s->mu); - s->shutdown_complete = shutdown_complete ? shutdown_complete : dont_care_about_shutdown_completion; + s->shutdown_complete = shutdown_complete + ? shutdown_complete + : dont_care_about_shutdown_completion; s->shutdown_complete_arg = shutdown_complete_arg; /* shutdown all fd's */ diff --git a/src/core/iomgr/tcp_server_windows.c b/src/core/iomgr/tcp_server_windows.c index 896c9e5d08a..a43d5670a41 100644 --- a/src/core/iomgr/tcp_server_windows.c +++ b/src/core/iomgr/tcp_server_windows.c @@ -93,8 +93,8 @@ grpc_tcp_server *grpc_tcp_server_create(void) { } void grpc_tcp_server_destroy(grpc_tcp_server *s, - void(*shutdown_done)(void *shutdown_done_arg), - void *shutdown_done_arg) { + void (*shutdown_done)(void *shutdown_done_arg), + void *shutdown_done_arg) { size_t i; gpr_mu_lock(&s->mu); /* shutdown all fd's */ @@ -116,13 +116,13 @@ void grpc_tcp_server_destroy(grpc_tcp_server *s, gpr_free(s); if (shutdown_done) { - shutdown_done(shutdown_done_arg); + shutdown_done(shutdown_done_arg); } } /* Prepare a recently-created socket for listening. */ -static int prepare_socket(SOCKET sock, - const struct sockaddr *addr, int addr_len) { +static int prepare_socket(SOCKET sock, const struct sockaddr *addr, + int addr_len) { struct sockaddr_storage sockname_temp; socklen_t sockname_len; @@ -153,15 +153,15 @@ static int prepare_socket(SOCKET sock, } sockname_len = sizeof(sockname_temp); - if (getsockname(sock, (struct sockaddr *) &sockname_temp, &sockname_len) - == SOCKET_ERROR) { + if (getsockname(sock, (struct sockaddr *)&sockname_temp, &sockname_len) == + SOCKET_ERROR) { char *utf8_message = gpr_format_message(WSAGetLastError()); gpr_log(GPR_ERROR, "getsockname: %s", utf8_message); gpr_free(utf8_message); goto error; } - return grpc_sockaddr_get_port((struct sockaddr *) &sockname_temp); + return grpc_sockaddr_get_port((struct sockaddr *)&sockname_temp); error: if (sock != INVALID_SOCKET) closesocket(sock); @@ -227,8 +227,7 @@ static void on_accept(void *arg, int success) { DWORD transfered_bytes = 0; DWORD flags; BOOL wsa_success = WSAGetOverlappedResult(sock, &info->overlapped, - &transfered_bytes, FALSE, - &flags); + &transfered_bytes, FALSE, &flags); if (!wsa_success) { char *utf8_message = gpr_format_message(WSAGetLastError()); gpr_log(GPR_ERROR, "on_accept error: %s", utf8_message); @@ -263,9 +262,9 @@ static int add_socket_to_server(grpc_tcp_server *s, SOCKET sock, if (sock == INVALID_SOCKET) return -1; - status = WSAIoctl(sock, SIO_GET_EXTENSION_FUNCTION_POINTER, - &guid, sizeof(guid), &AcceptEx, sizeof(AcceptEx), - &ioctl_num_bytes, NULL, NULL); + status = + WSAIoctl(sock, SIO_GET_EXTENSION_FUNCTION_POINTER, &guid, sizeof(guid), + &AcceptEx, sizeof(AcceptEx), &ioctl_num_bytes, NULL, NULL); if (status != 0) { char *utf8_message = gpr_format_message(WSAGetLastError()); @@ -313,9 +312,8 @@ int grpc_tcp_server_add_port(grpc_tcp_server *s, const void *addr, for (i = 0; i < s->nports; i++) { sockname_len = sizeof(sockname_temp); if (0 == getsockname(s->ports[i].socket->socket, - (struct sockaddr *) &sockname_temp, - &sockname_len)) { - port = grpc_sockaddr_get_port((struct sockaddr *) &sockname_temp); + (struct sockaddr *)&sockname_temp, &sockname_len)) { + port = grpc_sockaddr_get_port((struct sockaddr *)&sockname_temp); if (port > 0) { allocated_addr = malloc(addr_len); memcpy(allocated_addr, addr, addr_len); @@ -336,7 +334,7 @@ int grpc_tcp_server_add_port(grpc_tcp_server *s, const void *addr, if (grpc_sockaddr_is_wildcard(addr, &port)) { grpc_sockaddr_make_wildcard6(port, &wildcard); - addr = (struct sockaddr *) &wildcard; + addr = (struct sockaddr *)&wildcard; addr_len = sizeof(wildcard); } @@ -375,4 +373,4 @@ void grpc_tcp_server_start(grpc_tcp_server *s, grpc_pollset **pollset, gpr_mu_unlock(&s->mu); } -#endif /* GPR_WINSOCK_SOCKET */ +#endif /* GPR_WINSOCK_SOCKET */ diff --git a/src/core/security/server_secure_chttp2.c b/src/core/security/server_secure_chttp2.c index d5b3a82b878..081272724cf 100644 --- a/src/core/security/server_secure_chttp2.c +++ b/src/core/security/server_secure_chttp2.c @@ -85,10 +85,10 @@ static void on_secure_transport_setup_done(void *statep, if (status == GRPC_SECURITY_OK) { gpr_mu_lock(&state->mu); if (!state->is_shutdown) { - grpc_create_chttp2_transport( - setup_transport, state->server, - grpc_server_get_channel_args(state->server), - secure_endpoint, NULL, 0, grpc_mdctx_create(), 0); + grpc_create_chttp2_transport(setup_transport, state->server, + grpc_server_get_channel_args(state->server), + secure_endpoint, NULL, 0, + grpc_mdctx_create(), 0); } else { /* We need to consume this here, because the server may already have gone * away. */ @@ -104,7 +104,8 @@ static void on_secure_transport_setup_done(void *statep, static void on_accept(void *statep, grpc_endpoint *tcp) { grpc_server_secure_state *state = statep; state_ref(state); - grpc_setup_secure_transport(state->ctx, tcp, on_secure_transport_setup_done, state); + grpc_setup_secure_transport(state->ctx, tcp, on_secure_transport_setup_done, + state); } /* Server callback: start listening on our ports */ @@ -120,12 +121,14 @@ static void destroy(grpc_server *server, void *statep) { grpc_server_secure_state *state = statep; gpr_mu_lock(&state->mu); state->is_shutdown = 1; - grpc_tcp_server_destroy(state->tcp, grpc_server_listener_destroy_done, server); + grpc_tcp_server_destroy(state->tcp, grpc_server_listener_destroy_done, + server); gpr_mu_unlock(&state->mu); state_unref(state); } -int grpc_server_add_secure_http2_port(grpc_server *server, const char *addr, grpc_server_credentials *creds) { +int grpc_server_add_secure_http2_port(grpc_server *server, const char *addr, + grpc_server_credentials *creds) { grpc_resolved_addresses *resolved = NULL; grpc_tcp_server *tcp = NULL; grpc_server_secure_state *state = NULL; diff --git a/src/core/surface/completion_queue.c b/src/core/surface/completion_queue.c index b08bd93693e..24f4a05071a 100644 --- a/src/core/surface/completion_queue.c +++ b/src/core/surface/completion_queue.c @@ -436,6 +436,7 @@ grpc_pollset *grpc_cq_pollset(grpc_completion_queue *cc) { void grpc_cq_hack_spin_pollset(grpc_completion_queue *cc) { gpr_mu_lock(GRPC_POLLSET_MU(&cc->pollset)); grpc_pollset_kick(&cc->pollset); - grpc_pollset_work(&cc->pollset, gpr_time_add(gpr_now(), gpr_time_from_millis(100))); + grpc_pollset_work(&cc->pollset, + gpr_time_add(gpr_now(), gpr_time_from_millis(100))); gpr_mu_unlock(GRPC_POLLSET_MU(&cc->pollset)); } diff --git a/src/core/surface/completion_queue.h b/src/core/surface/completion_queue.h index 3e9e2d186ef..3a7cc99dda9 100644 --- a/src/core/surface/completion_queue.h +++ b/src/core/surface/completion_queue.h @@ -116,4 +116,4 @@ grpc_pollset *grpc_cq_pollset(grpc_completion_queue *cc); void grpc_cq_hack_spin_pollset(grpc_completion_queue *cc); -#endif /* GRPC_INTERNAL_CORE_SURFACE_COMPLETION_QUEUE_H */ +#endif /* GRPC_INTERNAL_CORE_SURFACE_COMPLETION_QUEUE_H */ diff --git a/src/core/surface/server.c b/src/core/surface/server.c index 2e013ea7421..17cba9a505b 100644 --- a/src/core/surface/server.c +++ b/src/core/surface/server.c @@ -592,9 +592,8 @@ static void destroy_channel_elem(grpc_channel_element *elem) { } static const grpc_channel_filter server_surface_filter = { - call_op, channel_op, sizeof(call_data), - init_call_elem, destroy_call_elem, sizeof(channel_data), - init_channel_elem, destroy_channel_elem, "server", + call_op, channel_op, sizeof(call_data), init_call_elem, destroy_call_elem, + sizeof(channel_data), init_channel_elem, destroy_channel_elem, "server", }; static void addcq(grpc_server *server, grpc_completion_queue *cq) { @@ -737,7 +736,8 @@ grpc_transport_setup_result grpc_server_setup_transport( channel = grpc_channel_create_from_filters(filters, num_filters, s->channel_args, mdctx, 0); chand = (channel_data *)grpc_channel_stack_element( - grpc_channel_get_channel_stack(channel), 0)->channel_data; + grpc_channel_get_channel_stack(channel), 0) + ->channel_data; chand->server = s; server_ref(s); chand->channel = channel; @@ -758,7 +758,7 @@ grpc_transport_setup_result grpc_server_setup_transport( method = grpc_mdstr_from_string(mdctx, rm->method); hash = GRPC_MDSTR_KV_HASH(host ? host->hash : 0, method->hash); for (probes = 0; chand->registered_methods[(hash + probes) % slots] - .server_registered_method != NULL; + .server_registered_method != NULL; probes++) ; if (probes > max_probes) max_probes = probes; @@ -927,7 +927,8 @@ void grpc_server_destroy(grpc_server *server) { gpr_mu_lock(&server->mu); } - gpr_cv_wait(&server->cv, &server->mu, gpr_time_add(gpr_now(), gpr_time_from_millis(100))); + gpr_cv_wait(&server->cv, &server->mu, + gpr_time_add(gpr_now(), gpr_time_from_millis(100))); } while (server->listeners) { diff --git a/src/core/surface/server.h b/src/core/surface/server.h index 548a16c6c96..2cfa38fa436 100644 --- a/src/core/surface/server.h +++ b/src/core/surface/server.h @@ -48,7 +48,8 @@ grpc_server *grpc_server_create_from_filters(grpc_completion_queue *cq, and when it shuts down, it will call destroy */ void grpc_server_add_listener(grpc_server *server, void *listener, void (*start)(grpc_server *server, void *arg, - grpc_pollset **pollsets, size_t npollsets), + grpc_pollset **pollsets, + size_t npollsets), void (*destroy)(grpc_server *server, void *arg)); void grpc_server_listener_destroy_done(void *server); @@ -62,4 +63,4 @@ grpc_transport_setup_result grpc_server_setup_transport( const grpc_channel_args *grpc_server_get_channel_args(grpc_server *server); -#endif /* GRPC_INTERNAL_CORE_SURFACE_SERVER_H */ +#endif /* GRPC_INTERNAL_CORE_SURFACE_SERVER_H */ diff --git a/src/core/surface/server_chttp2.c b/src/core/surface/server_chttp2.c index 9a23125752a..f3b9219f8b0 100644 --- a/src/core/surface/server_chttp2.c +++ b/src/core/surface/server_chttp2.c @@ -66,7 +66,8 @@ static void new_transport(void *server, grpc_endpoint *tcp) { } /* Server callback: start listening on our ports */ -static void start(grpc_server *server, void *tcpp, grpc_pollset **pollsets, size_t pollset_count) { +static void start(grpc_server *server, void *tcpp, grpc_pollset **pollsets, + size_t pollset_count) { grpc_tcp_server *tcp = tcpp; grpc_tcp_server_start(tcp, pollsets, pollset_count, new_transport, server); } diff --git a/test/core/end2end/tests/cancel_after_invoke.c b/test/core/end2end/tests/cancel_after_invoke.c index 326321f4e21..592dfd415f9 100644 --- a/test/core/end2end/tests/cancel_after_invoke.c +++ b/test/core/end2end/tests/cancel_after_invoke.c @@ -110,7 +110,8 @@ static void test_cancel_after_invoke(grpc_end2end_test_config config, grpc_op ops[6]; grpc_op *op; grpc_call *c; - grpc_end2end_test_fixture f = begin_test(config, __FUNCTION__, mode, NULL, NULL); + grpc_end2end_test_fixture f = + begin_test(config, __FUNCTION__, mode, NULL, NULL); gpr_timespec deadline = five_seconds_time(); cq_verifier *v_client = cq_verifier_create(f.client_cq); grpc_metadata_array initial_metadata_recv; diff --git a/test/core/end2end/tests/cancel_test_helpers.h b/test/core/end2end/tests/cancel_test_helpers.h index 3d92b64ae49..0d680fcfe11 100644 --- a/test/core/end2end/tests/cancel_test_helpers.h +++ b/test/core/end2end/tests/cancel_test_helpers.h @@ -47,6 +47,8 @@ static grpc_call_error wait_for_deadline(grpc_call *call) { static const cancellation_mode cancellation_modes[] = { {"cancel", grpc_call_cancel, GRPC_STATUS_CANCELLED, ""}, - {"deadline", wait_for_deadline, GRPC_STATUS_DEADLINE_EXCEEDED, "Deadline Exceeded"}, }; + {"deadline", wait_for_deadline, GRPC_STATUS_DEADLINE_EXCEEDED, + "Deadline Exceeded"}, +}; -#endif /* GRPC_TEST_CORE_END2END_TESTS_CANCEL_TEST_HELPERS_H */ +#endif /* GRPC_TEST_CORE_END2END_TESTS_CANCEL_TEST_HELPERS_H */ From 59838499889025fab037db4fcd0201f4bc45702d Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 8 Apr 2015 15:47:14 -0700 Subject: [PATCH 04/11] Added script to generate test .php files from .proto files --- src/php/bin/generate_proto_php.sh | 43 +++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100755 src/php/bin/generate_proto_php.sh diff --git a/src/php/bin/generate_proto_php.sh b/src/php/bin/generate_proto_php.sh new file mode 100755 index 00000000000..3cc07aba349 --- /dev/null +++ b/src/php/bin/generate_proto_php.sh @@ -0,0 +1,43 @@ +#!/bin/sh +# 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. + + +set +e +cd $(dirname $0) + +cd ../tests/generated_code + +protoc-gen-php -i . -o . ./math.proto + +cd - + +cd ../tests/interop + +protoc-gen-php -i . -o . ./test.proto From 35d06e99fb310c81b1fdd79a8ffda2a5bb3fe02f Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 8 Apr 2015 15:49:56 -0700 Subject: [PATCH 05/11] Simplified script --- src/php/bin/generate_proto_php.sh | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/php/bin/generate_proto_php.sh b/src/php/bin/generate_proto_php.sh index 3cc07aba349..16f93747ab8 100755 --- a/src/php/bin/generate_proto_php.sh +++ b/src/php/bin/generate_proto_php.sh @@ -32,12 +32,9 @@ set +e cd $(dirname $0) -cd ../tests/generated_code +gen_code='../tests/generated_code' +interop='../tests/interop' -protoc-gen-php -i . -o . ./math.proto +protoc-gen-php -i $gen_code -o $gen_code $gen_code/math.proto -cd - - -cd ../tests/interop - -protoc-gen-php -i . -o . ./test.proto +protoc-gen-php -i $interop -o $interop $interop/test.proto From aa11066573722b704c718f418514735f1c86edb6 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Wed, 8 Apr 2015 16:53:47 -0700 Subject: [PATCH 06/11] Fixed memory leaks in PHP extension code --- src/php/ext/grpc/call.c | 11 +++++++---- src/php/ext/grpc/channel.c | 1 + 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/php/ext/grpc/call.c b/src/php/ext/grpc/call.c index 6bc65b53679..b1525e9246a 100644 --- a/src/php/ext/grpc/call.c +++ b/src/php/ext/grpc/call.c @@ -443,8 +443,9 @@ PHP_METHOD(Call, startBatch) { add_property_bool(result, "send_status", true); break; case GRPC_OP_RECV_INITIAL_METADATA: - add_property_zval(result, "metadata", - grpc_parse_metadata_array(&recv_metadata)); + array = grpc_parse_metadata_array(&recv_metadata); + add_property_zval(result, "metadata", array); + Z_DELREF_P(array); break; case GRPC_OP_RECV_MESSAGE: byte_buffer_to_string(message, &message_str, &message_len); @@ -458,11 +459,13 @@ PHP_METHOD(Call, startBatch) { case GRPC_OP_RECV_STATUS_ON_CLIENT: MAKE_STD_ZVAL(recv_status); object_init(recv_status); - add_property_zval(recv_status, "metadata", - grpc_parse_metadata_array(&recv_trailing_metadata)); + array = grpc_parse_metadata_array(&recv_trailing_metadata); + add_property_zval(recv_status, "metadata", array); + Z_DELREF_P(array); add_property_long(recv_status, "code", status); add_property_string(recv_status, "details", status_details, true); add_property_zval(result, "status", recv_status); + Z_DELREF_P(recv_status); break; case GRPC_OP_RECV_CLOSE_ON_SERVER: add_property_bool(result, "cancelled", cancelled); diff --git a/src/php/ext/grpc/channel.c b/src/php/ext/grpc/channel.c index 51a3eae0f4e..b8262db162b 100644 --- a/src/php/ext/grpc/channel.c +++ b/src/php/ext/grpc/channel.c @@ -62,6 +62,7 @@ void free_wrapped_grpc_channel(void *object TSRMLS_DC) { if (channel->wrapped != NULL) { grpc_channel_destroy(channel->wrapped); } + efree(channel->target); efree(channel); } From c76b5652d47fe65f825cd3bce31a13916dad1503 Mon Sep 17 00:00:00 2001 From: Tim Emiola Date: Wed, 8 Apr 2015 17:19:34 -0700 Subject: [PATCH 07/11] Use the release tag specified by the version --- tools/distpackages/build_deb_packages.sh | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/distpackages/build_deb_packages.sh b/tools/distpackages/build_deb_packages.sh index 366a5184c8b..0beb41ed0a5 100755 --- a/tools/distpackages/build_deb_packages.sh +++ b/tools/distpackages/build_deb_packages.sh @@ -45,7 +45,13 @@ if [ -f /version.txt ]; then pkg_version=$(cat /version.txt) fi version="${pkg_version}.0" -echo "Target release => $pkg_version" +release_tag="release-${pkg_version//./_}" +echo "Target release => $pkg_version, will checkout tag $release_tag" + +# Switch grpc_root to the release tag +pushd $grpc_root +git checkout $release_tag || { echo "bad release tag ${release_tag}"; exit 1; } +popd if [ -f /.dockerinit ]; then # We're in Docker where uname -p returns "unknown". From e6b00382235a5d071598792f7046537290ef9ac6 Mon Sep 17 00:00:00 2001 From: Masood Malekghassemi Date: Thu, 9 Apr 2015 12:25:39 -0700 Subject: [PATCH 08/11] Add OAuth interop tests to Python Also adds the commands to the grpc_docker shell library script to support running in docker. --- .../interop/interop/_interop_test_case.py | 10 ++-- src/python/interop/interop/client.py | 24 ++++++-- src/python/interop/interop/methods.py | 60 +++++++++++++++---- src/python/interop/setup.py | 2 +- tools/gce_setup/grpc_docker.sh | 29 +++++++++ 5 files changed, 103 insertions(+), 22 deletions(-) diff --git a/src/python/interop/interop/_interop_test_case.py b/src/python/interop/interop/_interop_test_case.py index fec8f1915d5..cd6a574e90d 100644 --- a/src/python/interop/interop/_interop_test_case.py +++ b/src/python/interop/interop/_interop_test_case.py @@ -40,16 +40,16 @@ class InteropTestCase(object): """ def testEmptyUnary(self): - methods.TestCase.EMPTY_UNARY.test_interoperability(self.stub) + methods.TestCase.EMPTY_UNARY.test_interoperability(self.stub, None) def testLargeUnary(self): - methods.TestCase.LARGE_UNARY.test_interoperability(self.stub) + methods.TestCase.LARGE_UNARY.test_interoperability(self.stub, None) def testServerStreaming(self): - methods.TestCase.SERVER_STREAMING.test_interoperability(self.stub) + methods.TestCase.SERVER_STREAMING.test_interoperability(self.stub, None) def testClientStreaming(self): - methods.TestCase.CLIENT_STREAMING.test_interoperability(self.stub) + methods.TestCase.CLIENT_STREAMING.test_interoperability(self.stub, None) def testPingPong(self): - methods.TestCase.PING_PONG.test_interoperability(self.stub) + methods.TestCase.PING_PONG.test_interoperability(self.stub, None) diff --git a/src/python/interop/interop/client.py b/src/python/interop/interop/client.py index 85a0dcd9983..bae5e174604 100644 --- a/src/python/interop/interop/client.py +++ b/src/python/interop/interop/client.py @@ -30,6 +30,7 @@ """The Python implementation of the GRPC interoperability test client.""" import argparse +from oauth2client import client as oauth2client_client from grpc.early_adopter import implementations @@ -43,9 +44,6 @@ def _args(): parser = argparse.ArgumentParser() parser.add_argument( '--server_host', help='the host to which to connect', type=str) - parser.add_argument( - '--server_host_override', - help='the server host to which to claim to connect', type=str) parser.add_argument( '--server_port', help='the port to which to connect', type=int) parser.add_argument( @@ -56,10 +54,25 @@ def _args(): parser.add_argument( '--use_test_ca', help='replace platform root CAs with ca.pem', action='store_true') + parser.add_argument( + '--server_host_override', + help='the server host to which to claim to connect', type=str) + parser.add_argument('--oauth_scope', help='scope for OAuth tokens', type=str) + parser.add_argument( + '--default_service_account', + help='email address of the default service account', type=str) return parser.parse_args() +def _oauth_access_token(args): + credentials = client.GoogleCredentials.get_application_default() + scoped_credentials = credentials.create_scoped([args.oauth_scope]) + return scoped_credentials.get_access_token().access_token def _stub(args): + if args.oauth_scope: + metadata_transformer = lambda x: [('Authorization', 'Bearer %s' % _oauth_access_token(args))] + else: + metadata_transformer = lambda x: [] if args.use_tls: if args.use_test_ca: root_certificates = resources.test_root_certificates() @@ -68,7 +81,8 @@ def _stub(args): stub = implementations.stub( methods.SERVICE_NAME, methods.CLIENT_METHODS, args.server_host, - args.server_port, secure=True, root_certificates=root_certificates, + args.server_port, metadata_transformer=metadata_transformer, + secure=True, root_certificates=root_certificates, server_host_override=args.server_host_override) else: stub = implementations.stub( @@ -89,7 +103,7 @@ def _test_interoperability(): args = _args() stub = _stub(args) test_case = _test_case_from_arg(args.test_case) - test_case.test_interoperability(stub) + test_case.test_interoperability(stub, args) if __name__ == '__main__': diff --git a/src/python/interop/interop/methods.py b/src/python/interop/interop/methods.py index 79550a37893..c69771dff1e 100644 --- a/src/python/interop/interop/methods.py +++ b/src/python/interop/interop/methods.py @@ -30,8 +30,12 @@ """Implementations of interoperability test methods.""" import enum +import json +import os import threading +from oauth2client import client as oauth2client_client + from grpc.framework.alpha import utilities from interop import empty_pb2 @@ -150,19 +154,12 @@ SERVER_METHODS = { } -def _empty_unary(stub): - with stub: - response = stub.EmptyCall(empty_pb2.Empty(), _TIMEOUT) - if not isinstance(response, empty_pb2.Empty): - raise TypeError( - 'response is of type "%s", not empty_pb2.Empty!', type(response)) - - -def _large_unary(stub): +def _large_unary_common_behavior(stub, fill_username, fill_oauth_scope): with stub: request = messages_pb2.SimpleRequest( response_type=messages_pb2.COMPRESSABLE, response_size=314159, - payload=messages_pb2.Payload(body=b'\x00' * 271828)) + payload=messages_pb2.Payload(body=b'\x00' * 271828), + fill_username=fill_username, fill_oauth_scope=fill_oauth_scope) response_future = stub.UnaryCall.async(request, _TIMEOUT) response = response_future.result() if response.payload.type is not messages_pb2.COMPRESSABLE: @@ -171,6 +168,19 @@ def _large_unary(stub): if len(response.payload.body) != 314159: raise ValueError( 'response body of incorrect size %d!' % len(response.payload.body)) + return response + + +def _empty_unary(stub): + with stub: + response = stub.EmptyCall(empty_pb2.Empty(), _TIMEOUT) + if not isinstance(response, empty_pb2.Empty): + raise TypeError( + 'response is of type "%s", not empty_pb2.Empty!', type(response)) + + +def _large_unary(stub): + _large_unary_common_behavior(stub, False, False) def _client_streaming(stub): @@ -266,6 +276,28 @@ def _ping_pong(stub): pipe.close() +def _compute_engine_creds(stub, args): + response = _large_unary_common_behavior(stub, True, True) + if args.default_service_account != response.username: + raise ValueError( + 'expected username %s, got %s' % (args.default_service_account, + response.username)) + + +def _service_account_creds(stub, args): + json_key_filename = os.environ[ + oauth2client_client.GOOGLE_APPLICATION_CREDENTIALS] + wanted_email = json.load(open(json_key_filename, 'rb'))['client_email'] + response = _large_unary_common_behavior(stub, True, True) + if wanted_email != response.username: + raise ValueError( + 'expected username %s, got %s' % (wanted_email, response.username)) + if response.oauth_scope in args.oauth_scope: + raise ValueError( + 'expected to find oauth scope "%s" in received "%s"' % + (response.oauth_scope, args.oauth_scope)) + + @enum.unique class TestCase(enum.Enum): EMPTY_UNARY = 'empty_unary' @@ -273,8 +305,10 @@ class TestCase(enum.Enum): SERVER_STREAMING = 'server_streaming' CLIENT_STREAMING = 'client_streaming' PING_PONG = 'ping_pong' + COMPUTE_ENGINE_CREDS = 'compute_engine_creds' + SERVICE_ACCOUNT_CREDS = 'service_account_creds' - def test_interoperability(self, stub): + def test_interoperability(self, stub, args): if self is TestCase.EMPTY_UNARY: _empty_unary(stub) elif self is TestCase.LARGE_UNARY: @@ -285,5 +319,9 @@ class TestCase(enum.Enum): _client_streaming(stub) elif self is TestCase.PING_PONG: _ping_pong(stub) + elif self is TestCase.COMPUTE_ENGINE_CREDS: + _compute_engine_creds(stub, args) + elif self is TestCase.SERVICE_ACCOUNT_CREDS: + _service_account_creds(stub, args) else: raise NotImplementedError('Test case "%s" not implemented!' % self.name) diff --git a/src/python/interop/setup.py b/src/python/interop/setup.py index 7a329924a55..502fcbedd84 100644 --- a/src/python/interop/setup.py +++ b/src/python/interop/setup.py @@ -45,7 +45,7 @@ _PACKAGE_DATA = { 'credentials/server1.pem',] } -_INSTALL_REQUIRES = ['grpcio>=0.4.0a4'] +_INSTALL_REQUIRES = ['oauth2client>=1.4.7', 'grpcio>=0.4.0a4'] setuptools.setup( name='interop', diff --git a/tools/gce_setup/grpc_docker.sh b/tools/gce_setup/grpc_docker.sh index dc480983ff4..40634291cf9 100755 --- a/tools/gce_setup/grpc_docker.sh +++ b/tools/gce_setup/grpc_docker.sh @@ -1041,6 +1041,35 @@ grpc_interop_gen_python_cmd() { echo $the_cmd } +# constructs the full dockerized python service_account auth interop test cmd. +# +# call-seq: +# flags= .... # generic flags to include the command +# cmd=$($grpc_gen_test_cmd $flags) +grpc_cloud_prod_auth_service_account_creds_gen_python_cmd() { + local cmd_prefix="sudo docker run grpc/ruby bin/bash -l -c"; + local gfe_flags=$(_grpc_prod_gfe_flags) + local added_gfe_flags=$(_grpc_default_creds_test_flags) + local env_prefix="SSL_CERT_FILE=/cacerts/roots.pem" + env_prefix+=" GOOGLE_APPLICATION_CREDENTIALS=/service_account/stubbyCloudTestingTest-7dd63462c60c.json" + local the_cmd="$cmd_prefix '$env_prefix python -B -m interop.client --use_tls $gfe_flags $added_gfe_flags $@'" + echo $the_cmd +} + +# constructs the full dockerized python gce auth interop test cmd. +# +# call-seq: +# flags= .... # generic flags to include the command +# cmd=$($grpc_gen_test_cmd $flags) +grpc_cloud_prod_auth_compute_engine_creds_gen_python_cmd() { + local cmd_prefix="sudo docker run grpc/ruby bin/bash -l -c"; + local gfe_flags=$(_grpc_prod_gfe_flags) + local added_gfe_flags=$(_grpc_gce_test_flags) + local env_prefix="SSL_CERT_FILE=/cacerts/roots.pem" + local the_cmd="$cmd_prefix '$env_prefix python -B -m interop.client --use_tls $gfe_flags $added_gfe_flags $@'" + echo $the_cmd +} + # constructs the full dockerized java interop test cmd. # # call-seq: From 0caebbfcfe45a6469b7ff6744fffe2e0a4871485 Mon Sep 17 00:00:00 2001 From: "Nicolas \"Pixel\" Noble" Date: Thu, 9 Apr 2015 23:08:51 +0200 Subject: [PATCH 09/11] Splitting gRPC service class codegen into its own set of files. --- Makefile | 118 ++++++++++++----- examples/pubsub/publisher.h | 2 +- examples/pubsub/subscriber.h | 2 +- src/compiler/cpp_generator.cc | 153 +++++++++++++++++++++-- src/compiler/cpp_generator.h | 19 ++- src/compiler/cpp_plugin.cc | 36 +++--- templates/Makefile.template | 16 +-- test/cpp/end2end/async_end2end_test.cc | 4 +- test/cpp/end2end/end2end_test.cc | 4 +- test/cpp/end2end/generic_end2end_test.cc | 2 +- test/cpp/interop/client.cc | 6 +- test/cpp/interop/server.cc | 6 +- test/cpp/qps/client.h | 2 +- test/cpp/qps/client_async.cc | 2 +- test/cpp/qps/client_sync.cc | 2 +- test/cpp/qps/driver.h | 2 +- test/cpp/qps/histogram.h | 2 +- test/cpp/qps/server.h | 2 +- test/cpp/qps/server_async.cc | 2 +- test/cpp/qps/server_sync.cc | 2 +- test/cpp/qps/worker.cc | 2 +- test/cpp/util/cli_call_test.cc | 2 +- 22 files changed, 302 insertions(+), 86 deletions(-) diff --git a/Makefile b/Makefile index 8e01bb88dc6..c50eb8c0cb8 100644 --- a/Makefile +++ b/Makefile @@ -2001,92 +2001,152 @@ endif ifeq ($(NO_PROTOC),true) $(GENDIR)/examples/pubsub/empty.pb.cc: protoc_dep_error +$(GENDIR)/examples/pubsub/empty.grpc.pb.cc: protoc_dep_error else $(GENDIR)/examples/pubsub/empty.pb.cc: examples/pubsub/empty.proto $(PROTOBUF_DEP) $(PROTOC_PLUGINS) $(E) "[PROTOC] Generating protobuf CC file from $<" $(Q) mkdir -p `dirname $@` - $(Q) $(PROTOC) --cpp_out=$(GENDIR) --grpc_out=$(GENDIR) --plugin=protoc-gen-grpc=$(BINDIR)/$(CONFIG)/grpc_cpp_plugin $< + $(Q) $(PROTOC) --cpp_out=$(GENDIR) $< + +$(GENDIR)/examples/pubsub/empty.grpc.pb.cc: examples/pubsub/empty.proto $(PROTOBUF_DEP) $(PROTOC_PLUGINS) + $(E) "[GRPC] Generating gRPC's protobuf service CC file from $<" + $(Q) mkdir -p `dirname $@` + $(Q) $(PROTOC) --grpc_out=$(GENDIR) --plugin=protoc-gen-grpc=$(BINDIR)/$(CONFIG)/grpc_cpp_plugin $< endif ifeq ($(NO_PROTOC),true) $(GENDIR)/examples/pubsub/label.pb.cc: protoc_dep_error +$(GENDIR)/examples/pubsub/label.grpc.pb.cc: protoc_dep_error else $(GENDIR)/examples/pubsub/label.pb.cc: examples/pubsub/label.proto $(PROTOBUF_DEP) $(PROTOC_PLUGINS) $(E) "[PROTOC] Generating protobuf CC file from $<" $(Q) mkdir -p `dirname $@` - $(Q) $(PROTOC) --cpp_out=$(GENDIR) --grpc_out=$(GENDIR) --plugin=protoc-gen-grpc=$(BINDIR)/$(CONFIG)/grpc_cpp_plugin $< + $(Q) $(PROTOC) --cpp_out=$(GENDIR) $< + +$(GENDIR)/examples/pubsub/label.grpc.pb.cc: examples/pubsub/label.proto $(PROTOBUF_DEP) $(PROTOC_PLUGINS) + $(E) "[GRPC] Generating gRPC's protobuf service CC file from $<" + $(Q) mkdir -p `dirname $@` + $(Q) $(PROTOC) --grpc_out=$(GENDIR) --plugin=protoc-gen-grpc=$(BINDIR)/$(CONFIG)/grpc_cpp_plugin $< endif ifeq ($(NO_PROTOC),true) $(GENDIR)/examples/pubsub/pubsub.pb.cc: protoc_dep_error +$(GENDIR)/examples/pubsub/pubsub.grpc.pb.cc: protoc_dep_error else $(GENDIR)/examples/pubsub/pubsub.pb.cc: examples/pubsub/pubsub.proto $(PROTOBUF_DEP) $(PROTOC_PLUGINS) $(E) "[PROTOC] Generating protobuf CC file from $<" $(Q) mkdir -p `dirname $@` - $(Q) $(PROTOC) --cpp_out=$(GENDIR) --grpc_out=$(GENDIR) --plugin=protoc-gen-grpc=$(BINDIR)/$(CONFIG)/grpc_cpp_plugin $< + $(Q) $(PROTOC) --cpp_out=$(GENDIR) $< + +$(GENDIR)/examples/pubsub/pubsub.grpc.pb.cc: examples/pubsub/pubsub.proto $(PROTOBUF_DEP) $(PROTOC_PLUGINS) + $(E) "[GRPC] Generating gRPC's protobuf service CC file from $<" + $(Q) mkdir -p `dirname $@` + $(Q) $(PROTOC) --grpc_out=$(GENDIR) --plugin=protoc-gen-grpc=$(BINDIR)/$(CONFIG)/grpc_cpp_plugin $< endif ifeq ($(NO_PROTOC),true) $(GENDIR)/test/cpp/interop/empty.pb.cc: protoc_dep_error +$(GENDIR)/test/cpp/interop/empty.grpc.pb.cc: protoc_dep_error else $(GENDIR)/test/cpp/interop/empty.pb.cc: test/cpp/interop/empty.proto $(PROTOBUF_DEP) $(PROTOC_PLUGINS) $(E) "[PROTOC] Generating protobuf CC file from $<" $(Q) mkdir -p `dirname $@` - $(Q) $(PROTOC) --cpp_out=$(GENDIR) --grpc_out=$(GENDIR) --plugin=protoc-gen-grpc=$(BINDIR)/$(CONFIG)/grpc_cpp_plugin $< + $(Q) $(PROTOC) --cpp_out=$(GENDIR) $< + +$(GENDIR)/test/cpp/interop/empty.grpc.pb.cc: test/cpp/interop/empty.proto $(PROTOBUF_DEP) $(PROTOC_PLUGINS) + $(E) "[GRPC] Generating gRPC's protobuf service CC file from $<" + $(Q) mkdir -p `dirname $@` + $(Q) $(PROTOC) --grpc_out=$(GENDIR) --plugin=protoc-gen-grpc=$(BINDIR)/$(CONFIG)/grpc_cpp_plugin $< endif ifeq ($(NO_PROTOC),true) $(GENDIR)/test/cpp/interop/messages.pb.cc: protoc_dep_error +$(GENDIR)/test/cpp/interop/messages.grpc.pb.cc: protoc_dep_error else $(GENDIR)/test/cpp/interop/messages.pb.cc: test/cpp/interop/messages.proto $(PROTOBUF_DEP) $(PROTOC_PLUGINS) $(E) "[PROTOC] Generating protobuf CC file from $<" $(Q) mkdir -p `dirname $@` - $(Q) $(PROTOC) --cpp_out=$(GENDIR) --grpc_out=$(GENDIR) --plugin=protoc-gen-grpc=$(BINDIR)/$(CONFIG)/grpc_cpp_plugin $< + $(Q) $(PROTOC) --cpp_out=$(GENDIR) $< + +$(GENDIR)/test/cpp/interop/messages.grpc.pb.cc: test/cpp/interop/messages.proto $(PROTOBUF_DEP) $(PROTOC_PLUGINS) + $(E) "[GRPC] Generating gRPC's protobuf service CC file from $<" + $(Q) mkdir -p `dirname $@` + $(Q) $(PROTOC) --grpc_out=$(GENDIR) --plugin=protoc-gen-grpc=$(BINDIR)/$(CONFIG)/grpc_cpp_plugin $< endif ifeq ($(NO_PROTOC),true) $(GENDIR)/test/cpp/interop/test.pb.cc: protoc_dep_error +$(GENDIR)/test/cpp/interop/test.grpc.pb.cc: protoc_dep_error else $(GENDIR)/test/cpp/interop/test.pb.cc: test/cpp/interop/test.proto $(PROTOBUF_DEP) $(PROTOC_PLUGINS) $(E) "[PROTOC] Generating protobuf CC file from $<" $(Q) mkdir -p `dirname $@` - $(Q) $(PROTOC) --cpp_out=$(GENDIR) --grpc_out=$(GENDIR) --plugin=protoc-gen-grpc=$(BINDIR)/$(CONFIG)/grpc_cpp_plugin $< + $(Q) $(PROTOC) --cpp_out=$(GENDIR) $< + +$(GENDIR)/test/cpp/interop/test.grpc.pb.cc: test/cpp/interop/test.proto $(PROTOBUF_DEP) $(PROTOC_PLUGINS) + $(E) "[GRPC] Generating gRPC's protobuf service CC file from $<" + $(Q) mkdir -p `dirname $@` + $(Q) $(PROTOC) --grpc_out=$(GENDIR) --plugin=protoc-gen-grpc=$(BINDIR)/$(CONFIG)/grpc_cpp_plugin $< endif ifeq ($(NO_PROTOC),true) $(GENDIR)/test/cpp/qps/qpstest.pb.cc: protoc_dep_error +$(GENDIR)/test/cpp/qps/qpstest.grpc.pb.cc: protoc_dep_error else $(GENDIR)/test/cpp/qps/qpstest.pb.cc: test/cpp/qps/qpstest.proto $(PROTOBUF_DEP) $(PROTOC_PLUGINS) $(E) "[PROTOC] Generating protobuf CC file from $<" $(Q) mkdir -p `dirname $@` - $(Q) $(PROTOC) --cpp_out=$(GENDIR) --grpc_out=$(GENDIR) --plugin=protoc-gen-grpc=$(BINDIR)/$(CONFIG)/grpc_cpp_plugin $< + $(Q) $(PROTOC) --cpp_out=$(GENDIR) $< + +$(GENDIR)/test/cpp/qps/qpstest.grpc.pb.cc: test/cpp/qps/qpstest.proto $(PROTOBUF_DEP) $(PROTOC_PLUGINS) + $(E) "[GRPC] Generating gRPC's protobuf service CC file from $<" + $(Q) mkdir -p `dirname $@` + $(Q) $(PROTOC) --grpc_out=$(GENDIR) --plugin=protoc-gen-grpc=$(BINDIR)/$(CONFIG)/grpc_cpp_plugin $< endif ifeq ($(NO_PROTOC),true) $(GENDIR)/test/cpp/util/echo.pb.cc: protoc_dep_error +$(GENDIR)/test/cpp/util/echo.grpc.pb.cc: protoc_dep_error else $(GENDIR)/test/cpp/util/echo.pb.cc: test/cpp/util/echo.proto $(PROTOBUF_DEP) $(PROTOC_PLUGINS) $(E) "[PROTOC] Generating protobuf CC file from $<" $(Q) mkdir -p `dirname $@` - $(Q) $(PROTOC) --cpp_out=$(GENDIR) --grpc_out=$(GENDIR) --plugin=protoc-gen-grpc=$(BINDIR)/$(CONFIG)/grpc_cpp_plugin $< + $(Q) $(PROTOC) --cpp_out=$(GENDIR) $< + +$(GENDIR)/test/cpp/util/echo.grpc.pb.cc: test/cpp/util/echo.proto $(PROTOBUF_DEP) $(PROTOC_PLUGINS) + $(E) "[GRPC] Generating gRPC's protobuf service CC file from $<" + $(Q) mkdir -p `dirname $@` + $(Q) $(PROTOC) --grpc_out=$(GENDIR) --plugin=protoc-gen-grpc=$(BINDIR)/$(CONFIG)/grpc_cpp_plugin $< endif ifeq ($(NO_PROTOC),true) $(GENDIR)/test/cpp/util/echo_duplicate.pb.cc: protoc_dep_error +$(GENDIR)/test/cpp/util/echo_duplicate.grpc.pb.cc: protoc_dep_error else $(GENDIR)/test/cpp/util/echo_duplicate.pb.cc: test/cpp/util/echo_duplicate.proto $(PROTOBUF_DEP) $(PROTOC_PLUGINS) $(E) "[PROTOC] Generating protobuf CC file from $<" $(Q) mkdir -p `dirname $@` - $(Q) $(PROTOC) --cpp_out=$(GENDIR) --grpc_out=$(GENDIR) --plugin=protoc-gen-grpc=$(BINDIR)/$(CONFIG)/grpc_cpp_plugin $< + $(Q) $(PROTOC) --cpp_out=$(GENDIR) $< + +$(GENDIR)/test/cpp/util/echo_duplicate.grpc.pb.cc: test/cpp/util/echo_duplicate.proto $(PROTOBUF_DEP) $(PROTOC_PLUGINS) + $(E) "[GRPC] Generating gRPC's protobuf service CC file from $<" + $(Q) mkdir -p `dirname $@` + $(Q) $(PROTOC) --grpc_out=$(GENDIR) --plugin=protoc-gen-grpc=$(BINDIR)/$(CONFIG)/grpc_cpp_plugin $< endif ifeq ($(NO_PROTOC),true) $(GENDIR)/test/cpp/util/messages.pb.cc: protoc_dep_error +$(GENDIR)/test/cpp/util/messages.grpc.pb.cc: protoc_dep_error else $(GENDIR)/test/cpp/util/messages.pb.cc: test/cpp/util/messages.proto $(PROTOBUF_DEP) $(PROTOC_PLUGINS) $(E) "[PROTOC] Generating protobuf CC file from $<" $(Q) mkdir -p `dirname $@` - $(Q) $(PROTOC) --cpp_out=$(GENDIR) --grpc_out=$(GENDIR) --plugin=protoc-gen-grpc=$(BINDIR)/$(CONFIG)/grpc_cpp_plugin $< + $(Q) $(PROTOC) --cpp_out=$(GENDIR) $< + +$(GENDIR)/test/cpp/util/messages.grpc.pb.cc: test/cpp/util/messages.proto $(PROTOBUF_DEP) $(PROTOC_PLUGINS) + $(E) "[GRPC] Generating gRPC's protobuf service CC file from $<" + $(Q) mkdir -p `dirname $@` + $(Q) $(PROTOC) --grpc_out=$(GENDIR) --plugin=protoc-gen-grpc=$(BINDIR)/$(CONFIG)/grpc_cpp_plugin $< endif @@ -3417,9 +3477,9 @@ $(OBJDIR)/$(CONFIG)/src/cpp/util/time.o: LIBGRPC++_TEST_UTIL_SRC = \ - $(GENDIR)/test/cpp/util/messages.pb.cc \ - $(GENDIR)/test/cpp/util/echo.pb.cc \ - $(GENDIR)/test/cpp/util/echo_duplicate.pb.cc \ + $(GENDIR)/test/cpp/util/messages.pb.cc $(GENDIR)/test/cpp/util/messages.grpc.pb.cc \ + $(GENDIR)/test/cpp/util/echo.pb.cc $(GENDIR)/test/cpp/util/echo.grpc.pb.cc \ + $(GENDIR)/test/cpp/util/echo_duplicate.pb.cc $(GENDIR)/test/cpp/util/echo_duplicate.grpc.pb.cc \ test/cpp/util/cli_call.cc \ test/cpp/util/create_test_channel.cc \ @@ -3480,8 +3540,8 @@ endif -$(OBJDIR)/$(CONFIG)/test/cpp/util/cli_call.o: $(GENDIR)/test/cpp/util/messages.pb.cc $(GENDIR)/test/cpp/util/echo.pb.cc $(GENDIR)/test/cpp/util/echo_duplicate.pb.cc -$(OBJDIR)/$(CONFIG)/test/cpp/util/create_test_channel.o: $(GENDIR)/test/cpp/util/messages.pb.cc $(GENDIR)/test/cpp/util/echo.pb.cc $(GENDIR)/test/cpp/util/echo_duplicate.pb.cc +$(OBJDIR)/$(CONFIG)/test/cpp/util/cli_call.o: $(GENDIR)/test/cpp/util/messages.pb.cc $(GENDIR)/test/cpp/util/messages.grpc.pb.cc $(GENDIR)/test/cpp/util/echo.pb.cc $(GENDIR)/test/cpp/util/echo.grpc.pb.cc $(GENDIR)/test/cpp/util/echo_duplicate.pb.cc $(GENDIR)/test/cpp/util/echo_duplicate.grpc.pb.cc +$(OBJDIR)/$(CONFIG)/test/cpp/util/create_test_channel.o: $(GENDIR)/test/cpp/util/messages.pb.cc $(GENDIR)/test/cpp/util/messages.grpc.pb.cc $(GENDIR)/test/cpp/util/echo.pb.cc $(GENDIR)/test/cpp/util/echo.grpc.pb.cc $(GENDIR)/test/cpp/util/echo_duplicate.pb.cc $(GENDIR)/test/cpp/util/echo_duplicate.grpc.pb.cc LIBGRPC++_UNSECURE_SRC = \ @@ -3658,9 +3718,9 @@ $(OBJDIR)/$(CONFIG)/src/compiler/ruby_generator.o: LIBPUBSUB_CLIENT_LIB_SRC = \ - $(GENDIR)/examples/pubsub/label.pb.cc \ - $(GENDIR)/examples/pubsub/empty.pb.cc \ - $(GENDIR)/examples/pubsub/pubsub.pb.cc \ + $(GENDIR)/examples/pubsub/label.pb.cc $(GENDIR)/examples/pubsub/label.grpc.pb.cc \ + $(GENDIR)/examples/pubsub/empty.pb.cc $(GENDIR)/examples/pubsub/empty.grpc.pb.cc \ + $(GENDIR)/examples/pubsub/pubsub.pb.cc $(GENDIR)/examples/pubsub/pubsub.grpc.pb.cc \ examples/pubsub/publisher.cc \ examples/pubsub/subscriber.cc \ @@ -3721,12 +3781,12 @@ endif -$(OBJDIR)/$(CONFIG)/examples/pubsub/publisher.o: $(GENDIR)/examples/pubsub/label.pb.cc $(GENDIR)/examples/pubsub/empty.pb.cc $(GENDIR)/examples/pubsub/pubsub.pb.cc -$(OBJDIR)/$(CONFIG)/examples/pubsub/subscriber.o: $(GENDIR)/examples/pubsub/label.pb.cc $(GENDIR)/examples/pubsub/empty.pb.cc $(GENDIR)/examples/pubsub/pubsub.pb.cc +$(OBJDIR)/$(CONFIG)/examples/pubsub/publisher.o: $(GENDIR)/examples/pubsub/label.pb.cc $(GENDIR)/examples/pubsub/label.grpc.pb.cc $(GENDIR)/examples/pubsub/empty.pb.cc $(GENDIR)/examples/pubsub/empty.grpc.pb.cc $(GENDIR)/examples/pubsub/pubsub.pb.cc $(GENDIR)/examples/pubsub/pubsub.grpc.pb.cc +$(OBJDIR)/$(CONFIG)/examples/pubsub/subscriber.o: $(GENDIR)/examples/pubsub/label.pb.cc $(GENDIR)/examples/pubsub/label.grpc.pb.cc $(GENDIR)/examples/pubsub/empty.pb.cc $(GENDIR)/examples/pubsub/empty.grpc.pb.cc $(GENDIR)/examples/pubsub/pubsub.pb.cc $(GENDIR)/examples/pubsub/pubsub.grpc.pb.cc LIBQPS_SRC = \ - $(GENDIR)/test/cpp/qps/qpstest.pb.cc \ + $(GENDIR)/test/cpp/qps/qpstest.pb.cc $(GENDIR)/test/cpp/qps/qpstest.grpc.pb.cc \ test/cpp/qps/driver.cc \ test/cpp/qps/timer.cc \ @@ -3783,8 +3843,8 @@ endif endif -$(OBJDIR)/$(CONFIG)/test/cpp/qps/driver.o: $(GENDIR)/test/cpp/qps/qpstest.pb.cc -$(OBJDIR)/$(CONFIG)/test/cpp/qps/timer.o: $(GENDIR)/test/cpp/qps/qpstest.pb.cc +$(OBJDIR)/$(CONFIG)/test/cpp/qps/driver.o: $(GENDIR)/test/cpp/qps/qpstest.pb.cc $(GENDIR)/test/cpp/qps/qpstest.grpc.pb.cc +$(OBJDIR)/$(CONFIG)/test/cpp/qps/timer.o: $(GENDIR)/test/cpp/qps/qpstest.pb.cc $(GENDIR)/test/cpp/qps/qpstest.grpc.pb.cc LIBGRPC_CSHARP_EXT_SRC = \ @@ -8426,9 +8486,9 @@ endif INTEROP_CLIENT_SRC = \ - $(GENDIR)/test/cpp/interop/empty.pb.cc \ - $(GENDIR)/test/cpp/interop/messages.pb.cc \ - $(GENDIR)/test/cpp/interop/test.pb.cc \ + $(GENDIR)/test/cpp/interop/empty.pb.cc $(GENDIR)/test/cpp/interop/empty.grpc.pb.cc \ + $(GENDIR)/test/cpp/interop/messages.pb.cc $(GENDIR)/test/cpp/interop/messages.grpc.pb.cc \ + $(GENDIR)/test/cpp/interop/test.pb.cc $(GENDIR)/test/cpp/interop/test.grpc.pb.cc \ test/cpp/interop/client.cc \ INTEROP_CLIENT_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(INTEROP_CLIENT_SRC)))) @@ -8474,9 +8534,9 @@ endif INTEROP_SERVER_SRC = \ - $(GENDIR)/test/cpp/interop/empty.pb.cc \ - $(GENDIR)/test/cpp/interop/messages.pb.cc \ - $(GENDIR)/test/cpp/interop/test.pb.cc \ + $(GENDIR)/test/cpp/interop/empty.pb.cc $(GENDIR)/test/cpp/interop/empty.grpc.pb.cc \ + $(GENDIR)/test/cpp/interop/messages.pb.cc $(GENDIR)/test/cpp/interop/messages.grpc.pb.cc \ + $(GENDIR)/test/cpp/interop/test.pb.cc $(GENDIR)/test/cpp/interop/test.grpc.pb.cc \ test/cpp/interop/server.cc \ INTEROP_SERVER_OBJS = $(addprefix $(OBJDIR)/$(CONFIG)/, $(addsuffix .o, $(basename $(INTEROP_SERVER_SRC)))) diff --git a/examples/pubsub/publisher.h b/examples/pubsub/publisher.h index c90406ffef8..33bcf98df48 100644 --- a/examples/pubsub/publisher.h +++ b/examples/pubsub/publisher.h @@ -37,7 +37,7 @@ #include #include -#include "examples/pubsub/pubsub.pb.h" +#include "examples/pubsub/pubsub.grpc.pb.h" namespace grpc { namespace examples { diff --git a/examples/pubsub/subscriber.h b/examples/pubsub/subscriber.h index c587c01b825..40ab45471d5 100644 --- a/examples/pubsub/subscriber.h +++ b/examples/pubsub/subscriber.h @@ -37,7 +37,7 @@ #include #include -#include "examples/pubsub/pubsub.pb.h" +#include "examples/pubsub/pubsub.grpc.pb.h" namespace grpc { namespace examples { diff --git a/src/compiler/cpp_generator.cc b/src/compiler/cpp_generator.cc index 0a84c735200..c78f0333d89 100644 --- a/src/compiler/cpp_generator.cc +++ b/src/compiler/cpp_generator.cc @@ -109,8 +109,47 @@ bool HasBidiStreaming(const grpc::protobuf::FileDescriptor *file) { } return false; } + +grpc::string FilenameIdentifier(const grpc::string& filename) { + grpc::string result; + for (unsigned i = 0; i < filename.size(); i++) { + char c = filename[i]; + if (isalnum(c)) { + result.push_back(c); + } else { + static char hex[] = "0123456789abcdef"; + result.push_back('_'); + result.push_back(hex[(c >> 4) & 0xf]); + result.push_back(hex[c & 0xf]); + } + } + return result; +} } // namespace +grpc::string GetHeaderPrologue(const grpc::protobuf::FileDescriptor *file, + const Parameters ¶ms) { + grpc::string output; + grpc::protobuf::io::StringOutputStream output_stream(&output); + grpc::protobuf::io::Printer printer(&output_stream, '$'); + std::map vars; + + vars["filename"] = file->name(); + vars["filename_identifier"] = FilenameIdentifier(file->name()); + vars["filename_base"] = grpc_generator::StripProto(file->name()); + + printer.Print(vars, "// Generated by the gRPC protobuf plugin.\n"); + printer.Print(vars, "// If you make any local change, they will be lost.\n"); + printer.Print(vars, "// source: $filename$\n"); + printer.Print(vars, "#ifndef GRPC_$filename_identifier$__INCLUDED\n"); + printer.Print(vars, "#define GRPC_$filename_identifier$__INCLUDED\n"); + printer.Print(vars, "\n"); + printer.Print(vars, "#include \"$filename_base$.pb.h\"\n"); + printer.Print(vars, "\n"); + + return output; +} + grpc::string GetHeaderIncludes(const grpc::protobuf::FileDescriptor *file, const Parameters ¶ms) { grpc::string temp = @@ -156,17 +195,21 @@ grpc::string GetHeaderIncludes(const grpc::protobuf::FileDescriptor *file, "class ServerAsyncReaderWriter;\n"); } temp.append("} // namespace grpc\n"); - return temp; -} -grpc::string GetSourceIncludes(const Parameters ¶m) { - return "#include \n" - "#include \n" - "#include \n" - "#include \n" - "#include \n" - "#include \n" - "#include \n"; + temp.append("\n"); + + std::vector parts = + grpc_generator::tokenize(file->package(), "."); + + for (auto part = parts.begin(); part != parts.end(); part++) { + temp.append("namespace "); + temp.append(*part); + temp.append(" {\n"); + } + + temp.append("\n"); + + return temp; } void PrintHeaderClientMethod(grpc::protobuf::io::Printer *printer, @@ -378,6 +421,78 @@ grpc::string GetHeaderServices(const grpc::protobuf::FileDescriptor *file, return output; } +grpc::string GetHeaderEpilogue(const grpc::protobuf::FileDescriptor *file, + const Parameters ¶ms) { + grpc::string output; + grpc::protobuf::io::StringOutputStream output_stream(&output); + grpc::protobuf::io::Printer printer(&output_stream, '$'); + std::map vars; + + vars["filename"] = file->name(); + vars["filename_identifier"] = FilenameIdentifier(file->name()); + + std::vector parts = + grpc_generator::tokenize(file->package(), "."); + + for (auto part = parts.rbegin(); part != parts.rend(); part++) { + vars["part"] = *part; + printer.Print(vars, "} // namespace $part$\n"); + } + + printer.Print(vars, "\n\n"); + printer.Print(vars, "#endif // GRPC_$filename_identifier$__INCLUDED\n"); + + return output; +} + +grpc::string GetSourcePrologue(const grpc::protobuf::FileDescriptor *file, + const Parameters ¶ms) { + grpc::string output; + grpc::protobuf::io::StringOutputStream output_stream(&output); + grpc::protobuf::io::Printer printer(&output_stream, '$'); + std::map vars; + + vars["filename"] = file->name(); + vars["filename_base"] = grpc_generator::StripProto(file->name()); + + printer.Print(vars, "// Generated by the gRPC protobuf plugin.\n"); + printer.Print(vars, "// If you make any local change, they will be lost.\n"); + printer.Print(vars, "// source: $filename$\n\n"); + printer.Print(vars, "#include \"$filename_base$.pb.h\"\n"); + printer.Print(vars, "#include \"$filename_base$.grpc.pb.h\"\n"); + printer.Print(vars, "\n"); + + return output; +} + +grpc::string GetSourceIncludes(const grpc::protobuf::FileDescriptor *file, + const Parameters ¶m) { + grpc::string output; + grpc::protobuf::io::StringOutputStream output_stream(&output); + grpc::protobuf::io::Printer printer(&output_stream, '$'); + std::map vars; + + printer.Print(vars, "#include \n"); + printer.Print(vars, "#include \n"); + printer.Print(vars, "#include \n"); + printer.Print(vars, "#include \n"); + printer.Print(vars, "#include \n"); + printer.Print(vars, "#include \n"); + printer.Print(vars, "#include \n"); + + std::vector parts = + grpc_generator::tokenize(file->package(), "."); + + for (auto part = parts.begin(); part != parts.end(); part++) { + vars["part"] = *part; + printer.Print(vars, "namespace $part$ {\n"); + } + + printer.Print(vars, "\n"); + + return output; +} + void PrintSourceClientMethod(grpc::protobuf::io::Printer *printer, const grpc::protobuf::MethodDescriptor *method, std::map *vars) { @@ -741,4 +856,22 @@ grpc::string GetSourceServices(const grpc::protobuf::FileDescriptor *file, return output; } +grpc::string GetSourceEpilogue(const grpc::protobuf::FileDescriptor *file, + const Parameters ¶ms) { + grpc::string temp; + + std::vector parts = + grpc_generator::tokenize(file->package(), "."); + + for (auto part = parts.begin(); part != parts.end(); part++) { + temp.append("} // namespace "); + temp.append(*part); + temp.append("\n"); + } + + temp.append("\n"); + + return temp; +} + } // namespace grpc_cpp_generator diff --git a/src/compiler/cpp_generator.h b/src/compiler/cpp_generator.h index 04ad71c0673..70c2e985f6b 100644 --- a/src/compiler/cpp_generator.h +++ b/src/compiler/cpp_generator.h @@ -44,12 +44,25 @@ struct Parameters { grpc::string services_namespace; }; +// Return the prologue of the generated header file. +grpc::string GetHeaderPrologue(const grpc::protobuf::FileDescriptor *file, + const Parameters ¶ms); + // Return the includes needed for generated header file. grpc::string GetHeaderIncludes(const grpc::protobuf::FileDescriptor *file, const Parameters ¶ms); // Return the includes needed for generated source file. -grpc::string GetSourceIncludes(const Parameters ¶ms); +grpc::string GetSourceIncludes(const grpc::protobuf::FileDescriptor *file, + const Parameters ¶ms); + +// Return the epilogue of the generated header file. +grpc::string GetHeaderEpilogue(const grpc::protobuf::FileDescriptor *file, + const Parameters ¶ms); + +// Return the prologue of the generated source file. +grpc::string GetSourcePrologue(const grpc::protobuf::FileDescriptor *file, + const Parameters ¶ms); // Return the services for generated header file. grpc::string GetHeaderServices(const grpc::protobuf::FileDescriptor *file, @@ -59,6 +72,10 @@ grpc::string GetHeaderServices(const grpc::protobuf::FileDescriptor *file, grpc::string GetSourceServices(const grpc::protobuf::FileDescriptor *file, const Parameters ¶ms); +// Return the epilogue of the generated source file. +grpc::string GetSourceEpilogue(const grpc::protobuf::FileDescriptor *file, + const Parameters ¶ms); + } // namespace grpc_cpp_generator #endif // GRPC_INTERNAL_COMPILER_CPP_GENERATOR_H diff --git a/src/compiler/cpp_plugin.cc b/src/compiler/cpp_plugin.cc index acbe128213a..88c704948ec 100644 --- a/src/compiler/cpp_plugin.cc +++ b/src/compiler/cpp_plugin.cc @@ -58,11 +58,6 @@ class CppGrpcGenerator : public grpc::protobuf::compiler::CodeGenerator { return false; } - if (file->service_count() == 0) { - // No services. Do nothing. - return true; - } - grpc_cpp_generator::Parameters generator_parameters; if (!parameter.empty()) { @@ -84,16 +79,27 @@ class CppGrpcGenerator : public grpc::protobuf::compiler::CodeGenerator { grpc::string file_name = grpc_generator::StripProto(file->name()); - // Generate .pb.h - Insert(context, file_name + ".pb.h", "includes", - grpc_cpp_generator::GetHeaderIncludes(file, generator_parameters)); - Insert(context, file_name + ".pb.h", "namespace_scope", - grpc_cpp_generator::GetHeaderServices(file, generator_parameters)); - // Generate .pb.cc - Insert(context, file_name + ".pb.cc", "includes", - grpc_cpp_generator::GetSourceIncludes(generator_parameters)); - Insert(context, file_name + ".pb.cc", "namespace_scope", - grpc_cpp_generator::GetSourceServices(file, generator_parameters)); + grpc::string header_code = + grpc_cpp_generator::GetHeaderPrologue(file, generator_parameters) + + grpc_cpp_generator::GetHeaderIncludes(file, generator_parameters) + + grpc_cpp_generator::GetHeaderServices(file, generator_parameters) + + grpc_cpp_generator::GetHeaderEpilogue(file, generator_parameters); + std::unique_ptr header_output( + context->Open(file_name + ".grpc.pb.h")); + grpc::protobuf::io::CodedOutputStream header_coded_out( + header_output.get()); + header_coded_out.WriteRaw(header_code.data(), header_code.size()); + + grpc::string source_code = + grpc_cpp_generator::GetSourcePrologue(file, generator_parameters) + + grpc_cpp_generator::GetSourceIncludes(file, generator_parameters) + + grpc_cpp_generator::GetSourceServices(file, generator_parameters) + + grpc_cpp_generator::GetSourceEpilogue(file, generator_parameters); + std::unique_ptr source_output( + context->Open(file_name + ".grpc.pb.cc")); + grpc::protobuf::io::CodedOutputStream source_coded_out( + source_output.get()); + source_coded_out.WriteRaw(source_code.data(), source_code.size()); return true; } diff --git a/templates/Makefile.template b/templates/Makefile.template index 848ee87bbc0..776ee7a49b9 100644 --- a/templates/Makefile.template +++ b/templates/Makefile.template @@ -35,17 +35,11 @@ proto_re = re.compile('(.*)\\.proto') - def excluded(filename, exclude_res): - for r in exclude_res: - if r.match(filename): - return True - return False - def proto_to_cc(filename): m = proto_re.match(filename) if not m: return filename - return '$(GENDIR)/' + m.group(1) + '.pb.cc' + return '$(GENDIR)/' + m.group(1) + '.pb.cc $(GENDIR)/' + m.group(1) + '.grpc.pb.cc' %> @@ -840,11 +834,17 @@ endif % for p in protos: ifeq ($(NO_PROTOC),true) $(GENDIR)/${p}.pb.cc: protoc_dep_error +$(GENDIR)/${p}.grpc.pb.cc: protoc_dep_error else $(GENDIR)/${p}.pb.cc: ${p}.proto $(PROTOBUF_DEP) $(PROTOC_PLUGINS) $(E) "[PROTOC] Generating protobuf CC file from $<" $(Q) mkdir -p `dirname $@` - $(Q) $(PROTOC) --cpp_out=$(GENDIR) --grpc_out=$(GENDIR) --plugin=protoc-gen-grpc=$(BINDIR)/$(CONFIG)/grpc_cpp_plugin $< + $(Q) $(PROTOC) --cpp_out=$(GENDIR) $< + +$(GENDIR)/${p}.grpc.pb.cc: ${p}.proto $(PROTOBUF_DEP) $(PROTOC_PLUGINS) + $(E) "[GRPC] Generating gRPC's protobuf service CC file from $<" + $(Q) mkdir -p `dirname $@` + $(Q) $(PROTOC) --grpc_out=$(GENDIR) --plugin=protoc-gen-grpc=$(BINDIR)/$(CONFIG)/grpc_cpp_plugin $< endif % endfor diff --git a/test/cpp/end2end/async_end2end_test.cc b/test/cpp/end2end/async_end2end_test.cc index 9938fcf12eb..dd294d95163 100644 --- a/test/cpp/end2end/async_end2end_test.cc +++ b/test/cpp/end2end/async_end2end_test.cc @@ -35,8 +35,8 @@ #include #include "test/core/util/test_config.h" -#include "test/cpp/util/echo_duplicate.pb.h" -#include "test/cpp/util/echo.pb.h" +#include "test/cpp/util/echo_duplicate.grpc.pb.h" +#include "test/cpp/util/echo.grpc.pb.h" #include "src/cpp/util/time.h" #include #include diff --git a/test/cpp/end2end/end2end_test.cc b/test/cpp/end2end/end2end_test.cc index 0d5db046df4..f96051cafac 100644 --- a/test/cpp/end2end/end2end_test.cc +++ b/test/cpp/end2end/end2end_test.cc @@ -35,8 +35,8 @@ #include #include "test/core/util/test_config.h" -#include "test/cpp/util/echo_duplicate.pb.h" -#include "test/cpp/util/echo.pb.h" +#include "test/cpp/util/echo_duplicate.grpc.pb.h" +#include "test/cpp/util/echo.grpc.pb.h" #include "src/cpp/util/time.h" #include "src/cpp/server/thread_pool.h" #include diff --git a/test/cpp/end2end/generic_end2end_test.cc b/test/cpp/end2end/generic_end2end_test.cc index 9cdd8c94d12..eb6f5369a92 100644 --- a/test/cpp/end2end/generic_end2end_test.cc +++ b/test/cpp/end2end/generic_end2end_test.cc @@ -38,7 +38,7 @@ #include "src/cpp/util/time.h" #include "test/core/util/port.h" #include "test/core/util/test_config.h" -#include "test/cpp/util/echo.pb.h" +#include "test/cpp/util/echo.grpc.pb.h" #include #include #include diff --git a/test/cpp/interop/client.cc b/test/cpp/interop/client.cc index de6c6b7b77e..7c5e1aa715a 100644 --- a/test/cpp/interop/client.cc +++ b/test/cpp/interop/client.cc @@ -51,9 +51,9 @@ #include #include #include "test/cpp/util/create_test_channel.h" -#include "test/cpp/interop/test.pb.h" -#include "test/cpp/interop/empty.pb.h" -#include "test/cpp/interop/messages.pb.h" +#include "test/cpp/interop/test.grpc.pb.h" +#include "test/cpp/interop/empty.grpc.pb.h" +#include "test/cpp/interop/messages.grpc.pb.h" DEFINE_bool(enable_ssl, false, "Whether to use ssl/tls."); DEFINE_bool(use_prod_roots, false, "True to use SSL roots for google"); diff --git a/test/cpp/interop/server.cc b/test/cpp/interop/server.cc index 780a7370acb..87bf48938bc 100644 --- a/test/cpp/interop/server.cc +++ b/test/cpp/interop/server.cc @@ -49,9 +49,9 @@ #include #include #include -#include "test/cpp/interop/test.pb.h" -#include "test/cpp/interop/empty.pb.h" -#include "test/cpp/interop/messages.pb.h" +#include "test/cpp/interop/test.grpc.pb.h" +#include "test/cpp/interop/empty.grpc.pb.h" +#include "test/cpp/interop/messages.grpc.pb.h" DEFINE_bool(enable_ssl, false, "Whether to use ssl/tls."); DEFINE_int32(port, 0, "Server port."); diff --git a/test/cpp/qps/client.h b/test/cpp/qps/client.h index cae7f44537c..08b338c7473 100644 --- a/test/cpp/qps/client.h +++ b/test/cpp/qps/client.h @@ -36,7 +36,7 @@ #include "test/cpp/qps/histogram.h" #include "test/cpp/qps/timer.h" -#include "test/cpp/qps/qpstest.pb.h" +#include "test/cpp/qps/qpstest.grpc.pb.h" #include #include diff --git a/test/cpp/qps/client_async.cc b/test/cpp/qps/client_async.cc index 1ed3c7157fc..c01d8401781 100644 --- a/test/cpp/qps/client_async.cc +++ b/test/cpp/qps/client_async.cc @@ -48,7 +48,7 @@ #include #include #include "test/cpp/util/create_test_channel.h" -#include "test/cpp/qps/qpstest.pb.h" +#include "test/cpp/qps/qpstest.grpc.pb.h" #include "test/cpp/qps/timer.h" #include "test/cpp/qps/client.h" diff --git a/test/cpp/qps/client_sync.cc b/test/cpp/qps/client_sync.cc index 77da1725ffa..947618121b3 100644 --- a/test/cpp/qps/client_sync.cc +++ b/test/cpp/qps/client_sync.cc @@ -55,7 +55,7 @@ #include #include "test/cpp/util/create_test_channel.h" #include "test/cpp/qps/client.h" -#include "test/cpp/qps/qpstest.pb.h" +#include "test/cpp/qps/qpstest.grpc.pb.h" #include "test/cpp/qps/histogram.h" #include "test/cpp/qps/timer.h" diff --git a/test/cpp/qps/driver.h b/test/cpp/qps/driver.h index d87e80dc552..348c032bf30 100644 --- a/test/cpp/qps/driver.h +++ b/test/cpp/qps/driver.h @@ -35,7 +35,7 @@ #define TEST_QPS_DRIVER_H #include "test/cpp/qps/histogram.h" -#include "test/cpp/qps/qpstest.pb.h" +#include "test/cpp/qps/qpstest.grpc.pb.h" namespace grpc { namespace testing { diff --git a/test/cpp/qps/histogram.h b/test/cpp/qps/histogram.h index 7ba00e94c39..eb17821cdc9 100644 --- a/test/cpp/qps/histogram.h +++ b/test/cpp/qps/histogram.h @@ -35,7 +35,7 @@ #define TEST_QPS_HISTOGRAM_H #include -#include "test/cpp/qps/qpstest.pb.h" +#include "test/cpp/qps/qpstest.grpc.pb.h" namespace grpc { namespace testing { diff --git a/test/cpp/qps/server.h b/test/cpp/qps/server.h index ef71cb94d00..68e01154101 100644 --- a/test/cpp/qps/server.h +++ b/test/cpp/qps/server.h @@ -35,7 +35,7 @@ #define TEST_QPS_SERVER_H #include "test/cpp/qps/timer.h" -#include "test/cpp/qps/qpstest.pb.h" +#include "test/cpp/qps/qpstest.grpc.pb.h" namespace grpc { namespace testing { diff --git a/test/cpp/qps/server_async.cc b/test/cpp/qps/server_async.cc index 65c170af811..2e366a8d722 100644 --- a/test/cpp/qps/server_async.cc +++ b/test/cpp/qps/server_async.cc @@ -52,7 +52,7 @@ #include #include #include "src/cpp/server/thread_pool.h" -#include "test/cpp/qps/qpstest.pb.h" +#include "test/cpp/qps/qpstest.grpc.pb.h" #include "test/cpp/qps/server.h" #include diff --git a/test/cpp/qps/server_sync.cc b/test/cpp/qps/server_sync.cc index 99644299010..2770233a7c0 100644 --- a/test/cpp/qps/server_sync.cc +++ b/test/cpp/qps/server_sync.cc @@ -47,7 +47,7 @@ #include #include #include "src/cpp/server/thread_pool.h" -#include "test/cpp/qps/qpstest.pb.h" +#include "test/cpp/qps/qpstest.grpc.pb.h" #include "test/cpp/qps/server.h" #include "test/cpp/qps/timer.h" diff --git a/test/cpp/qps/worker.cc b/test/cpp/qps/worker.cc index b6830cc0557..101eb9f9694 100644 --- a/test/cpp/qps/worker.cc +++ b/test/cpp/qps/worker.cc @@ -55,7 +55,7 @@ #include #include "test/core/util/grpc_profiler.h" #include "test/cpp/util/create_test_channel.h" -#include "test/cpp/qps/qpstest.pb.h" +#include "test/cpp/qps/qpstest.grpc.pb.h" #include "test/cpp/qps/client.h" #include "test/cpp/qps/server.h" diff --git a/test/cpp/util/cli_call_test.cc b/test/cpp/util/cli_call_test.cc index 91fc40c31f0..32ef392cc4c 100644 --- a/test/cpp/util/cli_call_test.cc +++ b/test/cpp/util/cli_call_test.cc @@ -33,7 +33,7 @@ #include "test/core/util/test_config.h" #include "test/cpp/util/cli_call.h" -#include "test/cpp/util/echo.pb.h" +#include "test/cpp/util/echo.grpc.pb.h" #include "src/cpp/server/thread_pool.h" #include #include From 41faf0f4c893b57ba5a9ee1cf1ad329c9c9cec08 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 10 Apr 2015 08:06:05 -0700 Subject: [PATCH 10/11] s/11/1 --- test/cpp/qps/client_async.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cpp/qps/client_async.cc b/test/cpp/qps/client_async.cc index 7e2850dd659..3ab06bd7c72 100644 --- a/test/cpp/qps/client_async.cc +++ b/test/cpp/qps/client_async.cc @@ -176,7 +176,7 @@ class AsyncUnaryClient GRPC_FINAL : public Client { bool ThreadFunc(Histogram* histogram, size_t thread_idx) GRPC_OVERRIDE { void* got_tag; bool ok; - switch (cli_cqs_[thread_idx]->AsyncNext(&got_tag, &ok, std::chrono::system_clock::now() + std::chrono::seconds(11))) { + switch (cli_cqs_[thread_idx]->AsyncNext(&got_tag, &ok, std::chrono::system_clock::now() + std::chrono::seconds(1))) { case CompletionQueue::SHUTDOWN: return false; case CompletionQueue::TIMEOUT: return true; case CompletionQueue::GOT_EVENT: break; From 66048f4fc1c0b181e05ef21f7821443abd258240 Mon Sep 17 00:00:00 2001 From: Craig Tiller Date: Fri, 10 Apr 2015 08:23:52 -0700 Subject: [PATCH 11/11] Add reporting library, use it --- Makefile | 3 ++ build.json | 2 + test/cpp/qps/histogram.h | 4 +- test/cpp/qps/qps_driver.cc | 37 ++------------- test/cpp/qps/report.cc | 94 ++++++++++++++++++++++++++++++++++++++ test/cpp/qps/report.h | 57 +++++++++++++++++++++++ test/cpp/qps/smoke_test.cc | 48 ++++--------------- 7 files changed, 171 insertions(+), 74 deletions(-) create mode 100644 test/cpp/qps/report.cc create mode 100644 test/cpp/qps/report.h diff --git a/Makefile b/Makefile index 6665eb418cb..d98202db01f 100644 --- a/Makefile +++ b/Makefile @@ -3729,6 +3729,7 @@ $(OBJDIR)/$(CONFIG)/examples/pubsub/subscriber.o: $(GENDIR)/examples/pubsub/ LIBQPS_SRC = \ $(GENDIR)/test/cpp/qps/qpstest.pb.cc \ test/cpp/qps/driver.cc \ + test/cpp/qps/report.cc \ test/cpp/qps/timer.cc \ @@ -3758,6 +3759,7 @@ ifneq ($(OPENSSL_DEP),) # otherwise parallel compilation will fail if a source is compiled first. test/cpp/qps/qpstest.proto: $(OPENSSL_DEP) test/cpp/qps/driver.cc: $(OPENSSL_DEP) +test/cpp/qps/report.cc: $(OPENSSL_DEP) test/cpp/qps/timer.cc: $(OPENSSL_DEP) endif @@ -3785,6 +3787,7 @@ endif $(OBJDIR)/$(CONFIG)/test/cpp/qps/driver.o: $(GENDIR)/test/cpp/qps/qpstest.pb.cc +$(OBJDIR)/$(CONFIG)/test/cpp/qps/report.o: $(GENDIR)/test/cpp/qps/qpstest.pb.cc $(OBJDIR)/$(CONFIG)/test/cpp/qps/timer.o: $(GENDIR)/test/cpp/qps/qpstest.pb.cc diff --git a/build.json b/build.json index f2a39c6f2c5..6f37a0a7287 100644 --- a/build.json +++ b/build.json @@ -554,11 +554,13 @@ "language": "c++", "headers": [ "test/cpp/qps/driver.h", + "test/cpp/qps/report.h", "test/cpp/qps/timer.h" ], "src": [ "test/cpp/qps/qpstest.proto", "test/cpp/qps/driver.cc", + "test/cpp/qps/report.cc", "test/cpp/qps/timer.cc" ] }, diff --git a/test/cpp/qps/histogram.h b/test/cpp/qps/histogram.h index 7ba00e94c39..fbb80abf87d 100644 --- a/test/cpp/qps/histogram.h +++ b/test/cpp/qps/histogram.h @@ -50,10 +50,10 @@ class Histogram { void Merge(Histogram* h) { gpr_histogram_merge(impl_, h->impl_); } void Add(double value) { gpr_histogram_add(impl_, value); } - double Percentile(double pctile) { + double Percentile(double pctile) const { return gpr_histogram_percentile(impl_, pctile); } - double Count() { return gpr_histogram_count(impl_); } + double Count() const { return gpr_histogram_count(impl_); } void Swap(Histogram* other) { std::swap(impl_, other->impl_); } void FillProto(HistogramData* p) { size_t n; diff --git a/test/cpp/qps/qps_driver.cc b/test/cpp/qps/qps_driver.cc index f42d538b165..220f826118f 100644 --- a/test/cpp/qps/qps_driver.cc +++ b/test/cpp/qps/qps_driver.cc @@ -35,7 +35,7 @@ #include #include "test/cpp/qps/driver.h" -#include "test/cpp/qps/stats.h" +#include "test/cpp/qps/report.h" DEFINE_int32(num_clients, 1, "Number of client binaries"); DEFINE_int32(num_servers, 1, "Number of server binaries"); @@ -65,7 +65,6 @@ using grpc::testing::ClientType; using grpc::testing::ServerType; using grpc::testing::RpcType; using grpc::testing::ResourceUsage; -using grpc::testing::sum; // In some distros, gflags is in the namespace google, and in some others, // in gflags. This hack is enabling us to find both. @@ -105,37 +104,9 @@ int main(int argc, char** argv) { server_config, FLAGS_num_servers, FLAGS_warmup_seconds, FLAGS_benchmark_seconds); - gpr_log(GPR_INFO, "QPS: %.1f", - result.latencies.Count() / - average(result.client_resources, - [](ResourceUsage u) { return u.wall_time; })); - - gpr_log(GPR_INFO, "Latencies (50/95/99/99.9%%-ile): %.1f/%.1f/%.1f/%.1f us", - result.latencies.Percentile(50) / 1000, - result.latencies.Percentile(95) / 1000, - result.latencies.Percentile(99) / 1000, - result.latencies.Percentile(99.9) / 1000); - - gpr_log(GPR_INFO, "Server system time: %.2f%%", - 100.0 * sum(result.server_resources, - [](ResourceUsage u) { return u.system_time; }) / - sum(result.server_resources, - [](ResourceUsage u) { return u.wall_time; })); - gpr_log(GPR_INFO, "Server user time: %.2f%%", - 100.0 * sum(result.server_resources, - [](ResourceUsage u) { return u.user_time; }) / - sum(result.server_resources, - [](ResourceUsage u) { return u.wall_time; })); - gpr_log(GPR_INFO, "Client system time: %.2f%%", - 100.0 * sum(result.client_resources, - [](ResourceUsage u) { return u.system_time; }) / - sum(result.client_resources, - [](ResourceUsage u) { return u.wall_time; })); - gpr_log(GPR_INFO, "Client user time: %.2f%%", - 100.0 * sum(result.client_resources, - [](ResourceUsage u) { return u.user_time; }) / - sum(result.client_resources, - [](ResourceUsage u) { return u.wall_time; })); + ReportQPSPerCore(result, server_config); + ReportLatency(result); + ReportTimes(result); grpc_shutdown(); return 0; diff --git a/test/cpp/qps/report.cc b/test/cpp/qps/report.cc new file mode 100644 index 00000000000..29d88da344a --- /dev/null +++ b/test/cpp/qps/report.cc @@ -0,0 +1,94 @@ +/* + * + * Copyright 2015, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#include "test/cpp/qps/report.h" + +#include +#include "test/cpp/qps/stats.h" + +namespace grpc { +namespace testing { + +// QPS: XXX +void ReportQPS(const ScenarioResult& result) { + gpr_log(GPR_INFO, "QPS: %.1f", + result.latencies.Count() / + average(result.client_resources, + [](ResourceUsage u) { return u.wall_time; })); +} + +// QPS: XXX (YYY/server core) +void ReportQPSPerCore(const ScenarioResult& result, const ServerConfig& server_config) { + auto qps = + result.latencies.Count() / + average(result.client_resources, + [](ResourceUsage u) { return u.wall_time; }); + + gpr_log(GPR_INFO, "QPS: %.1f (%.1f/server core)", qps, qps/server_config.threads()); +} + +// Latency (50/90/95/99/99.9%-ile): AA/BB/CC/DD/EE us +void ReportLatency(const ScenarioResult& result) { + gpr_log(GPR_INFO, "Latencies (50/90/95/99/99.9%%-ile): %.1f/%.1f/%.1f/%.1f/%.1f us", + result.latencies.Percentile(50) / 1000, + result.latencies.Percentile(90) / 1000, + result.latencies.Percentile(95) / 1000, + result.latencies.Percentile(99) / 1000, + result.latencies.Percentile(99.9) / 1000); +} + +void ReportTimes(const ScenarioResult& result) { + gpr_log(GPR_INFO, "Server system time: %.2f%%", + 100.0 * sum(result.server_resources, + [](ResourceUsage u) { return u.system_time; }) / + sum(result.server_resources, + [](ResourceUsage u) { return u.wall_time; })); + gpr_log(GPR_INFO, "Server user time: %.2f%%", + 100.0 * sum(result.server_resources, + [](ResourceUsage u) { return u.user_time; }) / + sum(result.server_resources, + [](ResourceUsage u) { return u.wall_time; })); + gpr_log(GPR_INFO, "Client system time: %.2f%%", + 100.0 * sum(result.client_resources, + [](ResourceUsage u) { return u.system_time; }) / + sum(result.client_resources, + [](ResourceUsage u) { return u.wall_time; })); + gpr_log(GPR_INFO, "Client user time: %.2f%%", + 100.0 * sum(result.client_resources, + [](ResourceUsage u) { return u.user_time; }) / + sum(result.client_resources, + [](ResourceUsage u) { return u.wall_time; })); +} + +} // namespace testing +} // namespace grpc diff --git a/test/cpp/qps/report.h b/test/cpp/qps/report.h new file mode 100644 index 00000000000..343e426ca4a --- /dev/null +++ b/test/cpp/qps/report.h @@ -0,0 +1,57 @@ +/* + * + * Copyright 2015, Google Inc. + * All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions are + * met: + * + * * Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * * Redistributions in binary form must reproduce the above + * copyright notice, this list of conditions and the following disclaimer + * in the documentation and/or other materials provided with the + * distribution. + * * Neither the name of Google Inc. nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + * + */ + +#ifndef TEST_QPS_REPORT_H +#define TEST_QPS_REPORT_H + +#include "test/cpp/qps/driver.h" + +namespace grpc { +namespace testing { + +// QPS: XXX +void ReportQPS(const ScenarioResult& result); +// QPS: XXX (YYY/server core) +void ReportQPSPerCore(const ScenarioResult& result, const ServerConfig& config); +// Latency (50/90/95/99/99.9%-ile): AA/BB/CC/DD/EE us +void ReportLatency(const ScenarioResult& result); +// Server system time: XX% +// Server user time: XX% +// Client system time: XX% +// Client user time: XX% +void ReportTimes(const ScenarioResult& result); + +} // namespace testing +} // namespace grpc + +#endif diff --git a/test/cpp/qps/smoke_test.cc b/test/cpp/qps/smoke_test.cc index 5cdabb88a0d..c9d321f133c 100644 --- a/test/cpp/qps/smoke_test.cc +++ b/test/cpp/qps/smoke_test.cc @@ -34,7 +34,7 @@ #include #include "test/cpp/qps/driver.h" -#include "test/cpp/qps/stats.h" +#include "test/cpp/qps/report.h" namespace grpc { namespace testing { @@ -60,15 +60,8 @@ static void RunSynchronousUnaryPingPong() { auto result = RunScenario(client_config, 1, server_config, 1, WARMUP, BENCHMARK); - gpr_log(GPR_INFO, "QPS: %.1f", - result.latencies.Count() / - average(result.client_resources, - [](ResourceUsage u) { return u.wall_time; })); - gpr_log(GPR_INFO, "Latencies (50/95/99/99.9%%-ile): %.1f/%.1f/%.1f/%.1f us", - result.latencies.Percentile(50) / 1000, - result.latencies.Percentile(95) / 1000, - result.latencies.Percentile(99) / 1000, - result.latencies.Percentile(99.9) / 1000); + ReportQPS(result); + ReportLatency(result); } static void RunSynchronousStreamingPingPong() { @@ -89,15 +82,8 @@ static void RunSynchronousStreamingPingPong() { auto result = RunScenario(client_config, 1, server_config, 1, WARMUP, BENCHMARK); - gpr_log(GPR_INFO, "QPS: %.1f", - result.latencies.Count() / - average(result.client_resources, - [](ResourceUsage u) { return u.wall_time; })); - gpr_log(GPR_INFO, "Latencies (50/95/99/99.9%%-ile): %.1f/%.1f/%.1f/%.1f us", - result.latencies.Percentile(50) / 1000, - result.latencies.Percentile(95) / 1000, - result.latencies.Percentile(99) / 1000, - result.latencies.Percentile(99.9) / 1000); + ReportQPS(result); + ReportLatency(result); } static void RunAsyncUnaryPingPong() { @@ -119,15 +105,8 @@ static void RunAsyncUnaryPingPong() { auto result = RunScenario(client_config, 1, server_config, 1, WARMUP, BENCHMARK); - gpr_log(GPR_INFO, "QPS: %.1f", - result.latencies.Count() / - average(result.client_resources, - [](ResourceUsage u) { return u.wall_time; })); - gpr_log(GPR_INFO, "Latencies (50/95/99/99.9%%-ile): %.1f/%.1f/%.1f/%.1f us", - result.latencies.Percentile(50) / 1000, - result.latencies.Percentile(95) / 1000, - result.latencies.Percentile(99) / 1000, - result.latencies.Percentile(99.9) / 1000); + ReportQPS(result); + ReportLatency(result); } static void RunQPS() { @@ -149,17 +128,8 @@ static void RunQPS() { auto result = RunScenario(client_config, 1, server_config, 1, WARMUP, BENCHMARK); - auto qps = - result.latencies.Count() / - average(result.client_resources, - [](ResourceUsage u) { return u.wall_time; }); - - gpr_log(GPR_INFO, "QPS: %.1f (%.1f/core)", qps, qps/client_config.client_channels()); - gpr_log(GPR_INFO, "Latencies (50/95/99/99.9%%-ile): %.1f/%.1f/%.1f/%.1f us", - result.latencies.Percentile(50) / 1000, - result.latencies.Percentile(95) / 1000, - result.latencies.Percentile(99) / 1000, - result.latencies.Percentile(99.9) / 1000); + ReportQPSPerCore(result, server_config); + ReportLatency(result); } } // namespace testing