Решетников Алексей, ИТМО DWS, stage 5 #193
Conversation
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
| try { | ||
| return future.get(); | ||
| } catch (InterruptedException | ExecutionException e) { | ||
| LOGGER.error("Got error while redirecting request: {}", e.getMessage()); |
There was a problem hiding this comment.
для InterruptedException следует заново проставлять флаг interrupted для треда
| private List<Response> extractFuturesAndGetNotErrorResponses(List<CompletableFuture<Response>> futures) { | ||
| List<Response> responses = futures.stream().map(future -> { | ||
| try { | ||
| return future.get(); |
There was a problem hiding this comment.
Гарантируется ли во время future.get() завершение всех CompletableFuture в списке? Если нет, то вызов get() будет блокирующим. Лучше работать через неблокирующие коллбэки
| return httpClient.sendAsync(redirectRequest, HttpResponse.BodyHandlers.ofByteArray()) | ||
| .thenApplyAsync(resp -> { | ||
| Response response = new Response(Integer.toString(resp.statusCode()), resp.body()); | ||
| long timestamp = resp.headers().firstValueAsLong(HTTP_TIMESTAMP_HEADER_NAME).orElse(0); |
There was a problem hiding this comment.
Почему orElse(0)? В каких случаях заголовок HTTP_TIMESTAMP_HEADER_NAME может отсутствовать здесь?
There was a problem hiding this comment.
В штатном режиме отсутствие заголовка исключено.
Так как при процесс получения значения из хедера по ключу подразумевает работу с Optional<>, то нужно обработать каким-либо образом отсутствие значения по данному заголовку. Думаю, тут можно считать, что отсутствие хедера будет считаться, что данные как минимум не внушают доверия. Если же в ответе другой ноды найдется респонс с хедером, то он будет считаться более надежным, чем ответ без хедера
There was a problem hiding this comment.
Также можно было бы кидать эксепшн, если хедера нет, но в этом случае потенциально можем потерять данные
| 90.000% | 7.56s | 1.13s | ||
| 99.000% | 7.82s | 1.50s | ||
| 99.900% | 7.86s | 1.62s | ||
| 99.990% | 7.88s | 1.64s |
There was a problem hiding this comment.
Секунды в перцентилях - это очень много!
There was a problem hiding this comment.
тут ведь прогон с аналогичными с 4 лабой настройками wrk
соответственно, главное здесь показать, что, с улучшением кода, производительность увеличилась
|
|
||
| По числам при `GET` примерно такая же картина, как и `PUT` | ||
|
|
||
| ## Вывод |
There was a problem hiding this comment.
А что произошло с блокировками? Как изменился профиль блокировок? На чем теперь блокируемся? Появилось ли что-то новое? Если да, как можно уменьшить или избавиться вообще?
Отчет не полный.
Что сделано
RequestRouter.redirectтеперь возвращаетCompletableFuture<Response>, ответ из которого будет вытаскиваться в момент обработки и отправки ответа на запрос (в методеHttpServerImpl.extractFuturesAndGetNotErrorResponseslocalExecutor, который подхватит запросы на обработку бизнес логики (методHttpServerImpl.invokeLocal). Это нужно для того, чтобы поток не блокировался на обработку бизнес логики в методеHttpServiceImpl.sendRequestsAndGetResponses