Problem/Motivation
BigPipeRegressionTest takes 30-45s on my local, but sometimes over 3m on gitlab pipelines.
This is due to the test coverage for multiple (e.g. 2000) placeholder replacement, which had to have that many placeholders to trigger a timing race condition.
Investigation in this issue shows that even when reverting the original fix, the test does not fail with 2000 placeholders, and when restoring the number of placeholders to the pre-w3c testing 3000, it times out in selenium (the reason we had to reduce it from 3000 to 2000).
This issue therefore skips that test, with a follow-up to replace it, or delete it if it really is impossible to test effectively.
While investigating this, noticed that a couple of test methods require different modules to the rest of the test, so split them to their own test classes.
Steps to reproduce
Proposed resolution
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
Issue fork drupal-3590076
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #3
catchOK did some more investigation and nearly the entire time is spent in the multiple placeholders method, which was added in #3390178: big_pipe sometimes fails to load blocks .
We reduced the placeholder count from 3000 to 2000 in #3421202: Enable W3C-compliant webdriver testing probably because it exploded with the driver change.
Going to try reducing the count to 300 which was discussed in the original issue. The worst that can happen is the test doesn't catch the bug, the second worst that can happen is that if we regress it turns into a random test failure.
Another possible option would be to play with higher CPU request limits for functional js tests, but it really seems wasteful to be cranking chrome for 170s for every commit to every Drupal core MR.
Comment #4
catch37s on https://git.drupalcode.org/project/drupal/-/jobs/9810841 with independent test methods.
13s 13.032s Drupal\Tests\big_pipe\FunctionalJavascript\BigPipeRegressionTest 1 passed with the existing protected methods solution.
Now I guess let's find out if there's a tipping point with number of placeholders
Comment #5
catch1000 placeholders
- 20.635s Drupal\Tests\big_pipe\FunctionalJavascript\BigPipeRegressionTest 1 passed
https://git.drupalcode.org/issue/drupal-3590076/-/jobs/9811334
1250 placeholders:
33.560s Drupal\Tests\big_pipe\FunctionalJavascript\BigPipeRegressionTest 1 passed
https://git.drupalcode.org/issue/drupal-3590076/-/jobs/9812364
1375 placeholders:
40.081s Drupal\Tests\big_pipe\FunctionalJavascript\BigPipeRegressionTest 1 passed
https://git.drupalcode.org/issue/drupal-3590076/-/jobs/9812527
1425 placeholders:
50.356s Drupal\Tests\big_pipe\FunctionalJavascript\BigPipeRegressionTest 1 passed
https://git.drupalcode.org/issue/drupal-3590076/-/jobs/9812604
1475 placeholders:
35.922s Drupal\Tests\big_pipe\FunctionalJavascript\BigPipeRegressionTest 1 passed
(yes this was quicker than 1425, probably different AWS host)
1500 placeholders -
131.149s Drupal\Tests\big_pipe\FunctionalJavascript\BigPipeRegressionTest 1 passed
53.837s testPlaceholderHtmlEdgeCases
2000 placeholders - 140s
https://git.drupalcode.org/issue/drupal-3590076/-/jobs/9811259
86.666s Drupal\Tests\big_pipe\FunctionalJavascript\BigPipeRegressionTest 1 passed
https://git.drupalcode.org/issue/drupal-3590076/-/jobs/9813751
Comment #6
catchAnd back to 1250 again:
30.533s Drupal\Tests\big_pipe\FunctionalJavascript\BigPipeRegressionTest 1 passed
https://drupal.slack.com/archives/CGKLP028K/p1778771277164539
What I think is happening:
Some jobs handle 2000 placeholders fine - still under 40s, and some shoot through the roof to 180s.
Some pods are more resource constrained than others, this can be either AWS instance type, or other pods sharing the same hardware and also doing expensive things, or both at the same time.
It looks like 1250 keeps this within a reasonable constraint while still counting as a very large number of big pipe placeholders.
Comment #7
catchHopefully last pipeline run gives:
35.432s Drupal\Tests\big_pipe\FunctionalJavascript\BigPipeRegressionTest 1 passed
Comment #8
catch@nicxvan had the good idea to try reverting the original fix. With 2000 placeholders the test still passes, with 3000 placeholders there is a selenium timeout. So afaict there is no way to make the test fail correctly and reliably at the moment, which was to at least some extent the case when it went in but w3c lowered the tolerance for how evil we can make the test. So... I opened #3590200: Re-enable Big pipe multiple replacements test and let's skip it here - we're locking up 180s of CPU every core pipeline run for a test that will never fail the way we want it to.
Comment #9
catchComment #10
nicxvan commentedYeah I think this is worth skipping the test.
After the w3c limitation stopped allowing 3000 iterations the test wasn't working anyway.
This will save significant cpu time each month at least until we can find a proper way to test it.
Comment #11
alexpottCommitted cf604c3 and pushed to main. Thanks!
Backport to 11.x wasn't clean so leaving as main only.