MINIFICPP-1281 - Improve test performance by using event polling instead of sleep by sync #835

hunyadi-dev · 2020-07-08T10:53:55Z

Decreases the avg. test execution time by 57% (from 326 seconds to 139 seconds).

Measurements on test times of master (on MacBook Pro 16-inch, 2019):

➜ for repetition in {1..5}; do; time ninja test 2>&1 >> test_logs.txt; done;
ninja test 2>&1 >> test_logs.txt  46.05s user 19.32s system 18% cpu 5:58.81 total
ninja test 2>&1 >> test_logs.txt  45.81s user 13.33s system 18% cpu 5:16.95 total
ninja test 2>&1 >> test_logs.txt  46.10s user 13.22s system 18% cpu 5:18.84 total
ninja test 2>&1 >> test_logs.txt  46.08s user 13.39s system 18% cpu 5:17.21 total
ninja test 2>&1 >> test_logs.txt  46.16s user 13.51s system 18% cpu 5:16.61 total
➜ grep "Total Test time" test_logs.txt
Total Test time (real) = 358.74 sec
Total Test time (real) = 316.88 sec
Total Test time (real) = 318.78 sec
Total Test time (real) = 317.14 sec
Total Test time (real) = 316.57 sec

On the source branch of this PR:

➜ for repetition in {1..5}; do; time ninja test 2>&1 >> test_logs.txt; done;
ninja test 2>&1 >> test_logs.txt  24.04s user 9.62s system 24% cpu 2:19.84 total
ninja test 2>&1 >> test_logs.txt  23.98s user 9.17s system 24% cpu 2:17.74 total
ninja test 2>&1 >> test_logs.txt  23.89s user 9.44s system 24% cpu 2:18.17 total
ninja test 2>&1 >> test_logs.txt  23.82s user 9.53s system 24% cpu 2:18.72 total
ninja test 2>&1 >> test_logs.txt  24.04s user 9.56s system 23% cpu 2:20.81 total
➜ grep "Total Test time" test_logs.txt
Total Test time (real) = 139.77 sec
Total Test time (real) = 137.68 sec
Total Test time (real) = 138.10 sec
Total Test time (real) = 138.66 sec
Total Test time (real) = 140.75 sec

extensions/http-curl/tests/HTTPIntegrationBase.h

extensions/http-curl/tests/VerifyInvokeHTTPTest.cpp

libminifi/test/0-9

libminifi/include/utils/IntegrationTestUtils.h

arpadboda

Nice changes, added some comments.

Not sure if waitToVerifyProcessors should be thrown out, I think waiting for something and then asserting for something else is a valid scenario.

extensions/http-curl/tests/C2FailedUpdateTest.cpp

extensions/http-curl/tests/ControllerServiceIntegrationTests.cpp

extensions/http-curl/tests/HTTPSiteToSiteTests.cpp

extensions/http-curl/tests/HttpGetIntegrationTest.cpp

libminifi/include/utils/IntegrationTestUtils.h

libminifi/test/integration/OnScheduleErrorHandlingTests.cpp

libminifi/include/utils/IntegrationTestUtils.h

extensions/http-curl/tests/C2JstackTest.cpp

extensions/http-curl/tests/C2UpdateTest.cpp

extensions/http-curl/tests/ControllerServiceIntegrationTests.cpp

szaszm · 2020-07-21T09:19:13Z

extensions/http-curl/tests/C2UpdateAgentTest.cpp

@@ -34,6 +34,6 @@ int main(int argc, char **argv) {

  const auto then = std::chrono::system_clock::now();
  const auto seconds = std::chrono::duration_cast<std::chrono::seconds>(then - start).count();
-  assert(handler.calls_ <= (seconds) + 1);
+  assert(handler.calls_ <= (seconds) + 2);


What's the reason for this change?

Some libcurl black magic we do not understand. I could not figure it out but shared the problem with a group of contributors on Slack. I verified the logs and the events happening are what we expect, but curl immediately triggers the POST handle on the Civet Server twice. It was probably an issue beforehand masked by the long waiting time.

One of the CI jobs of @fgerlits had this issue earlier as well: https://travis-ci.org/github/apache/nifi-minifi-cpp/jobs/711468287

Seems like it is indeed the case that we had this error before, but masked it away with the long waiting.

extensions/http-curl/tests/ControllerServiceIntegrationTests.cpp

extensions/http-curl/tests/HTTPSiteToSiteTests.cpp

libminifi/test/integration/OnScheduleErrorHandlingTests.cpp

libminifi/include/utils/IntegrationTestUtils.h

extensions/http-curl/tests/VerifyInvokeHTTPTest.cpp

extensions/http-curl/tests/HTTPSiteToSiteTests.cpp

szaszm

bump: #835 (comment)

szaszm · 2020-07-24T10:29:18Z

extensions/librdkafka/tests/PublishKafkaOnScheduleTests.cpp

+        const int occurrences = result.second;
+        return 1 < occurrences;
+      }));
+      flowController_->updatePropertyValue("kafka", minifi::processors::PublishKafka::MaxMessageSize.getName(), "1999");


What's the reason for the change of MaxMessageSize?

There is no change, it was previously in the waitToVerifyProcessor() on line 57.

At least I hope there is none :)

extensions/standard-processors/tests/integration/TailFileTest.cpp

…e multithreaded tests

…egration tests to poll for events while waiting

…ing in verifyLogLinePresenceInPollTime

…nTests, simplify verifyLogLinePresenceInPollTime

…rationTests

Update comment for verifyLogLinePresenceInPollTime Co-authored-by: Ferenc Gerlits <fgerlits@users.noreply.github.com>

Co-authored-by: Márton Szász <szaszm@users.noreply.github.com>

…HTTPTest

fgerlits reviewed Jul 8, 2020

View reviewed changes

extensions/http-curl/tests/HTTPIntegrationBase.h Show resolved Hide resolved

extensions/http-curl/tests/VerifyInvokeHTTPTest.cpp Show resolved Hide resolved

libminifi/test/0-9 Outdated Show resolved Hide resolved

libminifi/include/utils/IntegrationTestUtils.h Show resolved Hide resolved

hunyadi-dev marked this pull request as draft July 8, 2020 14:06

hunyadi-dev force-pushed the MINIFICPP-1281_caffeinated_tests branch from 4639d8d to e97d5f2 Compare July 8, 2020 15:19

hunyadi-dev marked this pull request as ready for review July 8, 2020 15:22

hunyadi-dev force-pushed the MINIFICPP-1281_caffeinated_tests branch from e97d5f2 to 3b9d6ed Compare July 9, 2020 16:18

fgerlits approved these changes Jul 9, 2020

View reviewed changes

fgerlits mentioned this pull request Jul 10, 2020

MINIFICPP-1183 - Cleanup C2 Update tests #820

Closed

8 tasks

fgerlits reviewed Jul 13, 2020

View reviewed changes

libminifi/include/utils/IntegrationTestUtils.h Outdated Show resolved Hide resolved

arpadboda reviewed Jul 14, 2020

View reviewed changes

hunyadi-dev force-pushed the MINIFICPP-1281_caffeinated_tests branch from 3b9d6ed to 5e3fa22 Compare July 20, 2020 10:36

hunyadi-dev changed the base branch from master to main July 20, 2020 12:24

hunyadi-dev force-pushed the MINIFICPP-1281_caffeinated_tests branch from 5e3fa22 to c566ab5 Compare July 20, 2020 12:27

hunyadi-dev requested a review from arpadboda July 20, 2020 14:07

fgerlits reviewed Jul 21, 2020

View reviewed changes

libminifi/include/utils/IntegrationTestUtils.h Outdated Show resolved Hide resolved

