1148701 - React to Backoff and Retry-After headers from Reading List storage servers

Status: RESOLVED FIXED

(Firefox Graveyard :: Reading List, defect, P1)

Description

[Mark Hammond [:markh] [:mhammond]]

Desktop version of bug 1140812 - our Sync client doesn't react to any backoff or retry headers.  In theory the scheduler is all set to honor them (is is listening for observer notifications), so we just need the Sync engine to send them.

Not clear what the priority is - hopefully the impact of *not* doing so isn't going to be user visible, but I'm sure the services team would like it and it may help improve the overall reliability.

Comment 1

[Justin Dolske [:Dolske]]

P1 - sounds like we're going to want this as an important safety mechanism to deal with the possibility of an overloaded service.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Mark/Drew, can one of you take this?

Comment 3

[Mark Hammond [:markh] [:mhammond]]

Attachment: 0011-Bug-1148701-React-to-Backoff-and-Retry-After-headers.patch

Simple patch that has ServerClient send observer notifications upon the documented headers. Note that the scheduler already has support (and tests) for this notification.

Comment 4

[Mark Hammond [:markh] [:mhammond]]

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

Comment 5

[Carsten Book [:Tomcat]]

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

Comment 6

[Mark Hammond [:markh] [:mhammond]]

Comment on attachment 8591493 [details] [diff] [review]
0011-Bug-1148701-React-to-Backoff-and-Retry-After-headers.patch

Approval Request Comment
[Feature/regressing bug #]: readinglist
[User impact if declined]: Fx might overload servers causing everyone to have a bad time
[Describe test coverage new/current, TreeHerder]: New test
[Risks and why]: Very low risk - limited to schedule of automatic reading list syncs.
[String/UUID change made/needed]: None

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8591493 [details] [diff] [review]
0011-Bug-1148701-React-to-Backoff-and-Retry-After-headers.patch

We don't want to DOS ourself. Taking it
Should be in 38 beta 5.

Comment 8

[Ryan VanderMeulen (PTO, back 6-April)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/1566bebe56be

Comment 9

[Ryan VanderMeulen (PTO, back 6-April)]

https://hg.mozilla.org/releases/mozilla-beta/rev/91df81e2edac

Comment 10

[Andrei Vaida [:avaida]]

Setting qe-verify- since this fix seems to be already covered by automated tests.