1459920 - Rollout applies again on graduated preferences

Status: RESOLVED INVALID

(Firefox :: Normandy Client, enhancement)

Description

[Adrian Florinescu [:aflorinescu]]

[Description:]
Although this is a scenario that can be categorised in the area "we do not support downgrade", it's very likely to happen in the wild once we hit graduation scenarios, after which the profile is used on a channel where the graduated roll-out preference is not default yet. 
 

[reduced Steps:]
1. Rollout on prefx for Version_0.
2. Upgrade to Version_1 which makes prefx default -> rollout graduates.
3. Back on Version_0 with the same profile, the rollout is executed and the results is that the preference targeted by rollout is updated to starting point, before the rollout, after which on the next restart for the Version_0, the rollout is applied again.
4. Restart Version_0.

[Actual Result:]
3.
1525779732500	app.normandy.recipe-runner	INFO	Executing recipe "Rollout exp" (action=preference-rollout)
1525779732505	app.normandy.action.PreferenceRolloutAction	DEBUG	updating rollout-graduation1: test.int.1 previous value changed from 1 to null
1525779732506	app.normandy.action.PreferenceRolloutAction	DEBUG	updating rollout-graduation1: test.char.1 previous value changed from one to null
1525779732506	app.normandy.action.PreferenceRolloutAction	DEBUG	updating rollout-graduation1: test.bool.1 previous value changed from false to null
1525779732507	app.normandy.action.PreferenceRolloutAction	DEBUG	Ungraduated preference rollout rollout-graduation1

4. After restart, the rollout is applied again on Version_0.  

[Expected Result:]
The expected Result here is questionable:
A?
for step2 -
app.normandy.recipe-runner	INFO	Executing recipe "Rollout exp" (action=preference-rollout)
app.normandy.action.PreferenceRolloutAction	DEBUG	No updates to preference rollout rollout-graduation1

Even if its graduated, the rollout still executes, which means it's possible for it to get updated therefore the rollout to get in a "ungraduated" state.

B?
for step3 -  in a sense the results are correct, the Version_0 defaults for all the preferences should be NULL, but this should be handled by Firefox default preferences logic and not by Normandy in this case, since the rollout should be graduated. (should this be fixed by bug 1459899)

C?
for step4 - in a sense this is correct as well, since for Version_0 the rollout is eligible again, since for Version_0 the preferences in this example do not exists.

Comment 1

[Michael Cooper [:mythmon]]

One of the goals of preference rollout is that the state of the recipes on the server should always be asserted on the matching clients. Because of this, graduation is explicitly a temporary state: if the state of the browser stops matching the graduated state, the recipe will become active again. This afects both changing the recipe on the server, and also downgrades.

In other words, this is working as expected.

Comment 2

[Adrian Florinescu [:aflorinescu]]

I am considering reopening this bug - or maybe open a new issue due to the following reason/scenario:

A. version x runs Rollout 
B. version x updates to version x+1 which graduates rollout (preference/s rollout values are now default)
 
Actual Result:
I. all rollouts started in version x graduate (as expected) in version x+1.
II. additionally there are a few instances - for example all new profiles created in version x+1 which will enroll in the rollout and immediately afterwards will get graduated, which generate additional telemetry - which is both valid and invalid in the same time:
Although the above functions within the Normandy Rollout/Rollback architecture, I'm not sure how relevant the version+1 graduation data shall be or how easy to exclude the new profiles for version+1 enrolls/graduates for an actual analysis (since in the above scenario we will have enrolls/graduation for new profiles in version x+1 which is not relevant data from my point of view. 

I think we should at least consider changing the "graduation" status name because it might be misleading for what the status actually means. (comment 1)  

Note:
Similar behaviour is found in the case of roll-back, due Normandy running recipes in the recipe_id's order: the rollback shall be executed first and then the rollout, which in case of a new profile, the rollback will fail first run, then the user will enroll then at next run it will get rolled back. - this is not a concerning scenario since it would be expected that rollouts that need rollback will obviously get disabled first.

Mike, thoughts?

Comment 3

[Michael Cooper [:mythmon]]

Based on this bug, I think I've synthesized a few specific problems:

1) Rollouts which would have no affect shouldn't enroll. They should either fail to enroll, or silently do nothing. I'm leaning towards sending an enrolledFailed event for this with reason="would-be-noop", so that we can see the "problem" in telemetry. They wouldn't produce any further events (like graduation) though.

2) Rollouts and rollbacks should not both be active at the same time.

3) Rollbacks who's corresponding rollout never applied to a client should be considered ok, and not generate any telemetry.

#1 and #3 are changes we can make to the client. #2 is something we can enforce on the server. I'll file the appropriate bugs for this. Do the above changes help with your concerns?

Comment 4

[Michael Cooper [:mythmon]]

From comment 3, I've filed bug 1467558 for problem #1, bug 1467563 for problem #3, and Normandy issue #1400 [0] for problem #2.

[0]: https://github.com/mozilla/normandy/issues/1400