1143796 - Increasing TelemetryScheduler ticking interval when user is not active

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect)

Description

[Alessio Placitelli [:Dexter]]

TelemetryScheduler currently ticks every 5 minutes: this interval must be bumped to an hour when the user is not active.

Comment 1

[Alessio Placitelli [:Dexter]]

Attachment: part 1 - Enable idle detection in TelemetryScheduler.

This patch allows TelemetryScheduler to understand when user is idle and changes the tick time accordingly.

Comment 2

[Alessio Placitelli [:Dexter]]

Attachment: part 2 - Add test coverage.

Adds test coverage.

Comment 3

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8581766 [details] [diff] [review]
part 1 - Enable idle detection in TelemetryScheduler.

Review of attachment 8581766 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/telemetry/TelemetrySession.jsm
@@ +418,5 @@
>  
>    // The timer which drives the scheduler.
>    _schedulerTimer: null,
> +  // The interval used by the scheduler timer.
> +  _schedulerInterval: 0,

Lets just track the idle status in, say, _userIsIdle, and pass the right interval below accordingly.

Comment 4

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8581767 [details] [diff] [review]
part 2 - Add test coverage.

Review of attachment 8581767 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/telemetry/tests/unit/test_TelemetrySession.js
@@ +1526,5 @@
> +  // When not idle, the scheduler should have a 5 minutes tick interval.
> +  Assert.equal(schedulerTimeout, SCHEDULER_TICK_INTERVAL_MS);
> +
> +  // Send an "idle" notification to the scheduler.
> +  fakeIdleNotification("idle");

This can probably use a little less documenting:
// When idle, ...
fakeIdle...
Assert.equal(...

Dito below.

Comment 5

[Georg Fritzsche [:gfritzsche]]

(In reply to Georg Fritzsche [:gfritzsche] from comment #3)
> Comment on attachment 8581766 [details] [diff] [review]
> part 1 - Enable idle detection in TelemetryScheduler.
> 
> Review of attachment 8581766 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: toolkit/components/telemetry/TelemetrySession.jsm
> @@ +418,5 @@
> >  
> >    // The timer which drives the scheduler.
> >    _schedulerTimer: null,
> > +  // The interval used by the scheduler timer.
> > +  _schedulerInterval: 0,
> 
> Lets just track the idle status in, say, _userIsIdle, and pass the right
> interval below accordingly.

Nevermind, i'll do that in bug 1139751.

Comment 6

[Alessio Placitelli [:Dexter]]

Attachment: part 2 - Add test coverage. v2

Comment 7

[Vladan Djeric (:vladan)]

Comment on attachment 8581766 [details] [diff] [review]
part 1 - Enable idle detection in TelemetryScheduler.

Review of attachment 8581766 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/telemetry/TelemetrySession.jsm
@@ +512,5 @@
> +    switch(aTopic) {
> +      case "idle":
> +        // If the user is idle, increase the tick interval.
> +        this._schedulerInterval = SCHEDULER_TICK_IDLE_INTERVAL_MS;
> +        this._rescheduleTimeout();

you should take into account midnight for daily pings.. you don't want to miss midnight by 1hr+

Comment 8

[Alessio Placitelli [:Dexter]]

(In reply to Vladan Djeric (:vladan) -- please needinfo! from comment #7)
> you should take into account midnight for daily pings.. you don't want to
> miss midnight by 1hr+

This work is done in bug 1139751

Comment 9

[Georg Fritzsche [:gfritzsche]]

https://hg.mozilla.org/integration/fx-team/rev/f8ae18a7d017
https://hg.mozilla.org/integration/fx-team/rev/208c445781e7

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out for shutdown crashes:
https://hg.mozilla.org/integration/fx-team/rev/f72e41028cf0
https://hg.mozilla.org/integration/fx-team/rev/0088f4d79422

Comment 11

[Alessio Placitelli [:Dexter]]

Clearing the ni?, as bug 1139460 is the root cause for the shutdown crashes.

Comment 12

[Georg Fritzsche [:gfritzsche]]

try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=256df1f0f2f6
fxteam push: https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=d2567d89cda3

https://hg.mozilla.org/integration/fx-team/rev/7a6f6bdd6edf
https://hg.mozilla.org/integration/fx-team/rev/613fd260f646

Comment 13

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

This and everything else from that push is backed out in https://hg.mozilla.org/integration/fx-team/rev/6901a267f856 for Windows PGO XPCShell failures:

https://treeherder.mozilla.org/logviewer.html#?job_id=2520751&repo=fx-team

Comment 14

[Alessio Placitelli [:Dexter]]

Clearing n?, as bug 1140558 is the root cause for this issue (see bug 1140558 comment 65).

Comment 15

[Georg Fritzsche [:gfritzsche]]

fx-team push: https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=cda9f6c087a6
pgo try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=9cb8ce2e0436
previous full try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=256df1f0f2f6

https://hg.mozilla.org/integration/fx-team/rev/ad335ce95140
https://hg.mozilla.org/integration/fx-team/rev/c1a2d8973085

Comment 16

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ad335ce95140
https://hg.mozilla.org/mozilla-central/rev/c1a2d8973085

Comment 17

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8581766 [details] [diff] [review]
part 1 - Enable idle detection in TelemetryScheduler.

Approved for Aurora. For approval request see bug 1139460 comment 42. For approval comments see bug 1139460 comment 43.

Comment 18

[Georg Fritzsche [:gfritzsche]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/25a86f3c31e1
https://hg.mozilla.org/releases/mozilla-aurora/rev/c587374a3ba9