Dispose with timeout #72
Conversation
| /// <summary> | ||
| /// Dispose timeout for each component of <see cref="IVostokHostingEnvironment"/> | ||
| /// </summary> | ||
| public TimeSpan DisposeComponentTimeout { get; set; } = 5.Seconds(); |
There was a problem hiding this comment.
изначально думал заиспользовать ShutdownTimeoutBudget, но потом решил, что лучше дать каждому компоненту шанс задиспоузится
иначе мы можем даже не увидить (если не диспоузить файл-лог) сообщения о том, что не задиспоузилось что-то ещё
There was a problem hiding this comment.
Думаю, что это правильнее: диспоуз пользовательских компонентов в начале не должен надолго блочить диспоуз других компонентов.
Но тут есть несколько нюансов:
- Пользователь может ожидать, что его компоненты точно диспоузятся по порядку, и в случае если один из диспоузов завис, это не соблюдается, так как мы тупо забиваем по таймауту и идём дальше. Хз как это красиво порешать, правда.
- Мб стоит флаш логов зашедулить на старте диспоуза, чтобы даже если мы зависли где-то и не дошли до логов, то мы бы хотя бы выплюнули логи, перед тем как нас приложение прибьёт?
|
|
||
| log.Info("Disposed of the application in {ApplicationDisposeTime}.", watch.Elapsed.ToPrettyString()); | ||
| if (shouldLog && componentName == TheApplication) | ||
| log.Info("Disposed of the application in {ApplicationDisposeTime}.", watch.Elapsed.ToPrettyString()); | ||
| } | ||
| catch (Exception error) |
There was a problem hiding this comment.
@HolyPrapor глянь плиз свой тест Should_return_CrashedDuringStopping
я тут захендлил все ошибки, и теперь он не работает
There was a problem hiding this comment.
Он и не может проходить: там проверяется что если Dispose исключение выбросил, то всё обработано
Видимо теперь это поле невалидно, можно как Obsolete пометить, но я сомневаюсь что кто-то его использовал, мб можно и просто убрать
| @@ -61,7 +61,7 @@ internal class HostingShutdown : IDisposable | |||
| hostShutdownBudget = TimeBudget.CreateNew(totalTimeout); | |||
| tokenRegistration = token.Register(OnHostShutdownTriggered); | |||
| } | |||
|
|
|||
| context.HostExtensions = HostExtensions; | ||
|
|
||
| // note (kungurtsev, 02.11.2021): user components should be disposed right after application in reverse order |
There was a problem hiding this comment.
Немного неочевидно, что компоненты там уже лежат в правильном порядке, как-то это извне получается.
Было бы яснее, если бы мы прямо во время диспоуза делали такое преобразование
|
|
||
| log.Info("Disposed of the application in {ApplicationDisposeTime}.", watch.Elapsed.ToPrettyString()); | ||
| if (shouldLog && componentName == TheApplication) | ||
| log.Info("Disposed of the application in {ApplicationDisposeTime}.", watch.Elapsed.ToPrettyString()); | ||
| } | ||
| catch (Exception error) |
There was a problem hiding this comment.
Он и не может проходить: там проверяется что если Dispose исключение выбросил, то всё обработано
Видимо теперь это поле невалидно, можно как Obsolete пометить, но я сомневаюсь что кто-то его использовал, мб можно и просто убрать
| /// <summary> | ||
| /// Dispose timeout for each component of <see cref="IVostokHostingEnvironment"/> | ||
| /// </summary> | ||
| public TimeSpan DisposeComponentTimeout { get; set; } = 5.Seconds(); |
There was a problem hiding this comment.
Думаю, что это правильнее: диспоуз пользовательских компонентов в начале не должен надолго блочить диспоуз других компонентов.
Но тут есть несколько нюансов:
- Пользователь может ожидать, что его компоненты точно диспоузятся по порядку, и в случае если один из диспоузов завис, это не соблюдается, так как мы тупо забиваем по таймауту и идём дальше. Хз как это красиво порешать, правда.
- Мб стоит флаш логов зашедулить на старте диспоуза, чтобы даже если мы зависли где-то и не дошли до логов, то мы бы хотя бы выплюнули логи, перед тем как нас приложение прибьёт?
No description provided.