qa: Remove polling loop from test_runner #13384

1. 
   MarcoFalke commented at 6:24 pm on June 3, 2018: member

   My initial design for parallel tests was a busy loop (busy waiting) that polls the running processes every half a second.

   Using pythons own ThreadPoolExecutor makes the code smaller and hopefully easier to read/understand. By removing the half-a-second overhead, I believe the tests run now slightly faster (Verified on my workstation, but should also be verified by one of the reviewers, ideally)

   This is refactoring beside the following changes:

   • Dropped the dot-printing (….) while waiting for the next result
   • --failfast will now wait for unfinished jobs (similar to a failure in make)
   • The portseed_offset is no longer applied. This is no longer required since we already kill leftover processes (see #12904). This “fixes” #10869 because we deterministically pick ports starting at 11000
2. MarcoFalke added the label Tests on Jun 3, 2018
3. MarcoFalke force-pushed on Jun 3, 2018
4. MarcoFalke force-pushed on Jun 3, 2018
5. qa: Remove polling loop from test_runner fa35a022e9
6. MarcoFalke force-pushed on Jun 3, 2018
7. 
   laanwj commented at 12:21 pm on June 4, 2018: member

   I don’t see difference with regard to run time:

    0with
    1real    3m54.986s
    2real    3m56.017s
    3real    4m0.519s
    4real    3m45.475s
    5real    3m57.155s
    6
    7without
    8real    3m51.195s
    9real    3m54.101s
   10real    3m53.719s
   11real    4m1.334s
   12real    3m53.727s
   

   However I like the code cleanup and it doesn’t make it worse so utACK fa35a022e929948ee13b76093ab5c9907f3c4209

8. 
   laanwj commented at 11:13 am on June 7, 2018: member

   Seems that with this change, Ctrl-C does no longer quit the test runner immediately when the tests have started, looks like it has to be pressed twice.

   Dropped the dot-printing (….) while waiting for the next result

   I suddenly remember that this was added to prevent Travis from timing out due to no output. Is that no longer a concern?

9. 
   MarcoFalke commented at 12:59 pm on June 7, 2018: member

   I suddenly remember that this was added to prevent Travis from timing out due to no output. Is that no longer a concern?

   The run time is between 2 and 3 minutes. And travis would only time out after 10 minutes of no dots received. The extended tests are not run on travis for a couple of weeks now, so I can’t answer that…

10. MarcoFalke closed this on Jun 7, 2018

    ---

11. MarcoFalke deleted the branch on Jun 7, 2018
12. 
    MarcoFalke commented at 3:32 pm on June 7, 2018: member

    Closing for now. Not really worth the effort
13. 
    jnewbery commented at 5:24 pm on June 8, 2018: member

    Concept ACK. I’d considered replacing our custom-made process pool with multiprocessing.pool.Pool() previously.

    Even if you don’t include that change, it seems like some of the tidy-up in this PR is useful (eg the port_offset stuff)

14. MarcoFalke locked this on Sep 8, 2021

Contributors
MarcoFalke laanwj jnewbery

Labels
Tests