1626623 - Using the test_path filter can cause out of memory errors

Status: RESOLVED FIXED

(Tree Management :: Treeherder: Frontend, defect, P2)

Description

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

For the following Treeherder url, the content process grows to ~3 GB and system can become unresponsive: https://treeherder.mozilla.org/#/jobs?repo=autoland&selectedJob=295586068&resultStatus=success%2Ctestfailed%2Cbusted%2Cexception&tochange=c94aa54f1d5e26593a185f32493413c80085a53e&fromchange=572e02c8ba9a69e2b6d33d1b1e12f4f6a2765353&searchStr=10.14%2Cshippable%2Cbrowser-chrome&test_paths=toolkit%2Fcomponents%2Fpictureinpicture%2Ftests%2F

This also got shown below a push: Error on push with revision 03eda5ff313aed36a5e801ad74e6162fdcd21db0: out of memory

Comment 1

[Armen [:armenzg]]

Do you have steps to reproduce? I want to learn how you do this.

How often do you need to use &test_paths when you're looking at multiple revisions?

It's a really big range you're looking at.

Comment 2

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

The STR: Load the url mentioned above and a content process grows to 2.4-2.9GB.

Sheriffs always have TH views with 10+ pushes open except pushes to the Try repository. Every time somebody removes or adds mochitests, the chunk in which they run can change. In addition to that, it might only run every 10th push. So 50 pushes + test_paths filter is a reasonable way to track if a test is failing intermittently or permanently.

Comment 3

[Armen [:armenzg]]

If they want a view to track the behaviour of a test across pushes we might want to use a simpler interface since the current (In reply to Sebastian Hengst [:aryx] (needinfo on intermittent or backout) from comment #2)

The STR: Load the url mentioned above and a content process grows to 2.4-2.9GB.

Would that be a process in Activity Monitor? (On Mac OS X)
For me Firefox Nightly & Firefox NightlyCP Web Content reached 1.2 GB.

Sheriffs always have TH views with 10+ pushes open except pushes to the Try repository. Every time somebody removes or adds mochitests, the chunk in which they run can change. In addition to that, it might only run every 10th push. So 50 pushes + test_paths filter is a reasonable way to track if a test is failing intermittently or permanently.

With the current design, it would require a lot of work trying to reduce the impact of those large artifacts.
We might be able to create a simpler interface to track a test accross pushes with less work (I'm proponent of reducing complexity)

Comment 4

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

(In reply to Armen [:armenzg] from comment #3)

Would that be a process in Activity Monitor? (On Mac OS X)
For me Firefox Nightly & Firefox NightlyCP Web Content reached 1.2 GB.

In Windows Task Manager, it shows 2.5GB for a firefox.exe child process (about:memory?verbose has more detailed data).

Comment 5

[Armen [:armenzg]]

Thank you!

I want to measure the urgency and impact of this bug.
Can they still use Treeherder when they need to filter for a test path and then close that tab and open a fresh one without using test_paths?

Comment 6

[Armen [:armenzg]]

Switching to the API that bug 1636506 will provide might be good enough.
The ultimate change will be returning test_paths as part of the jobs API.

Comment 7

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/6694

Comment 8

[Armen [:armenzg]]

sheriffs: could you please test this out?
https://treeherder-only-test-pa-ff1pl4.herokuapp.com/#/jobs?repo=autoland&test_paths=toolkit%2Fcomponents%2Fpictureinpicture%2Ftests%2F

The performance of test path filtering should be much faster.

Comment 9

[Armen [:armenzg]]

Test cases:

• Sub path - toolkit/components/pictureinpicture/tests
• Full path - toolkit/components/pictureinpicture/tests/browser.ini
• Test file - toolkit/components/pictureinpicture/tests/browser_contextmenu.js
  • This one is the one that will not work

I could cheat the system and make the test file work, however, it would not be a guarantee that the test file actually was an option for a given push.

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

(In reply to Armen [:armenzg] from comment #8)

sheriffs: could you please test this out?
https://treeherder-only-test-pa-ff1pl4.herokuapp.com/#/jobs?repo=autoland&test_paths=toolkit%2Fcomponents%2Fpictureinpicture%2Ftests%2F

This seems broken, tasks don't get filtered. Example.

From the console:

16:33:06.190 Uncaught (in promise) TypeError: e.startsWith is not a function
    wi/</t[r]< Push.jsx:45
    wi Push.jsx:65
    wi Push.jsx:64
    fetchTestManifests Push.jsx:249
Push.jsx:45:11

Comment 11

[Armen [:armenzg]]

I've got the sheriffs to test this.
I've merged it to master and will promote it soon.

This removes the ability to filter by test file (we can add this back in the future), however, it speeds up a lot the filtering and reduces the memory footprint.

We can speed things up a bit more by ingesting the manifests (bug 1654643).

Comment 12

[Armen [:armenzg]]

This is fixed.
More improvements to come in bug 1654643.