Closed Bug 1203804 Opened 9 years ago Closed 9 years ago

Permaorange browser_perf-states.js | leaked 2 window(s) until shutdown [url = http://example.com/browser/browser/devtools/performance/test/doc_simple-test.html] after merge from m-c to m-i

Categories

(DevTools :: General, defect)

defect
Not set
blocker

Tracking

(firefox43 affected)

RESOLVED DUPLICATE of bug 1203888
Tracking Status
firefox43 --- affected

People

(Reporter: philor, Unassigned)

References

Details

Something in https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=0a18e5db95a4 disagrees with something in the range between the last thing on mozilla-inbound that is already merged to mozilla-central and the point on mozilla-inbound when that was merged in, https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=c75f9ca74a29&tochange=68ca35b8034b, so starting at the merge from m-c we have a permaleak in browser_perf-states.js and I don't have any idea where to start looking for things to back out either from the merge in or from the range of m-i that isn't merged around yet. mozilla-inbound is closed.
Depends on: 1171488
Depends on: 861335
No longer depends on: 1171488
Fixed by backing out the patch from bug 861335
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
(In reply to Phil Ringnalda (:philor) from comment #1) > Fixed by backing out the patch from bug 861335 wat
That didn't help either. I mean it helped, so now we've got the same bustage on different dt tests. I've backed out 28f1e57d4757 (bug 1171488) to see if that'll help. It appears those two bugs aren't working well together.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Resolution: --- → DUPLICATE
(In reply to Victor Porof [:vporof][:vp] from comment #2) > (In reply to Phil Ringnalda (:philor) from comment #1) > > Fixed by backing out the patch from bug 861335 > > wat That requires a fair bit of background. SETA, the Search for Extraneous Testing in Automation, is a joint ateam/releng project to completely ignore that saying about "past performance does not predict future results" and to instead say, quite literally, "well, we've never backed out a patch for failing _only_ in Linux32 debug dt2, so there's no need to run that chunk on that OS on debug more than once every seven pushes." So, that's fact one: unless you constantly remember, every single time that you see a chunk with a failure, that there is absolutely no reason to think that that particular chunk on that OS ran on any of the previous six pushes, you'll make mistakes. devtools-chrome runs chunk-by-dir, which is intended to keep things a bit more stable in terms of what test runs in what chunk, but it runs in a different number of chunks for opt versus debug and a different number of chunks on each OS, and in some cases the number of tests per chunk is relatively tiny, so a patch like yours, which removes six test files from a directory both alphabetically and in code far removed from performance/ can nevertheless cause the entire performance/tests/ directory to move from, say, dt3 which runs every push on Linux32 debug to dt2 which runs every seventh push on Linux32 debug. So if you see a new permaorange which clearly starts on a particular push, if it's in a chunked test suite you have to remember always, every single time, to look at whether or not that test ran in that chunk on the previous push. Then there's disabled tests. There were actually two permaleaking tests, browser_perf-recording-selected-04.js which was already disabled on Linux and thus we only got to see it leaking on Mac and Windows, and browser_perf-states.js which was leaking on Linux. Or maybe it was leaking on all three, but we never ran the chunk which includes it on Mac and Windows, dunno. I do understand the basis of your "wat," because to you there is absolutely no way that your patch could have had any effect on leaking in performance tests. We could have sheriffs who would know that, in one of two ways: we could go back to the days before I became the sheriff, when every developer was required to watch the tree until every single test suite had run green on their push or on a later push if it got skipped on their push, or, we could have bz, dbaron and gavin be the three sheriffs. I can't think of anyone else who has both the breadth and depth of understanding of our codebase to in every case say whether or not a particular patch could have caused a particular failure. And then there's the personal and particular reason I screwed up: I took the day off sick yesterday, which let me cover the time when we now have no sheriff coverage because MoCo isn't willing to invest in replacing the person who did the most sheriffing now that he is gone, and then I just kept going after that since I was free and could, so by the time I backed you out incorrectly, I had been working for free for 14 straight hours, on a day when I was too sick to work for pay. Sorry, I fucked it up, in a way that neither another volunteer sheriff nor two paid sheriffs nor you, since you left one permaorange enabled while disabling the other, and thus got yourself backed out wrongly yet again, realized.
This was an awesome explanation of what happened in this case, which has been puzzling me since last week, thanks philor! And we probably have different definitions for "screwed up", as it sounds to me that a particularly complicated set of problems were dealt with in a reasonable timeframe, even in the face of severe adverse events and external complications, so kudos!
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.