1121670 - Bug suggestions are not generated for leaks in the "foo process: N bytes leaked" format

Status: RESOLVED FIXED

(Tree Management :: Treeherder: Data Ingestion, defect, P1)

Description

[Andrew McCreight [:mccr8]]

I enabled leak checking for e10s, which revealed an intermittent leak, bug 1121263. Ryan pointed out that for some reason Treeherder isn't providing any starring suggestions for it.

Linux x64 debug Mochitest 3 shows the issue:
  https://treeherder.mozilla.org/ui/#/jobs?repo=mozilla-inbound&revision=38ad6d18d2ce

This is odd, because it is the same logging code we use for default process leaks.  Based on that, my one wild guess is that somehow there's special starring code for matching leaks, and it is looking for "default process" specifically, so when this line has "tab process" it gets confused?  I don't know if that makes sense or not.

13:45:54 WARNING - TEST-UNEXPECTED-FAIL | leakcheck | tab process: 44330 bytes leaked (AsyncLatencyLogger, AsyncTransactionTrackersHolder, BufferRecycleBin, CipherSuiteChangeObserver, CompositableChild, ...)

Comment 1

[Andrew McCreight [:mccr8]]

The code in treeherder-service/treeherder/log_parser/utils.py looks reasonable to me, though the unit test in LEAK_LINE_TEST_CASES needs to be updated to the new format.  So maybe the problem lies elsewhere.

Comment 2

[Ed Morley [:emorley]]

The bug suggestion artefact for the job in question:
https://treeherder.mozilla.org/api/project/mozilla-inbound/artifact/?job_id=5460829&name=Bug+suggestions&type=json

-> cleaned up error line was:
"TEST-UNEXPECTED-FAIL | leakcheck | tab process: 42114 bytes leaked (AsyncLatencyLogger, AsyncTransactionTrackersHolder, AudioOutputObserver, BufferRecycleBin, CipherSuiteChangeObserver, ...)"

log_parser/utils.py then splits the pipe symbol delimiter, takes the third part:
tab process: 42114 bytes leaked (AsyncLatencyLogger, AsyncTransactionTrackersHolder, AudioOutputObserver, BufferRecycleBin, CipherSuiteChangeObserver, ...)

And tries to .match() using:
r'\d+ bytes leaked \((.+)\)$'

In Python, regex .match() only matches from start of line, unlike .search(), ie equivalent to:
r'^\d+ bytes leaked \((.+)\)$'

This kind of issue would be much easier to spot if the bug suggestion artefact also contained the actual search term(s) used - filed bug 1124613.

Comment 3

[Ed Morley [:emorley]]

Attachment: Use search() instead of match() for search term leak regex

Comment 4

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder-service

https://github.com/mozilla/treeherder-service/commit/452af2e45a663acc0c9072079600f6d660985608
Bug 1121670 - Use search() instead of match() for search term leak regex

Since we were missing leak failure messages that were prefixed with the
process name, eg:
"... | leakcheck | tab process: 42114 bytes leaked (...)"

Using .search() is quicker than using .match() with '.*' prefixed to the
regex - see https://bugzilla.mozilla.org/show_bug.cgi?id=1076770#c1

Comment 5

[Ed Morley [:emorley]]

Trivial change + good test coverage - so I've just landed this without review.

Comment 6

[Andrew McCreight [:mccr8]]

Thanks!  I'm still not sure why it was working with "default process" but not "tab process", but oh well.

Comment 7

[Ed Morley [:emorley]]

I don't think it has been - I can't find a single bug that has multiple comments containing "leakcheck | default process:". The only ones I can see are where a new bug was filed, and the bug number added to that job, so treeherder commented on the job manually (rather than via bug suggestions - treeherder now comments for manual cases too).

Comment 8

[Andrew McCreight [:mccr8]]

Oh, wow, that's unfortunate.  Now that you mention it, I did see a lot of intermittent bugs that apparently only happened once...

Comment 9

[Ed Morley [:emorley]]

Yeah I think there may be a few dupes, oops! 

Unfortunately I believe the testcases were derived before bug 1069689 changed the default process case to have a prefix too, which is why this was missed.

Comment 10

[Ed Morley [:emorley]]

In production :-)