aws-sdk-perl icon indicating copy to clipboard operation
aws-sdk-perl copied to clipboard

Incomplete Retry schedule implementation

Open leonerd opened this issue 7 years ago • 2 comments

From looking at the definition of a retry schedule (e.g. from Paws/SMS.pm) it would appear to have three parameters:

{ base => 'rand', type => 'exponential', growth_factor => 2 } (https://metacpan.org/source/JLMARTIN/Paws-0.38/lib/Paws/SMS.pm#L10)

In fact, the code only obeys the type argument; both the random base and the growth factor of 2 are hardcoded into the algorithm:

(2 ** ($self->tries - 2)) + (rand(1) / 2); https://metacpan.org/source/JLMARTIN/Paws-0.38/lib/Paws/API/Retry.pm#L41

Almost all of the service definitions hardcode the very same definition for the retry schedule, though a few do not; e.g.

lib/Paws/DynamoDB.pm:10:    { base => '0.05', type => 'exponential', growth_factor => 2 }

so the user may be surprised to find this is not honoured.

leonerd avatar Nov 05 '18 18:11 leonerd

Given as the implementation hardcodes a growth factor of 2, and that is supplied to every single service default anyway, I'd suggest it might be simpler for now just to remove that from the definitions of them, since then nobody gets surprised if they change the value and it isn't honoured.

However, I see that those 2s are template-generated from the service definition JSON files, so it's only correct that currently everything uses a factor of 2. Maybe oneday one of these will change. Perhaps the code should be extended to use that after all?

leonerd avatar Nov 05 '18 18:11 leonerd

When I implemented the exponential backoff, I first had to import the parameters from botocore, thinking that I would implement the same as them, but I started getting strange results. It turns out that the backoff didn't seem exponentialit would have strange wait times, sometimes finally timing out very fast, and sometimes very slowly). That make me change it to be more "predictable", but I ended up ignoring some of the parameters from boto.

Note: it looks like botocore has the same problems the way they implement the backoff https://github.com/boto/botocore/issues/1471

pplu avatar Nov 21 '18 20:11 pplu