Configurable network error retry policy

[muhamadazmy]

Fixes #1769

This makes the network error retry policy configurable for the node

[tillrohrmann]

Thanks for making the network error retry policy configurable @muhamadazmy :-) The changes look good to me. I had one suggestion for a slight simplification. +1 for merging after resolving the comment.

[tillrohrmann]

Wondering whether we can define the type as RetryPolicy because RetryPolicy has a None variant as well.

[muhamadazmy]

@tillrohrmann
True, there is a None variant on the RetryPolicy. But i didn't want break compatibility with current configurations files which has no network_error_retry_policy defined. If we just use RetryPolicy it will default to None (hence no retry), which breaks the default behaviour which has exponential retry.

But with Option, if network_error_retry_policy is completely missing from the config the function network_error_retry_policy can still return a default policy (the one that was hard coded in code), unless you configure retry policy explicitly to None in config (like network_error_retry_policy: none ) to explicitly disable retries

I am still totally fine of using RetryPolicy instead of Option if you think that is acceptable behaviour, just let me know :)

[tillrohrmann]

Can we configure serde using the same default value as the one we define in the Default implementation? Then if nothing is specified we would default to

RetryPolicy::exponential(
      Duration::from_millis(10),
      2.0,
      Some(15),
      Some(Duration::from_secs(5))

[tillrohrmann]

I think that the current Default implementation should already be respected. No need to instruct serde separately. The defaults for the configuration are configured here:

restate/cli/src/cli_env.rs

Line 167 in 3271b79

let mut figment = Figment::from(Serialized::defaults(defaults))

.

[muhamadazmy]

Ah that's good to know. Good one :)

[tillrohrmann]

Nice work @muhamadazmy. LGTM. +1 for merging :-)