1629929 - Personality Provider isn't properly rehydrating itself with cached values

Status: VERIFIED FIXED

(Firefox :: New Tab Page, defect, P1)

Description

[Scott [:thecount] Downe]

If we re initialize a personality provider using cache, it's not properly passing the previous cached score into the new insistence, causing it to make them fresh which is not really ideal.

Comment 1

[Scott [:thecount] Downe]

[Tracking Requested - why for this release]: Should be a small fix.

It's adding back code that got lost in a refactor, so it's not completely new untested code.

It's also behind a pref.

The impact is to a study we want to run in 76, which might slow down the frequency in which we get the proper personalization, impacting the results of the experiment.

Comment 2

[Ryan VanderMeulen [:RyanVM]]

It's not clear to me that there's any major user-facing impact from this bug, so no need to track it for 76/77 AFAICT. That said, I'm still fine accepting a low-risk patch for uplift once one is ready and nominated for approval.

Comment 3

[Scott [:thecount] Downe]

Attachment: Bug 1629929 - Ensure we use cached scores if available in v2

Comment 4

[Scott [:thecount] Downe]

Steps to test.

Mostly this is regression testing. I would do the steps outlined here: https://bugzilla.mozilla.org/show_bug.cgi?id=1544922#ch-2

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f3fb7a863e92
Ensure we use cached scores if available in v2 r=gvn

Comment 6

[Scott [:thecount] Downe]

Comment on attachment 9140540 [details]
Bug 1629929 - Ensure we use cached scores if available in v2

Beta/Release Uplift Approval Request

• User impact if declined: Performance issues for an experiment.

Not huge, but without this the study results wouldn't be as accurate.

• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Steps to test.

Mostly this is regression testing. I would do the steps outlined here: https://bugzilla.mozilla.org/show_bug.cgi?id=1544922#ch-2

• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Adding back a few lines of code that were removed during a refactor. It's a regression.

Previous code was already doing this and working fine.

It's also a small three line patch.

preffed off, for a 1% experiment

• String changes made/needed:

Comment 7

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/f3fb7a863e92

Comment 8

[Marius Coman [:mcoman], Ecosystem QA]

I have verified that this issue is no longer reproducible with the latest Firefox Nightly (77.0a1 Build ID - 20200421094220) installed, on Windows 10 x64, Ubuntu 18.04 x64 and Mac 10.15.3, using the steps from here. I can confirm the values from the "spoc" data score are different from the one that is displayed for the "item_score".

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9140540 [details]
Bug 1629929 - Ensure we use cached scores if available in v2

Fixes a possible leak. Approved for 76.0b8.

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/f3735ccb9fb9

Comment 11

[Marius Coman [:mcoman], Ecosystem QA]

I have verified that this issue is no longer reproducible with the latest Firefox Beta (76.0b8 Build ID - 20200424000239) installed, on Windows 10 x64, Ubuntu 18.04 x64 and Mac 10.15.3, using the steps from here. I can confirm the values from the "spoc" data score are different from the one that is displayed for the "item_score".