szaszm reviewed Jul 21, 2020

View reviewed changes

hunyadi-dev force-pushed the MINIFICPP-1281_caffeinated_tests branch from c566ab5 to 3a99397 Compare July 21, 2020 13:09

szaszm reviewed Jul 22, 2020

View reviewed changes

hunyadi-dev force-pushed the MINIFICPP-1281_caffeinated_tests branch 4 times, most recently from 6423beb to b6a647f Compare July 28, 2020 07:56

szaszm reviewed Jul 31, 2020

View reviewed changes

szaszm approved these changes Jul 31, 2020

View reviewed changes

adamdebreceni reviewed Jul 31, 2020

View reviewed changes

extensions/standard-processors/tests/integration/TailFileTest.cpp Show resolved Hide resolved

hunyadi-dev added 6 commits July 31, 2020 16:30

MINIFICPP-1281 - Use event polling instead of sleep by sync for simpl…

6f2f033

…e multithreaded tests

MINIFICPP-1281 - Further improve test performance by changing the int…

ad80c60

…egration tests to poll for events while waiting

MINIFICPP-1281 - Decrease sleep and load in MinifiConcurrentQueueTests

e8f9429

MINIFICPP-1281 - Tweak integration test heartbeat periods

01c24fc

MINIFICPP-1281 - Improve comment explaining the parameter pack unpack…

b0b5bf8

…ing in verifyLogLinePresenceInPollTime

MINIFICPP-1281 - Combine lambda checks in ControllerServiceIntegratio…

2a98b1b

…nTests, simplify verifyLogLinePresenceInPollTime

hunyadi-dev and others added 4 commits July 31, 2020 16:33

MINIFICPP-1281 - Remove duplicate license from ControllerServiceInteg…

e97c355

…rationTests

Update libminifi/include/utils/IntegrationTestUtils.h

057a481

Update comment for verifyLogLinePresenceInPollTime Co-authored-by: Ferenc Gerlits <fgerlits@users.noreply.github.com>

Update libminifi/include/utils/IntegrationTestUtils.h

4428d59

Co-authored-by: Márton Szász <szaszm@users.noreply.github.com>

MINIFICPP-1281 - Change 2000 ms notation to 2 seconds in VerifyInvoke…

825c1db

…HTTPTest

hunyadi-dev force-pushed the MINIFICPP-1281_caffeinated_tests branch from b6a647f to 825c1db Compare July 31, 2020 14:34

arpadboda approved these changes Aug 3, 2020

View reviewed changes

arpadboda closed this in 26f9991 Aug 3, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MINIFICPP-1281 - Improve test performance by using event polling instead of sleep by sync #835

MINIFICPP-1281 - Improve test performance by using event polling instead of sleep by sync #835

hunyadi-dev commented Jul 8, 2020

arpadboda left a comment

szaszm Jul 21, 2020

hunyadi-dev Jul 21, 2020

hunyadi-dev Jul 27, 2020

szaszm left a comment

szaszm Jul 24, 2020

hunyadi-dev Jul 31, 2020

hunyadi-dev Jul 31, 2020

MINIFICPP-1281 - Improve test performance by using event polling instead of sleep by sync #835

MINIFICPP-1281 - Improve test performance by using event polling instead of sleep by sync #835

Conversation

hunyadi-dev commented Jul 8, 2020

arpadboda left a comment

Choose a reason for hiding this comment

szaszm Jul 21, 2020

Choose a reason for hiding this comment

hunyadi-dev Jul 21, 2020

Choose a reason for hiding this comment

hunyadi-dev Jul 27, 2020

Choose a reason for hiding this comment

szaszm left a comment

Choose a reason for hiding this comment

szaszm Jul 24, 2020

Choose a reason for hiding this comment

hunyadi-dev Jul 31, 2020

Choose a reason for hiding this comment

hunyadi-dev Jul 31, 2020

Choose a reason for hiding this comment