Conversation
| _replicaStatistics[replicaAddress] = | ||
| _replicaStatistics[replicaAddress] * ConfidenceFactor + workTime * (1 - ConfidenceFactor); |
There was a problem hiding this comment.
Здесь несколько операций: чтение по ключу; арифметические операции; запись по ключу.
Если два потока попытаются вызвать метод одновременно, никто не гарантирует, в каком порядке операции исполнятся (например, они одновременно могут прочитать 0, а затем каждый запишет 0.2*workTime, хотя верным бы было 0.8*(0.2*workTime1) + 0.2*workTime2)
Тебе нужен метод ConcurrentDictionary<,>.AddOrUpdate - он принимает делегат и гарантирует, что новое значение будет вычислено от актуального предыдущего, защищая от гонки потоков
(side-note: AddOrUpdate не гарантирует, что делегат будет вызван единожды, он лишь гарантирует, что новое значение будет верным; но конкретно здесь это будет вполне нормальное поведение и можно об этом не думать)
| tasks.Add(ProcessRequestAsync(request)); | ||
| } | ||
|
|
||
| var timeoutTask = Task.Delay(timeout).ContinueWith<string>(_ => throw new TimeoutException()); |
There was a problem hiding this comment.
ContinueWith ни на что не влияет: исключение внутри него, конечно, возникнет, но т.к. ты не делаешь await timeoutTask, оно останется завёрнутым в таску и дальше не вылетит. Ты можешь удалить ContinueWith, ведь ниже, в 33-й строке, у тебя и так есть явный выброс исключения.
| var remainingTime = timeout - timer.Elapsed; | ||
| var timeoutForReplica = TimeSpan.FromTicks(remainingTime.Ticks / (sortedReplicas.Length - i)); | ||
| var timeoutTask = Task.Delay(timeoutForReplica); |
There was a problem hiding this comment.
remainingTime и timeoutForReplica могут оказаться отрицательными. Task.Delay может или выбросить исключение, или, с маленьким шансом, превратиться в никогда не заканчивающуюся таску
| var requestResult = await Task.WhenAny(task, timeoutTask); | ||
| if (requestResult != timeoutTask) |
There was a problem hiding this comment.
если requestResult == timeoutTask, то статистика для реплики не обновится
| { | ||
| _replicasStatistics.UpdateStats(replicaData.Item1, time.TotalMilliseconds); | ||
| requests.Remove(result); | ||
| return result.Result; |
There was a problem hiding this comment.
Если все реплики будут тормозить, то соответствующие таски так и останутся в requests, и статистика для них не обновится
| for (var i = 0; i < ReplicaAddresses.Length; ++i) | ||
| { | ||
| var request = CreateRequest(ReplicaAddresses[i] + "?query=" + query); | ||
| tasks.Add(ProcessRequestAsync(request)); | ||
| } | ||
|
|
||
| var timeoutTask = Task.Delay(timeout).ContinueWith<string>(_ => throw new TimeoutException()); |
There was a problem hiding this comment.
По-хорошему timeoutTask бы создать до цикла c ProcessRequestAsync. Иначе выходит, что запросы уже какое-то время как отправлены
3 participants
No description provided.