Skip to content
This repository has been archived by the owner on Apr 3, 2019. It is now read-only.

Document basic backoff protocol. #323

Merged
merged 3 commits into from Nov 19, 2013
Merged

Document basic backoff protocol. #323

merged 3 commits into from Nov 19, 2013

Conversation

rfk
Copy link
Contributor

@rfk rfk commented Nov 18, 2013

Here is a super-basic backoff protocol to go along with the 429/503 error responses. It only does retry-after, no proof-of-work shortcuts allowed. We can expand with a PoW thing later, but let's get the basics into the protocol so clients can bake in support as early as possible.

It's not clear how much value we'll get out of the Backoff header for this service, since FxA clients wont tend to be doing lots of background requests to the server. But I think it's better to have a consistent mechanism across all services, so we might as well start by including it in here.

@ckarlof r?

@ckarlof
Copy link
Contributor

ckarlof commented Nov 18, 2013

Why don't 429s get any docs in the Backoff section? They are similar to the 503 case, but it would be nice to mention them.


```
HTTP/1.1 200 OK
Backoff: 20
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Do we really need to support this, i.e., make it simpler? Presumably we can shoot a 429 or 503 when we need to. This would just make some edge cases (maybe) better.
  2. If we really do need it, should it be X-Backoff?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm open to removing it in this case, since the client should not be sending lots of background traffic to the server. One place where it might be useful is when polling for verification state, but then we don't plan to poll for that in future anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. If we really do need it, should it be X-Backoff?

https://tools.ietf.org/html/rfc6648

But I don't really care either way.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not do Backoff in 200 unless you think we really need it. Would it have helped in the past Sync meltdowns?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it have helped in the past Sync meltdowns?

Yes, but I think that's particular to the way sync behaves. It does lots of background activity that can profitably be delayed upon request. It also gets a lot of value out of allowing clients to complete a sync, no matter how slowly, as this tends to decrease load on the DB due to future retries.

I'm happy to remove it here as its value would be marginal at best.

@rfk
Copy link
Contributor Author

rfk commented Nov 19, 2013

Comments addressed, @ckarlof r?

@ckarlof
Copy link
Contributor

ckarlof commented Nov 19, 2013

r+. I agree that 200 w/Backoff might be appropriate in Sync or when there are more chatty background ops. Implementation issue opened: #331.

ckarlof added a commit that referenced this pull request Nov 19, 2013
Document basic backoff protocol.
@ckarlof ckarlof merged commit c718e99 into master Nov 19, 2013
@dannycoates dannycoates deleted the rfk/basic-backoff branch January 15, 2014 00:33
rfk pushed a commit that referenced this pull request Oct 24, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants