search

salesforce / storm-dynamic-spout

A framework for building spouts for Apache Storm and a Kafka based spout for dynamically skipping messages to be processed later.
BSD 3-Clause "New" or "Revised" License
41 stars 13 forks source link

Change the default retry delay to have exponential growth #110

Closed mmoldavan closed 6 years ago

mmoldavan commented 6 years ago

While debugging retries, I noticed that the current retry behavior follows linear growth (2, 4, 6, 8 seconds). However, the comments on the DefaultRetryManager class and retry variables imply expontential growth (2, 4, 8, 16, etc.).

This fixes the retry code to grow exponentially as expected.

salesforce-cla[bot] commented 6 years ago

Thanks for the contribution! Before we can merge this, we need @mmoldavan to sign the Salesforce.com Contributor License Agreement.

stanlemon commented 6 years ago

@Crim side note to the PR, I kind of think we should rename this class in 0.10 to ExponentialBackoffRetryManager, what do you think of that?

stanlemon commented 6 years ago

@mmoldavan this looks great, thanks for the contribution. I left a few comments, nothing major. Once we get those covered can you add an item to the change log and we’ll get this merged in. Do you need me to backport this over to 0.9?

mmoldavan commented 6 years ago

👍 I will fix up the comments and changelog. If storm can be bumped to 0.10 easily, I don't think it needs backporting necessarily.

stanlemon commented 6 years ago

@mmoldavan 0.10 is still in development and none of the tooling internal for Stardust supports it yet so I'll make sure to back port this to 0.9. I don't imagine that being much of a problem. Once that's done I'll cut new releases.

stanlemon commented 6 years ago

Thanks for the contribution @mmoldavan!

I'll work on back porting this next week, feel free to ping me for updates.