Benjamin Smedberg Reporter
	Description • 9 years ago

If a user is disabling multiple addons quickly in a row, or making other changes, we will end up sending very short subsession pings for each change. This is not desirable.

I believe we should throttle environment changes so that they happen at most once every N minutes (suggest N=5). In detail this is how I think it should work:

* when we receive the "first" change notification, immediately reset/start a new subsession.
* Any subsequent change that happens within N minutes should not reset the subsession but should update the environment block of the existing subsession.

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 1 • 9 years ago

Stealing.

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 2 • 9 years ago

This throttles the environment changes to only fire if there was no change in the last 5 minutes.
Most of the patch is test updates.

	Vladan Djeric (:vladan)
	Comment 3 • 9 years ago

Gonna wait for clarifications on the other patch https://bugzilla.mozilla.org/show_bug.cgi?id=1140558#c36

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 4 • 9 years ago

Rebased, part of this stack: https://pastebin.mozilla.org/8827100

	Vladan Djeric (:vladan)
	Comment 5 • 9 years ago

Comment on attachment 8583139 [details] [diff] [review]
Throttle environment changes

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

r+ once the test question is addressed

::: toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js
@@ +937,5 @@
> +  fakeNow(gNow);
> +  Preferences.set(PREF_TEST, 3);
> +  yield deferred.promise;
> +
> +  Assert.equal(changeCount, 2);

will this be 2 because of the throttling happening in _onEnvironmentChange or in _checkForAddonChanges? i.e. which message gets printed to the console

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 6 • 9 years ago

(In reply to Vladan Djeric (:vladan) -- please needinfo! from comment #5)
> ::: toolkit/components/telemetry/tests/unit/test_TelemetryEnvironment.js
> @@ +937,5 @@
> > +  fakeNow(gNow);
> > +  Preferences.set(PREF_TEST, 3);
> > +  yield deferred.promise;
> > +
> > +  Assert.equal(changeCount, 2);
> 
> will this be 2 because of the throttling happening in _onEnvironmentChange
> or in _checkForAddonChanges? i.e. which message gets printed to the console

Because of the throttling in _onEnvironmentChange:
> TelemetryEnvironment::_onEnvironmentChange - throttling changes, now: Mon Feb 01 2010 13:41:00 GMT+0100 (CET), last change: Mon Feb 01 2010 13:40:00 GMT+0100 (CET)

Pref changes are not discarded for pending addon updates, only the addon/plugin/experiment changes go through there.

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 7 • 9 years ago

https://hg.mozilla.org/integration/fx-team/rev/756c80951bbb

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 8 • 9 years ago

Backed out for shutdown crashes:
https://hg.mozilla.org/integration/fx-team/rev/1d92657d70e5

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 9 • 9 years ago

Being handled in bug 1139460.

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 10 • 9 years ago

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/fdf9d0174142

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 11 • 9 years ago

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

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 12 • 9 years ago

See bug 1139460.

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 13 • 9 years ago

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/911d0d7068d6

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 14 • 9 years ago

https://hg.mozilla.org/mozilla-central/rev/911d0d7068d6

	Lawrence Mandel [:lmandel] (use needinfo)
	Comment 15 • 9 years ago

Comment on attachment 8583139 [details] [diff] [review]
Throttle environment changes

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

	Georg Fritzsche [:gfritzsche] Assignee
	Comment 16 • 9 years ago

https://hg.mozilla.org/releases/mozilla-aurora/rev/97b36b7bcf86