From ac3d765041e39d235ca82d82230a57a65c9c544e Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 5 Oct 2017 15:27:01 +0200 Subject: [PATCH 1/5] retry when uploading results to bigquery --- .../run_tests/python_utils/upload_test_results.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tools/run_tests/python_utils/upload_test_results.py b/tools/run_tests/python_utils/upload_test_results.py index 580e7f7d810..15e827769e1 100644 --- a/tools/run_tests/python_utils/upload_test_results.py +++ b/tools/run_tests/python_utils/upload_test_results.py @@ -102,6 +102,15 @@ def upload_results_to_bq(resultset, bq_table, args, platform): test_results['timestamp'] = time.strftime('%Y-%m-%d %H:%M:%S') row = big_query_utils.make_row(str(uuid.uuid4()), test_results) - if not big_query_utils.insert_rows(bq, _PROJECT_ID, _DATASET_ID, bq_table, [row]): - print('Error uploading result to bigquery.') - sys.exit(1) + + # TODO(jtattermusch): rows are inserted one by one, very inefficient + max_retries = 3 + for attempt in range(max_retries): + if big_query_utils.insert_rows(bq, _PROJECT_ID, _DATASET_ID, bq_table, [row]): + break + else: + if attempt < max_retries - 1: + print('Error uploading result to bigquery, will retry.') + else: + print('Error uploading result to bigquery, all attempts failed.') + sys.exit(1) From 8dbc2364b3b68bbf7e43eac3dea8e4d79e649d1d Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 5 Oct 2017 17:15:27 +0200 Subject: [PATCH 2/5] report elapsed time for jobs that time out --- tools/run_tests/python_utils/jobset.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tools/run_tests/python_utils/jobset.py b/tools/run_tests/python_utils/jobset.py index 82a3bc14352..d523095e703 100755 --- a/tools/run_tests/python_utils/jobset.py +++ b/tools/run_tests/python_utils/jobset.py @@ -306,8 +306,8 @@ class Job(object): else: self._state = _FAILURE if not self._suppress_failure_message: - message('FAILED', '%s [ret=%d, pid=%d]' % ( - self._spec.shortname, self._process.returncode, self._process.pid), + message('FAILED', '%s [ret=%d, pid=%d, time=%.1fsec]' % ( + self._spec.shortname, self._process.returncode, self._process.pid, elapsed), stdout(), do_newline=True) self.result.state = 'FAILED' self.result.num_failures += 1 @@ -326,7 +326,7 @@ class Job(object): self.result.cpu_estimated = float('%.01f' % self._spec.cpu_cost) measurement = '; cpu_cost=%.01f; estimated=%.01f' % (self.result.cpu_measured, self.result.cpu_estimated) if not self._quiet_success: - message('PASSED', '%s [time=%.1fsec; retries=%d:%d%s]' % ( + message('PASSED', '%s [time=%.1fsec, retries=%d:%d%s]' % ( self._spec.shortname, elapsed, self._retries, self._timeout_retries, measurement), stdout() if self._spec.verbose_success else None, do_newline=self._newline_on_success or self._travis) @@ -334,6 +334,8 @@ class Job(object): elif (self._state == _RUNNING and self._spec.timeout_seconds is not None and time.time() - self._start > self._spec.timeout_seconds): + elapsed = time.time() - self._start + self.result.elapsed_time = elapsed if self._timeout_retries < self._spec.timeout_retries: message('TIMEOUT_FLAKE', '%s [pid=%d]' % (self._spec.shortname, self._process.pid), stdout(), do_newline=True) self._timeout_retries += 1 @@ -344,7 +346,7 @@ class Job(object): self._process.terminate() self.start() else: - message('TIMEOUT', '%s [pid=%d]' % (self._spec.shortname, self._process.pid), stdout(), do_newline=True) + message('TIMEOUT', '%s [pid=%d, time=%.1fsec]' % (self._spec.shortname, self._process.pid, elapsed), stdout(), do_newline=True) self.kill() self.result.state = 'TIMEOUT' self.result.num_failures += 1 From c4afc644f736d3fd070716579f41baa2de19dfc2 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Thu, 5 Oct 2017 14:40:34 -0700 Subject: [PATCH 3/5] Reduce # of message sizes used in each scenario --- test/cpp/end2end/async_end2end_test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/cpp/end2end/async_end2end_test.cc b/test/cpp/end2end/async_end2end_test.cc index 41090d161aa..3adb0c5a175 100644 --- a/test/cpp/end2end/async_end2end_test.cc +++ b/test/cpp/end2end/async_end2end_test.cc @@ -1788,7 +1788,7 @@ std::vector CreateTestScenarios(bool test_disable_blocking, GPR_ASSERT(!credentials_types.empty()); messages.push_back("Hello"); - for (int sz = 1; sz < test_big_limit; sz *= 2) { + for (int sz = 1; sz <= test_big_limit; sz *= 32) { grpc::string big_msg; for (int i = 0; i < sz * 1024; i++) { char c = 'a' + (i % 26); From 541974a70927d60aa166d4ca98475f0d4e2e6d96 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Thu, 5 Oct 2017 15:12:13 -0700 Subject: [PATCH 4/5] Eliminate magic #s, follow API --- test/cpp/end2end/async_end2end_test.cc | 85 +++++++------------------- 1 file changed, 21 insertions(+), 64 deletions(-) diff --git a/test/cpp/end2end/async_end2end_test.cc b/test/cpp/end2end/async_end2end_test.cc index 41090d161aa..93d973dc7dd 100644 --- a/test/cpp/end2end/async_end2end_test.cc +++ b/test/cpp/end2end/async_end2end_test.cc @@ -537,33 +537,18 @@ TEST_P(AsyncEnd2endTest, SimpleClientStreamingWithCoalescingApi) { service_->RequestRequestStream(&srv_ctx, &srv_stream, cq_.get(), cq_.get(), tag(2)); + auto verif = Verifier(GetParam().disable_blocking); + verif.Expect(2, true); + cli_stream->Write(send_request, tag(3)); + verif.Expect(3, true); - // 65536(64KB) is the default flow control window size. Should change this - // number when default flow control window size changes. For the write of - // send_request larger than the flow control window size, tag:3 will not come - // up until server read is initiated. For write of send_request smaller than - // the flow control window size, the request can take the free ride with - // initial metadata due to coalescing, thus write tag:3 will come up here. - if (GetParam().message_content.length() < 65536 || GetParam().inproc) { - Verifier(GetParam().disable_blocking) - .Expect(2, true) - .Expect(3, true) - .Verify(cq_.get()); - } else { - Verifier(GetParam().disable_blocking).Expect(2, true).Verify(cq_.get()); + // Drain tag 2, optional to get tag 3 now + while (verif.Next(cq_.get(), false) != 2) { } srv_stream.Read(&recv_request, tag(4)); - - if (GetParam().message_content.length() < 65536 || GetParam().inproc) { - Verifier(GetParam().disable_blocking).Expect(4, true).Verify(cq_.get()); - } else { - Verifier(GetParam().disable_blocking) - .Expect(3, true) - .Expect(4, true) - .Verify(cq_.get()); - } + verif.Expect(4, true).Verify(cq_.get()); EXPECT_EQ(send_request.message(), recv_request.message()); @@ -832,33 +817,19 @@ TEST_P(AsyncEnd2endTest, SimpleBidiStreamingWithCoalescingApiWAF) { service_->RequestBidiStream(&srv_ctx, &srv_stream, cq_.get(), cq_.get(), tag(2)); + auto verif = Verifier(GetParam().disable_blocking); + verif.Expect(2, true); + cli_stream->WriteLast(send_request, WriteOptions(), tag(3)); + verif.Expect(3, true); - // 65536(64KB) is the default flow control window size. Should change this - // number when default flow control window size changes. For the write of - // send_request larger than the flow control window size, tag:3 will not come - // up until server read is initiated. For write of send_request smaller than - // the flow control window size, the request can take the free ride with - // initial metadata due to coalescing, thus write tag:3 will come up here. - if (GetParam().message_content.length() < 65536 || GetParam().inproc) { - Verifier(GetParam().disable_blocking) - .Expect(2, true) - .Expect(3, true) - .Verify(cq_.get()); - } else { - Verifier(GetParam().disable_blocking).Expect(2, true).Verify(cq_.get()); + // Drain tag 2, optional to get tag 3 now + while (verif.Next(cq_.get(), false) != 2) { } srv_stream.Read(&recv_request, tag(4)); + verif.Expect(4, true).Verify(cq_.get()); - if (GetParam().message_content.length() < 65536 || GetParam().inproc) { - Verifier(GetParam().disable_blocking).Expect(4, true).Verify(cq_.get()); - } else { - Verifier(GetParam().disable_blocking) - .Expect(3, true) - .Expect(4, true) - .Verify(cq_.get()); - } EXPECT_EQ(send_request.message(), recv_request.message()); srv_stream.Read(&recv_request, tag(5)); @@ -900,33 +871,19 @@ TEST_P(AsyncEnd2endTest, SimpleBidiStreamingWithCoalescingApiWL) { service_->RequestBidiStream(&srv_ctx, &srv_stream, cq_.get(), cq_.get(), tag(2)); + auto verif = Verifier(GetParam().disable_blocking); + verif.Expect(2, true); + cli_stream->WriteLast(send_request, WriteOptions(), tag(3)); + verif.Expect(3, true); - // 65536(64KB) is the default flow control window size. Should change this - // number when default flow control window size changes. For the write of - // send_request larger than the flow control window size, tag:3 will not come - // up until server read is initiated. For write of send_request smaller than - // the flow control window size, the request can take the free ride with - // initial metadata due to coalescing, thus write tag:3 will come up here. - if (GetParam().message_content.length() < 65536 || GetParam().inproc) { - Verifier(GetParam().disable_blocking) - .Expect(2, true) - .Expect(3, true) - .Verify(cq_.get()); - } else { - Verifier(GetParam().disable_blocking).Expect(2, true).Verify(cq_.get()); + // Drain tag 2, optional to get tag 3 now + while (verif.Next(cq_.get(), false) != 2) { } srv_stream.Read(&recv_request, tag(4)); + verif.Expect(4, true).Verify(cq_.get()); - if (GetParam().message_content.length() < 65536 || GetParam().inproc) { - Verifier(GetParam().disable_blocking).Expect(4, true).Verify(cq_.get()); - } else { - Verifier(GetParam().disable_blocking) - .Expect(3, true) - .Expect(4, true) - .Verify(cq_.get()); - } EXPECT_EQ(send_request.message(), recv_request.message()); srv_stream.Read(&recv_request, tag(5)); From f063f7951f9bc19f2a1667281fe8c15943a546e4 Mon Sep 17 00:00:00 2001 From: Vijay Pai Date: Thu, 5 Oct 2017 15:41:40 -0700 Subject: [PATCH 5/5] Add some const that is now allowed --- test/cpp/end2end/async_end2end_test.cc | 7 ++----- test/cpp/end2end/end2end_test.cc | 5 +---- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/test/cpp/end2end/async_end2end_test.cc b/test/cpp/end2end/async_end2end_test.cc index 41090d161aa..e9a1eebe642 100644 --- a/test/cpp/end2end/async_end2end_test.cc +++ b/test/cpp/end2end/async_end2end_test.cc @@ -223,11 +223,8 @@ class TestScenario { bool disable_blocking; bool inproc; bool health_check_service; - // Although the below grpc::string's are logically const, we can't declare - // them const because of a limitation in the way old compilers (e.g., gcc-4.4) - // manage vector insertion using a copy constructor - grpc::string credentials_type; - grpc::string message_content; + const grpc::string credentials_type; + const grpc::string message_content; }; static std::ostream& operator<<(std::ostream& out, diff --git a/test/cpp/end2end/end2end_test.cc b/test/cpp/end2end/end2end_test.cc index 5dae5b014bd..810ee303f2c 100644 --- a/test/cpp/end2end/end2end_test.cc +++ b/test/cpp/end2end/end2end_test.cc @@ -198,10 +198,7 @@ class TestScenario { void Log() const; bool use_proxy; bool inproc; - // Although the below grpc::string is logically const, we can't declare - // them const because of a limitation in the way old compilers (e.g., gcc-4.4) - // manage vector insertion using a copy constructor - grpc::string credentials_type; + const grpc::string credentials_type; }; static std::ostream& operator<<(std::ostream& out,