1555101 - Intermittent toolkit/components/telemetry/tests/browser/browser_TelemetryGC.js | evict_nursery_for_major_gc is in totals -

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect, P5)

Description

[Treeherder Bug Filer]

Filed by: aciure [at] mozilla.com
Parsed log: https://treeherder.mozilla.org/logviewer.html#?job_id=248760358&repo=mozilla-inbound
Full log: https://queue.taskcluster.net/v1/task/LWEWLpFFQM-_5nwLQpRFSA/runs/0/artifacts/public/logs/live_backing.log

---

[task 2019-05-28T16:37:03.203Z] 16:37:03 INFO - TEST-PASS | toolkit/components/telemetry/tests/browser/browser_TelemetryGC.js | decommit property should be a number -
[task 2019-05-28T16:37:03.203Z] 16:37:03 INFO - Buffered messages finished
[task 2019-05-28T16:37:03.208Z] 16:37:03 INFO - TEST-UNEXPECTED-FAIL | toolkit/components/telemetry/tests/browser/browser_TelemetryGC.js | evict_nursery_for_major_gc is in totals -
[task 2019-05-28T16:37:03.208Z] 16:37:03 INFO - Stack trace:
[task 2019-05-28T16:37:03.208Z] 16:37:03 INFO - chrome://mochikit/content/browser-test.js:test_ok:1314
[task 2019-05-28T16:37:03.208Z] 16:37:03 INFO - chrome://mochitests/content/browser/toolkit/components/telemetry/tests/browser/browser_TelemetryGC.js:check:95
[task 2019-05-28T16:37:03.208Z] 16:37:03 INFO - chrome://mochitests/content/browser/toolkit/components/telemetry/tests/browser/browser_TelemetryGC.js:test:180
[task 2019-05-28T16:37:03.209Z] 16:37:03 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1116
[task 2019-05-28T16:37:03.209Z] 16:37:03 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1144
[task 2019-05-28T16:37:03.209Z] 16:37:03 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:1005
[task 2019-05-28T16:37:03.209Z] 16:37:03 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:803
[task 2019-05-28T16:37:03.209Z] 16:37:03 INFO - Not taking screenshot here: see the one that was previously logged
[task 2019-05-28T16:37:03.213Z] 16:37:03 INFO - TEST-UNEXPECTED-FAIL | toolkit/components/telemetry/tests/browser/browser_TelemetryGC.js | evict_nursery_for_major_gc.mark_roots is in totals -
[task 2019-05-28T16:37:03.214Z] 16:37:03 INFO - Stack trace:
[task 2019-05-28T16:37:03.214Z] 16:37:03 INFO - chrome://mochikit/content/browser-test.js:test_ok:1314
[task 2019-05-28T16:37:03.214Z] 16:37:03 INFO - chrome://mochitests/content/browser/toolkit/components/telemetry/tests/browser/browser_TelemetryGC.js:check:95
[task 2019-05-28T16:37:03.214Z] 16:37:03 INFO - chrome://mochitests/content/browser/toolkit/components/telemetry/tests/browser/browser_TelemetryGC.js:test:180
[task 2019-05-28T16:37:03.214Z] 16:37:03 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest/<:1116
[task 2019-05-28T16:37:03.214Z] 16:37:03 INFO - chrome://mochikit/content/browser-test.js:Tester_execTest:1144
[task 2019-05-28T16:37:03.214Z] 16:37:03 INFO - chrome://mochikit/content/browser-test.js:nextTest/<:1005
[task 2019-05-28T16:37:03.214Z] 16:37:03 INFO - chrome://mochikit/content/tests/SimpleTest/SimpleTest.js:SimpleTest.waitForFocus/waitForFocusInner/focusedOrLoaded/<:803
[task 2019-05-28T16:37:03.215Z] 16:37:03 INFO - Not taking screenshot here: see the one that was previously logged

Comment 1

[Chris H-C :chutten]

I don't know much about this test file. pbone, would you know something about this?

Comment 2

[Paul Bone [:pbone]]

It just needs an update since we've been adding more phases.

Chris, please read on because I have a question.

Currently the telemetry code & this test enforce a maximum of 65 phases. (each one is a JSON field in terms of how much data we're sending). However there are currently 73 different phases defined, some some GCs may exceed this limit of 65, and the telemetry code will delete all the phase data rather than send too much data.

Chris, should we increase this limit to 73 and maybe continue to increase it as we add GC phases, or should we relax this test so that it will allow the telemetry code to delete all the phase data rather than send too much.

Thanks.

Comment 3

[Chris H-C :chutten]

If we relax the test, I would argue that we then need to ensure there's a test ensuring that the Telemetry code deletes the data as expected.

I'm not opposed to increasing it to 73, so we could take that route as well. What would be the delta on the size of the structure if we did? How frequently do you think we'll hit it? (double-checking that we won't hit a ping size budget)

Comment 4

[Paul Bone [:pbone]]

Each of these fields is its label (a string) plus an integer. They're not very big and most GCs won't include all phases.

What I'm worried about is today increasing it to 73, but next year it increases to 300 or whatever, that we don't at least think about it when we add more telemetry data. So I guess 73 is okay and if someone like yourself comes to tell me when it's 300 (yeah, that's exaggerated) then we can fix it.

Comment 5

[Paul Bone [:pbone]]

Attachment: Bug 1555101 - Raise the phase limit for GC telemetry to 73 r=chutten,jonco

Comment 6

[Intermittent Failures Robot]

2 failures in 4337 pushes (0.0 failures/push) were associated with this bug in the last 7 days.

Repository breakdown:

• mozilla-inbound: 1
• mozilla-central: 1

Platform breakdown:

• linux64: 2

For more details, see:
https://treeherder.mozilla.org/intermittent-failures.html#/bugdetails?bug=1555101&startday=2019-05-27&endday=2019-06-02&tree=all

Comment 7

[Paul Bone [:pbone]]

Now waiting on https://github.com/mozilla-services/mozilla-pipeline-schemas/pull/339 to land and be live before the patch can be merged.

Comment 8

[Paul Bone [:pbone]]

My patch included an off-by-one error. I've fixed it. Phases are 0-based, numbered 0-72, there are 73 of them.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/69e31119cd48
Raise the phase limit for GC telemetry to 73 r=chutten,jonco

Comment 10

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/69e31119cd48