Closed Bug 1262312 Opened 4 years ago Closed 4 years ago

A device update POST is made every time about:preferences:#sync is shown

Categories

(Firefox :: Sync, defect, P1)

defect

Tracking

()

RESOLVED FIXED
Firefox 48
Tracking Status
firefox46 blocking fixed
firefox47 + fixed
firefox48 --- fixed
firefox-esr45 --- unaffected

People

(Reporter: markh, Assigned: lina)

References

Details

Attachments

(1 file)

Every time soomething requests the name of the Sync device, a post is made to update the device name, even though the device name is not changed. This is due to a subtle implementation detail - the localName getter at https://dxr.mozilla.org/mozilla-central/rev/d9f50aa0a1aaf90499b85c31e0f329b762e80fdd/services/sync/modules/engines/clients.js#128 always calls the setter - even when the value is the same - and the setter causes us to update the device registration.
Flags: firefox-backlog+
Duplicate of this bug: 1262303
Assignee: nobody → kcambridge
Status: NEW → ASSIGNED
Comment on attachment 8738331 [details]
MozReview Request: Bug 1262312 - Don't update the device registration every time we open Sync preferences. r?markh

https://reviewboard.mozilla.org/r/44461/#review41201
Attachment #8738331 - Flags: review?(markh) → review+
https://hg.mozilla.org/mozilla-central/rev/43f067177f2d
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Comment on attachment 8738331 [details]
MozReview Request: Bug 1262312 - Don't update the device registration every time we open Sync preferences. r?markh

Approval Request Comment
[Feature/regressing bug #]: Bug 1227527.
[User impact if declined]: High load on the FxA servers every time the user syncs or opens Sync prefs.
[Describe test coverage new/current, TreeHerder]: Covered by existing unit tests.
[Risks and why]: Low risk. This change is limited in scope to the Sync clients engine, and rode the train to Aurora.
[String/UUID change made/needed]: None.
Attachment #8738331 - Flags: approval-mozilla-beta?
Marking this blocking for 46. We are holding back this morning from unthrottling 46. See bug 1268652.
Comment on attachment 8738331 [details]
MozReview Request: Bug 1262312 - Don't update the device registration every time we open Sync preferences. r?markh

Requesting approval for release; same rationale as comment 6.
Attachment #8738331 - Flags: approval-mozilla-release?
Comment on attachment 8738331 [details]
MozReview Request: Bug 1262312 - Don't update the device registration every time we open Sync preferences. r?markh

Needed to fix excessive server load on release. Please uplift to beta and m-r
Attachment #8738331 - Flags: approval-mozilla-release?
Attachment #8738331 - Flags: approval-mozilla-release+
Attachment #8738331 - Flags: approval-mozilla-beta?
Attachment #8738331 - Flags: approval-mozilla-beta+
Thanks Kit. I built from the release branch and verified it is working as expected. I can no longer reproduce unexpected requests to the device registration endpoint.
No one has mentioned esr. Does this affect 45.1.0 esr?
Flags: needinfo?(kcambridge)
I don't think so. Device registration is 46+, AFAIK.
Flags: needinfo?(kcambridge)
You need to log in before you can comment on or make changes to this bug.