Conversation
| { | ||
| var oldValue = Stats[replica]; | ||
| var ema = Stats[replica] * (1 - Coefficient) + latency * Coefficient; | ||
| Stats.TryUpdate(replica, ema, oldValue); |
There was a problem hiding this comment.
TryUpdate не гарантирует, что данные обновлены. В данной задаче шанс что попытка обновления провалится минимальна, но в реальной жизни (в реальном коде) с высоким параллелизмом и нагрузкой, нужно это учитывать и делать хотя бы попытки повтора, если TryUpdate вернул false либо использовать AddOrUpdate.
Либо быть уверенным, что потеря какой-то части данных не критична для конкретной задачи, тогда можно не усложнять
|
|
||
| if (result.Status == TaskStatus.RanToCompletion) | ||
| { | ||
| ReplicaStats.Update(address, sw.Elapsed.Milliseconds); |
There was a problem hiding this comment.
sw.Elapsed.Milliseconds возвращает не общее прошедшее время в миллисекундах, а количество миллисекунд текущего интервала (текущей секунды) и в целом может возвращать только значения от -999 до 999.
Чтобы получить общее (реальное) количество прошедшего времени в миллисекундах, нужно вызывать либо sw.ElapsedMilliseconds либо sw.Elapsed.TotalMicroseconds
|
|
||
| if (completedTask != timeoutTask && completedTask.Status == TaskStatus.RanToCompletion) | ||
| { | ||
| ReplicaStats.Update(address, sw.Elapsed.Milliseconds); |
There was a problem hiding this comment.
sw.Elapsed.Milliseconds возвращает не общее прошедшее время в миллисекундах, а количество миллисекунд текущего интервала (текущей секунды) и в целом может возвращать только значения от -999 до 999.
Чтобы получить общее (реальное) количество прошедшего времени в миллисекундах, нужно вызывать либо sw.ElapsedMilliseconds либо sw.Elapsed.TotalMicroseconds
|
|
||
| if (result.Status == TaskStatus.RanToCompletion) | ||
| { | ||
| ReplicaStats.Update(address, sw.Elapsed.Milliseconds); |
There was a problem hiding this comment.
Так же некорректно обновляется статистика. У нас в result может оказаться как таска запроса к текущему адресу (address в foreach), так и таска принадлежащая предыдущему адресу.
В итоге может быть ситуация, когда мы записываем статистику о времени выполнения для адреса address2, но вернулась таска с завершившимся запросом к address1. В итоге мы некорректно наполняем статистику
| public abstract class ClusterClientBase | ||
| { | ||
| public abstract class ClusterClientBase | ||
| protected readonly ReplicaStats ReplicaStats; |
There was a problem hiding this comment.
ReplicaStats Получился локальным и хранит историю только для текущего экземпляра ClustrClient, если бы в будущем мы захотели в разных ситуациях делать запросы через разные стратегии, то у них бы была различная история производительности реплик и не оптимальная маршрутизация)
Это не ошибка, просто хотел подсветить один неучтенный сценарий)
No description provided.