Skip to content

Шубин Игорь#60

Open
Nevisinn wants to merge 1 commit intokontur-courses:masterfrom
Nevisinn:master
Open

Шубин Игорь#60
Nevisinn wants to merge 1 commit intokontur-courses:masterfrom
Nevisinn:master

Conversation

@Nevisinn
Copy link
Copy Markdown

No description provided.

{
var oldValue = Stats[replica];
var ema = Stats[replica] * (1 - Coefficient) + latency * Coefficient;
Stats.TryUpdate(replica, ema, oldValue);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TryUpdate не гарантирует, что данные обновлены. В данной задаче шанс что попытка обновления провалится минимальна, но в реальной жизни (в реальном коде) с высоким параллелизмом и нагрузкой, нужно это учитывать и делать хотя бы попытки повтора, если TryUpdate вернул false либо использовать AddOrUpdate.

Либо быть уверенным, что потеря какой-то части данных не критична для конкретной задачи, тогда можно не усложнять


if (result.Status == TaskStatus.RanToCompletion)
{
ReplicaStats.Update(address, sw.Elapsed.Milliseconds);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sw.Elapsed.Milliseconds возвращает не общее прошедшее время в миллисекундах, а количество миллисекунд текущего интервала (текущей секунды) и в целом может возвращать только значения от -999 до 999.

Чтобы получить общее (реальное) количество прошедшего времени в миллисекундах, нужно вызывать либо sw.ElapsedMilliseconds либо sw.Elapsed.TotalMicroseconds


if (completedTask != timeoutTask && completedTask.Status == TaskStatus.RanToCompletion)
{
ReplicaStats.Update(address, sw.Elapsed.Milliseconds);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sw.Elapsed.Milliseconds возвращает не общее прошедшее время в миллисекундах, а количество миллисекунд текущего интервала (текущей секунды) и в целом может возвращать только значения от -999 до 999.

Чтобы получить общее (реальное) количество прошедшего времени в миллисекундах, нужно вызывать либо sw.ElapsedMilliseconds либо sw.Elapsed.TotalMicroseconds


if (result.Status == TaskStatus.RanToCompletion)
{
ReplicaStats.Update(address, sw.Elapsed.Milliseconds);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Так же некорректно обновляется статистика. У нас в result может оказаться как таска запроса к текущему адресу (address в foreach), так и таска принадлежащая предыдущему адресу.

В итоге может быть ситуация, когда мы записываем статистику о времени выполнения для адреса address2, но вернулась таска с завершившимся запросом к address1. В итоге мы некорректно наполняем статистику

public abstract class ClusterClientBase
{
public abstract class ClusterClientBase
protected readonly ReplicaStats ReplicaStats;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ReplicaStats Получился локальным и хранит историю только для текущего экземпляра ClustrClient, если бы в будущем мы захотели в разных ситуациях делать запросы через разные стратегии, то у них бы была различная история производительности реплик и не оптимальная маршрутизация)

Это не ошибка, просто хотел подсветить один неучтенный сценарий)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants