Азанов Андрей#61
Conversation
| .Where(t => !ResponseStatisticsHistory.TryGetValue(t, out var statistics) | ||
| || statistics.IsSuccessful) |
There was a problem hiding this comment.
В доп задании требовалось сортировать их по времени, а не совсем исключать в случае ошибок)
Вне лабораторных условий может произойти ситуация, когда такой подход вычеркнет все реплики из-за к примеру кратковременной сетевой недоступности самого приложения. В итоге отправка запросов встанет до перезапуска.
Лучше либо делать скользящее окно — сохранять и использовать время последней ошибки и исключать из списка реплик только когда ошибка "свежая", либо просто делать их ResponseTime максимально плохим, чтобы они становились последними в списке.
Совсем исключать их навсегда — плохой подход
| if (!completedTask.IsCompletedSuccessfully) | ||
| goodAddressesCount--; |
There was a problem hiding this comment.
Если запрос не успел выполнится до истечения таймаута, в Task.WhenAny вернется Task.Delay и его результат, а он собственно всегда будет IsCompletedSuccessfully
В итоге не всегда будет уменьшаться goodAddressCount
| ResponseStatisticsHistory[replicaAddress] = | ||
| new ResponseStatistics(completedTask.IsCompletedSuccessfully, sw.Elapsed); |
There was a problem hiding this comment.
Тут по причине выше мы записываем неверную статистику, мы для реплики записываем статистику "удачности" и "времени выполнения", вот только в случае таймаута мы в статистику будет записывать true если completedTask != requestTask.
В итоге у нас почти никогда не будет неработающих реплик, только если ошибка запроса вывалится раньше, чем отработает таймаут Task.Delay — не совсем корректное поведение получается
| _ = requestTask.ContinueWith(t => ResponseStatisticsHistory[replicaAddress] | ||
| = new ResponseStatistics(t.IsCompletedSuccessfully, sw.Elapsed)); |
There was a problem hiding this comment.
Тут происходит замыкание и переменная sw захватывается делегатом в ContinueWith.
Переменная sw переопределяется на каждой итерации. Все continuation будут захватывать одну и ту же переменную, а не её значение на момент запуска задачи. Когда continuation выполнится (возможно, спустя несколько итераций), sw.Elapsed вернёт время, прошедшее с последнего перезапуска секундомера, а не с момента отправки конкретного запроса.
Здесь надо аккуратнее работать с замыканиями.
Под капотом Stopwatch.StartNew() создается новый экземпляр и гипотетически не должно быть проблем .net может это разрулить, но не во всех ситуациях; Лучше избегать замыканий переменных
|
|
||
| if (completedTask == timeoutTask) | ||
| { | ||
| ResponseStatisticsHistory[replicaAddress] = new ResponseStatistics(true, sw.Elapsed); |
There was a problem hiding this comment.
Таумаут тоже считаем "успешным" завершением? Немного странновато тогда выглядит.
Плюс у нас ContinueWith перезапишет статистику реплики "реальным" временем после завершения запроса, запись этой статистики выглядит избыточной и скорее даже вредной т.к. может привести к состоянию гонки
2 participants
No description provided.