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

Command icon 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

catch created an issue. See original summary.

catch’s picture

Title: Split up BigPipeRegressionTest » Speed up BigPipeRegressionTest

OK 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.

catch’s picture

37s 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

catch’s picture

1000 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

catch’s picture

Status: Active » Needs review

And 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.

catch’s picture

Hopefully last pipeline run gives:

35.432s Drupal\Tests\big_pipe\FunctionalJavascript\BigPipeRegressionTest 1 passed

catch’s picture

@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.

catch’s picture

Issue summary: View changes
nicxvan’s picture

Status: Needs review » Reviewed & tested by the community

Yeah 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.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed cf604c3 and pushed to main. Thanks!

Backport to 11.x wasn't clean so leaving as main only.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

  • alexpott committed cf604c3a on main
    task: #3590076 Speed up BigPipeRegressionTest
    
    By: catch
    

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.