-
Notifications
You must be signed in to change notification settings - Fork 54
Тимур Бабаев ДЗ №2 #64
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,9 +13,29 @@ public SmartClusterClient(string[] replicaAddresses) : base(replicaAddresses) | |
| { | ||
| } | ||
|
|
||
| public override Task<string> ProcessRequestAsync(string query, TimeSpan timeout) | ||
| public override async Task<string> ProcessRequestAsync(string query, TimeSpan timeout) | ||
| { | ||
| throw new NotImplementedException(); | ||
| var replicas = OrderReplicas().ToArray(); | ||
| var tasks = new List<Task<string>>(); | ||
| var timeoutTask = Task.Delay(timeout); | ||
|
|
||
| foreach (var replica in replicas) | ||
| { | ||
| var task = ProcessWithStatsAsync(replica, query); | ||
| tasks.Add(task); | ||
|
|
||
| await Task.WhenAny(task, Task.Delay(timeout / replicas.Length), timeoutTask); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Нужно было учесть, что запрос может упасть и он занял меньше времени чем выделенный ему таймаут и потратить оставшееся время на других запросы, чтобы они прошли В целом логика должна была быть такая:
В итоге мы для каждого последующего запроса будем делать индивидуальный perReplicaTimeout учитывая сколько времени (реального) потратили на предыдущую попытку
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Так же для эффективности, здесь можно ждать не только текущую таску и таймауты, но и все уже запущенные. Тогда мы можем быстрее среагировать, если кто-то какой-то предыдущий запрос успел выполнится. НО нужно выкидывать из списка выполняющихся таск упавшие, чтобы они не мешались в WhenAny |
||
|
|
||
| if (task.IsCompletedSuccessfully) | ||
| return task.Result; | ||
| } | ||
|
|
||
| var final = await Task.WhenAny(tasks.Append(timeoutTask)); | ||
|
|
||
| if (final == timeoutTask) | ||
| throw new TimeoutException(); | ||
|
|
||
| return await (Task<string>)final; | ||
|
Comment on lines
+33
to
+38
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Здесь в финале нам может вернуться не успешно выполненная, а упавшая таска и тогда мы в return получим исключение. Тут бы хорошо в цикле проходится по всем таскам и возвращать только если она успешно выполнена, попутно проверяя не закончилось ли время у timeoutTask |
||
| } | ||
|
|
||
| protected override ILog Log => LogManager.GetLogger(typeof(SmartClusterClient)); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Тут статистика обновляется только если запрос успешный, если он не успешный, то статистика никогда, не критично, но не будет отображать реальную картину