dotnet / aspire

Tools, templates, and packages to accelerate building observable, production-ready apps
https://learn.microsoft.com/dotnet/aspire
MIT License
3.91k stars 475 forks source link

Fix issue 5972 #6684

Closed karolz-ms closed 1 week ago

karolz-ms commented 1 week ago

Description

Turns out that we did not wait long enough for the Container graceful stop to occur during restart sequence. This change tweaks the timeout used there.

Honestly I am not exactly sure how Polly calculates the exponential delay periods. Mathematically the original retry strategy settings should have been sufficient, but in reality we have been waiting too short. Polly also lacks the equivalent of MaxElapsedTime present in other retry frameworks, so I have verified experimentally that the current settings give desired max retry time.

Fixes #5972

Checklist

davidfowl commented 1 week ago

cc @martincostello for polly comment

martincostello commented 1 week ago

For the behaviour you're after (don't exceed this amount of time in total) you'd typically add a timeout policy to the resilience pipeline. Then the cancellation token for that would be passed from the timeout policy to the retry one, and it would observe it and terminate the retries when it's signalled.

karolz-ms commented 6 days ago

Thanks @martincostello I will keep this in mind for the next time when I am working in this part of the codebase.