clarify retry strategy #14

Closed
jed opened this Issue Apr 27, 2012 · 8 comments

Comments

Projects
None yet
2 participants
@jed
Owner

jed commented Apr 27, 2012

while currently undocumented, this library takes a different approach to retries using the (err, data, next) signature in callback functions. in this case, next is a function that has the currently specified query in a closure:

  • if err does not exist, next is used for iteration/pagination to get the next set of results.
  • if err does exist, next is a retry function that can be executed at user discretion

the idea behind diverging from other SDKs is that retrying shouldn't be baked into the library, but performed according to user policy. would love to get feedback on this before settling on it in the documentation.

@mhart

This comment has been minimized.

Show comment
Hide comment
@mhart

mhart May 3, 2012

Contributor

The tricky thing about this is that there is some DynamoDB-specific logic that needs to be dealt with - and leaving this up to the user unnecessarily burdens them IMO.

Some examples of the sorts of things that other client libraries deal with:

https://github.com/amazonwebservices/aws-sdk-for-php/blob/master/sdk.class.php#L936-977

https://github.com/boto/boto/blob/develop/boto/dynamodb/layer1.py#L150-179

https://github.com/teleportd/node-dynamodb/blob/master/lib/ddb.js#L918-950

So it might be OK to exclude this from the lib - but I think it would be really nice to include an example of the things you need to keep in mind when retrying, or even better, a util to do the retrying so that users can opt in.

Contributor

mhart commented May 3, 2012

The tricky thing about this is that there is some DynamoDB-specific logic that needs to be dealt with - and leaving this up to the user unnecessarily burdens them IMO.

Some examples of the sorts of things that other client libraries deal with:

https://github.com/amazonwebservices/aws-sdk-for-php/blob/master/sdk.class.php#L936-977

https://github.com/boto/boto/blob/develop/boto/dynamodb/layer1.py#L150-179

https://github.com/teleportd/node-dynamodb/blob/master/lib/ddb.js#L918-950

So it might be OK to exclude this from the lib - but I think it would be really nice to include an example of the things you need to keep in mind when retrying, or even better, a util to do the retrying so that users can opt in.

@jed

This comment has been minimized.

Show comment
Hide comment
@jed

jed May 3, 2012

Owner

good points, @mhart, and thanks for the other references. it seems there's enough consensus here.

how about this: we handle all retry logic according to best practices (backoff, etc) for the next function, but allow the user to decide whether to call it or not. thoughts?

Owner

jed commented May 3, 2012

good points, @mhart, and thanks for the other references. it seems there's enough consensus here.

how about this: we handle all retry logic according to best practices (backoff, etc) for the next function, but allow the user to decide whether to call it or not. thoughts?

@mhart

This comment has been minimized.

Show comment
Hide comment
@mhart

mhart May 3, 2012

Contributor

I guess I haven't used it enough in production yet to know what the most likely use case would be.

My main question is: when would you not want to retry? Are there any examples you know of using the other APIs where the retry has somehow bitten people? (... checks Google ... can't find anything - the only cases I could imagine it would be a problem would be the non-idempotent ones, incrementing counters, non-conditional updates, etc)

Contributor

mhart commented May 3, 2012

I guess I haven't used it enough in production yet to know what the most likely use case would be.

My main question is: when would you not want to retry? Are there any examples you know of using the other APIs where the retry has somehow bitten people? (... checks Google ... can't find anything - the only cases I could imagine it would be a problem would be the non-idempotent ones, incrementing counters, non-conditional updates, etc)

@jed

This comment has been minimized.

Show comment
Hide comment
@jed

jed May 3, 2012

Owner

it's not that you wouldn't want to retry so much as you may want to retry on your own terms. but the more i think about it, the less i'm inclined to expose it. perhaps setting maxRetries for a client is enough?

Owner

jed commented May 3, 2012

it's not that you wouldn't want to retry so much as you may want to retry on your own terms. but the more i think about it, the less i'm inclined to expose it. perhaps setting maxRetries for a client is enough?

@mhart

This comment has been minimized.

Show comment
Hide comment
@mhart

mhart May 3, 2012

Contributor

Yeah - I think that'd be perfect - then if someone doesn't want to (which I'm pretty sure would not be the norm), then they can specify 0 maxRetries or whatever.

Contributor

mhart commented May 3, 2012

Yeah - I think that'd be perfect - then if someone doesn't want to (which I'm pretty sure would not be the norm), then they can specify 0 maxRetries or whatever.

@mhart

This comment has been minimized.

Show comment
Hide comment
@mhart

mhart May 3, 2012

Contributor

Perhaps down the track you could allow tuning of the backoff, allow a retry function to be passed in, something like that.

Contributor

mhart commented May 3, 2012

Perhaps down the track you could allow tuning of the backoff, allow a retry function to be passed in, something like that.

@jed

This comment has been minimized.

Show comment
Hide comment
@jed

jed Jul 11, 2012

Owner

by the way, this has been added to jed/dynamo-client@efd8e0d.

Owner

jed commented Jul 11, 2012

by the way, this has been added to jed/dynamo-client@efd8e0d.

@mhart

This comment has been minimized.

Show comment
Hide comment
@mhart

mhart Jul 11, 2012

Contributor

Schweeet

Contributor

mhart commented Jul 11, 2012

Schweeet

@jed jed closed this Mar 6, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment