Add overload EnableRetryOnFailure that allows specifying errorCodesToAdd without count / delay

[midgleyc]

Feature request: add another overload to NpgsqlDbContextOptionsBuilder.EnableRetryOnFailure that takes errorCodesToAdd without forcing maxRetryCount and maxRetryDelay to be added.

I want to keep the latter two default, so I copied the values from ExecutionStrategy as they're protected and so not visible externally, but it would be nicer if I didn't have to do this and could just specify the errorCodesToAdd property I'm interested in.

[roji]

@midgleyc sorry for not answering this sooner.

This API mirrors the corresponding SQL Server API - if you feel strongly about this being needed, can you please open an issue on the EF Core repo?

I personally don't think there's any value in not specifying maxRetryCount and maxRetryDelay - the defaults (6 and 30 seconds) are quite arbitrary and it should be perfectly fine to just specify them explicitly.

[midgleyc]

Thanks for the response.

As you say, they're somewhat arbitrary magic numbers, and if I specify them in my code, I also have to leave a comment explaining where they come from, so as to avoid potentially puzzling readers. Whereas if I didn't specify them explicitly, I wouldn't have to comment it, because there would be nothing to comment :)

Additionally, if the defaults ever changed, my application would get the updated values, instead of having to manually update them. Again, an unlikely event, but a possible one?

I'm not overly concerned about this, and I did indeed go for the explicit magic numbers + comment approach.

[roji]

As you say, they're somewhat arbitrary magic numbers, and if I specify them in my code, I also have to leave a comment explaining where they come from, so as to avoid potentially puzzling readers.

Do you feel like you'd have to document this choice if the API didn't have defaults in the first place, and so forced you to specify the values regardless of errorCodesToAdd? If not, I'm not sure why you'd need to document in this case either.

Additionally, if the defaults ever changed, my application would get the updated values, instead of having to manually update them. Again, an unlikely event, but a possible one?

But that's the point about these numbers being arbitrary - I don't think there's any reason to want to get "updated" default values. I personally consider the default of 6 retries to be too much, for example - it may be better to actually think and decide on the values you want.

[midgleyc]

it may be better to actually think and decide on the values you want

I agree, but I'm not an expert at how these retry values relate to performance. I can reasonably assume that whoever wrote the class with these defaults knows significantly more than I do, and that the numbers they pick are significantly better than numbers I would pick (in general). Now I can do some testing and think about my own application, and that might be enough -- but I don't know what I don't know (in the Donald Rumsfeld "unknown unknowns" sense), and I might be very wrong. I don't think I am, but that's always a risk I can avoid by just using somebody else's defaults (in return for gaining the risk that their defaults might be completely wrong for my application).

I am convinced that you don't want updated defaults, though: if a given set of numbers work for your application, that's reason to keep them.

Do you feel like you'd have to document this choice if the API didn't have defaults in the first place, and so forced you to specify the values regardless of errorCodesToAdd?

Yes. Not a detailed documentation, but something like "numbers arbitrary based on feeling" or "numbers chosen to be the same as <popular project / other internal project>". I think there should be some justification, especially if the justification is that there's no justification, so a future reader knows whether they can reasonably replace them or whether they're set like they are for a good reason.

[roji]

OK, thanks for the explanations. FWIW I don't think the EF Core defaults have any particular smarts behind them - the number of retries and the delay is something you can probably pick reasonably well.

In any case, how about suggesting this at the EF Core level in that repo? Based on the discussion there I can follow suit here.

[midgleyc]

Filed dotnet/efcore#27074. Thanks.

[roji]

Note that this was approved on the EF Core side for 7.0, so if you want to submit a PR for this on the Npgsql side I can merge that as well.

[midgleyc]

Filed #2206.

Cheers.