snowplow / snowbridge

For replicating streams across clouds, accounts and regions
Other
14 stars 8 forks source link

Add configurable retries for target writes #361

Closed pondzix closed 1 month ago

pondzix commented 1 month ago

PDP-1346

Currently retrying configuration is hardcoded to 5 attempts with exponential delay (inital 1 second) for each type of an error.

This commit brings a couple of improvements to retrying:

pondzix commented 1 month ago

General question - what does this implementation mean for the idea of relying on the SDK retries for other targets?

Not much. Retrying in SDKs is still there 😁

Whatever transient error leaks from the SDK (like throttling error for kinesis) will handled here + from now retrying details (delay, max attempts) could also be configured. In the future we can also extend other target to catch and return setup errors to use different retry strategy. But we don't have to do anything now.

With current state of things I just think this solution is a nice middle ground.

colmsnowplow commented 1 month ago

With current state of things I just think this solution is a nice middle ground.

I don't disagree!

Overall looks good, just two things I think need tweaking before I approve (possible value leakage and placement of parsing respose) :)