Stage 5, Андрей Чешев, Политех #167
Conversation
…into stage-2-Cheshev
# Conflicts: # src/main/java/ru/vk/itmo/test/andreycheshev/ServerImpl.java
Зарезолвил конфликты. |
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
| private int counter; | ||
|
|
||
| public WorkerThreadFactory(String prefix) { | ||
| this.prefix = prefix + "-"; |
There was a problem hiding this comment.
кажется, теперь это уже лишнее
There was a problem hiding this comment.
WorkerThreadFactory до сих пор используется мною для конфигурирования executor`ов.
|
|
||
| @Override | ||
| public int compareTo(ResponseElements o) { | ||
| long diff = this.getTimestamp() - o.getTimestamp(); |
There was a problem hiding this comment.
кажется, можно не через геттеры, а по полям обращаться? В обоих кейсах
| } | ||
|
|
||
| public byte[] getBody() { | ||
| return Arrays.copyOf(body, body.length); // CodeClimate requirement. |
There was a problem hiding this comment.
целиком копировать массив - выглядит дорого. Может быть есть способ выдавать его read-only вариацию?
There was a problem hiding this comment.
В стандартной библиотеке Java нет прямого способа сделать массив read-only без копирования.
There was a problem hiding this comment.
Сделал с использованием обертки ByteBuffer, но копирование все равно осталось при получении.
| } | ||
|
|
||
| private static boolean isResponseSucceeded(int status) { | ||
| return status == 404 || status == 410 |
There was a problem hiding this comment.
мб статический сет с допустимыми статусами? Вместо множества условий с магическими константами)
| return (method == Request.METHOD_GET && collector.size() >= ack) || responseCount.get() >= from; | ||
| } | ||
|
|
||
| public void sendResponse() { |
There was a problem hiding this comment.
класс очень много за что отвечает. Из-за этого понять его логику очень трудно. Если класс является сборщиком ответов, то метод sendResponse в нем явно удивляет)
|
|
||
| List<Integer> nodes = distributor.getQuorumNodes(id, from); | ||
| List<Response> responses = new ArrayList<>(); | ||
| List<Integer> nodesIndices = distributor.getQuorumNodes(id, from); |
There was a problem hiding this comment.
а почему метод называется getQuorumNodes, если он возвращает ноды не по количеству кворумов, а столько, сколько запрошено через параметр from?)
|
|
||
|
|
||
| При реализации было использовано 5 экзекьюторов: | ||
| * Distributor - для распределения задач по CompletableFuture (6 потоков core, 6 потоков максимум); |
There was a problem hiding this comment.
А почему было использовано именно такое кол-во потоков на каждом из экзекьюторов? Было бы здорово пояснить, почему было выбрано именно так, поскольку на первый взгляд кажется, что у Distributor'а может быть меньше работы, чем у некоторых из ниже указанных
|  | ||
| При 90м, 99м, 99.9м персентилях задержка при новой реализация меньше примерно в два раза. Наблюдает значительное улучшение производительности. | ||
|
|
||
| И get и put запросы выдали меньшую задержку, чем ранее, но для get запросов после 99го персентиля замечен тренд резкого увеличения задержки. |
There was a problem hiding this comment.
с чем он может быть связан?
| * PUT | ||
|  | ||
|  | ||
| * Поскольку решение было переведено на использование java.net.HttpClient, то появились сопутствующие аллокации. |
There was a problem hiding this comment.
Маловато аналитики будет)
|  | ||
|  | ||
| * Нагрузка действительно распределилась по потокам. Наибольшее количество процессорного времени занимает RemoteCall протоки. | ||
| * LocalCall, Distributor потоки занимают в среднем от 0.5 до 1 проценту процессорного времени. |
|
Завтра смогу прислать дополненный отчет и исправления. |
|
@Marashov-Alexander, внес правки. |
Конфликты были зарезолвлены. |
No description provided.