HW2 #50
HW2 #50
Conversation
|
Я написал отчет. Однако точки разладки были подобраны, какие-то выводы уже сделаны |
# Conflicts: # src/main/java/ru/vk/itmo/test/kovalevigor/config/DaoServerConfig.java # src/main/java/ru/vk/itmo/test/kovalevigor/server/Server.java # src/main/java/ru/vk/itmo/test/kovalevigor/server/ServiceFactoryImpl.java
| try { | ||
| this.handleRequest(request, session); | ||
| } catch (IOException ioException) { | ||
| session.handleException(ioException); |
There was a problem hiding this comment.
Хорошее решение, еще было бы круто передать в клиентский сокет Connection: close.
Этот хедер может быть интересен proxy и балансировщикам нагрузки, которые могут переиспользовать одно соединение с бэкендом для разных сервисов/клиентов.
There was a problem hiding this comment.
Ну, можно, если у нас еще не побилось там все от предыдущей записи
| public void notAllowed(Request request, HttpSession session) throws IOException { | ||
| session.sendResponse(Responses.NOT_ALLOWED.toResponse()); | ||
| @Override | ||
| public void rejectedExecution(Runnable r, ThreadPoolExecutor executor) { |
There was a problem hiding this comment.
Было бы хорошо добавить запись в лог на такие ситуации, потому что они не являются нормой для приложения, соответственно, хочется быстро замечать такие ситуации, чтобы вовремя масштабировать систему.
There was a problem hiding this comment.
Логи - это долго. Не написал в отчете, но при перегрузе их было видно
There was a problem hiding this comment.
Можно варнинги типа rejected execution раз в N срабатываний писать, чтобы не аффектить приложение
| return Responses.ACCEPTED.toResponse(); | ||
| } | ||
|
|
||
| @Override | ||
| public void close() throws IOException { | ||
| executorService.close(); |
There was a problem hiding this comment.
В прошлом семестре обсуждали, что close может зависнуть надолго. В данном дз как будто нечему зависать, но когда будут range запросы, пожалуй, лучше будет классический awaitTermination, чтобы гарантировать остановку системы за какое-то время.
There was a problem hiding this comment.
Ой, я просто не заметил, что там 1 день ожидания стоит)
|
|
||
| @Override | ||
| public void handleRequest(Request request, HttpSession session) { | ||
| executorService.execute(new Task(request, session)); |
There was a problem hiding this comment.
Предлагаю ловить RejectedExecutionException прямо в этом месте, тогда вместо Task можно использовать обычную лямбду, так как не нужен будет каст. Соответственно, это класс можно совсем убрать.
There was a problem hiding this comment.
Мне показалось, что ошибка дороже, чем этот оверхед
Попрофилирую и узнаю
There was a problem hiding this comment.
Да, что-то я неправильно прочитал. Без исключения должно быть лучше, конечно. rejectedExecution происходит в момент пиковой нагрузки и резкий всплеск исключений может совсем прибить сервер.
Может вынести в другой класс реализацию RejectedExecutionHandler, более декомпозированным код будет.
А еще мне кажется, у вас красиво зайдет паттерн декоратор. Тогда будет чистый класс Server только с логикой обработки запросов и в двух дочерних классах добавится логика close и обработки rejectedExecution.
There was a problem hiding this comment.
Сделал, как Вы предложили.
Единственное, конечно, не понял, как должно получиться 2 класса, если по факту модификация 2 этих методов происходит одновременно
+Пришлось еще всунуть стратегию, т.к. HttpServer вызывает свои внутренние методы
И декоратор там не сильно поможет
| (wrk2 -c 1 -t 1 -L -d $1 -R $2 -s "../lua/$3.lua" http://localhost:8080 > "../../html/${NAME}_wrk.txt") & | ||
| ./ap/bin/asprof -e cpu,alloc -d $1 -f $JFR MainServer | ||
| PREFIX="../../html/stage2" | ||
| (wrk2 -c 64 -t 1 -L -d $1 -R $2 -s "../lua/$3.lua" http://localhost:8080 > "${PREFIX}/${NAME}_wrk.txt") & |
There was a problem hiding this comment.
Предлагаю попробовать также -t с параметром больше 1
|
|
||
| ## Будущее | ||
|
|
||
| Соответственно планирую решить вопрос с отсутствием расширения пула воркеров (пока не знаю как) |
There was a problem hiding this comment.
По CPU профилю выглядит как будто а базе мало данных, так как у воркеров много сэмплов про запись в сессию (это еще может быть есть значения очень объемные) и много сэмплов на взятие задачи из очереди. Вот взятие задачи должно быть быстрым при пиковой нагрузке, иначе селекторы не дорабатывают.
Еще я заметил много времени на GC в профиле, вероятно программа аллоцирует лишнее где-то.
There was a problem hiding this comment.
Ну 20 таблиц, по 45 MB обычно жду
У меня в SSTable при поиске создается нода только с ключом. Может быть в этом проблема. Есть какая-то возможность глянуть конкретнее, что GC собирает?
There was a problem hiding this comment.
Можно по профилю аллокаций посмотреть, где больше всего аллоцируется.
У вас, судя по всему, много копируются мемори сегменты. Так вышло из-за использования стандартного бинпоиска, он делает сначала копирующий ru.vk.itmo.test.kovalevigor.dao.IndexList#get, а потом сравнивает сегменты. При реализации собственного бинпоиска можно было бы использовать java.lang.foreign.MemorySegment#mismatch(java.lang.foreign.MemorySegment, long, long, java.lang.foreign.MemorySegment, long, long), который сравнивает два куска мемори сегмента без доп копирования. У Вадима в референсе так и сделано. Можно проверить, ускорит ли это ваше приложение.
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
| @@ -6,5 +6,5 @@ | |||
| public interface Service { | |||
| CompletableFuture<Void> start() throws IOException; | |||
|
|
|||
| CompletableFuture<Void> stop() throws IOException; | |||
| CompletableFuture<Void> stop() throws Exception; | |||
There was a problem hiding this comment.
Define and throw a dedicated exception instead of using a generic one.
| @@ -6,5 +6,5 @@ | |||
| public interface Service { | |||
| CompletableFuture<Void> start() throws IOException; | |||
|
|
|||
| CompletableFuture<Void> stop() throws IOException; | |||
| CompletableFuture<Void> stop() throws Exception; | |||
There was a problem hiding this comment.
A method/constructor should not explicitly throw java.lang.Exception
| closeSession(session, ioException); | ||
| } catch (Exception exception) { | ||
| Responses response = Responses.INTERNAL_ERROR; | ||
| if (exception instanceof HttpException) { |
There was a problem hiding this comment.
An instanceof check is being performed on the caught exception. Create a separate catch clause for this exception type.
There was a problem hiding this comment.
Почините библиотеку, пожалуйста
| @@ -6,5 +6,5 @@ | |||
| public interface Service { | |||
| CompletableFuture<Void> start() throws IOException; | |||
|
|
|||
| CompletableFuture<Void> stop() throws IOException; | |||
| CompletableFuture<Void> stop() throws Exception; | |||
There was a problem hiding this comment.
A method/constructor should not explicitly throw java.lang.Exception
src/main/java/ru/vk/itmo/test/kovalevigor/server/ServerExecutorStrategyDecorator.java
Show resolved
Hide resolved
|
Не делайте дети домашку в последний день. Поэтому я там сильно даже не старался) |
|
И не merg'ите после проверки. Я удалю свои файлы с репортами |
| } | ||
| SelectorThread[] selectors = httpServer.getSelectors(); | ||
| for (int i = 0; i < selectors.length; i++) { | ||
| executors.put(selectors[i], threadPoolExecutors[i % threadPoolExecutors.length]); |
There was a problem hiding this comment.
Интересный подход. Обращу внимание, что в one-nio есть метод one.nio.server.Server#reconfigure, который пересоздает селектор треды, это будут другие java объекты, поэтому по ним в мапе не будет значений.
Сейчас этот метод reconfigure не используется, поэтому не ошибка.
| @@ -9,4 +9,8 @@ | |||
| public class DaoServerConfig extends HttpServerConfig { | |||
| public Path basePath; | |||
| public long flushThresholdBytes; | |||
| public int corePoolSize = Runtime.getRuntime().availableProcessors(); | |||
| public int maximumPoolSize = Runtime.getRuntime().availableProcessors() * 4; | |||
There was a problem hiding this comment.
core и max сильно отличаются. Это может быть опасно по некоторым причинам, Вадим на паре говорил.
- может не хватить памяти на создание новых тредов
- паттерн, когда тредов в пуле x4, не будет тестироваться на пользователях (потому при обычной нагрузке будет ровно core потоков), поэтому при пиковой нагрузке могут произойти неожиданные спецэффекты.
| ); | ||
| } | ||
| SelectorThread[] selectors = httpServer.getSelectors(); | ||
| for (int i = 0; i < selectors.length; i++) { |
There was a problem hiding this comment.
Выглядит как будто оптимально при числе селекторов кратном числу тред пулов. Если селекторов меньше, то часть пулов будут простаивать. Если не кратно, то может быть неравномерное распределение нагрузки на пулы, но это будет заметно в профиле с разбивкой по тредам, что один столбик больше других.
У вас, судя по профилям, все хорошо получилось, но, возможно, стоит валидацию добавить или просто иметь в виду.
| [lock](1709758089_lock.html), | ||
| [rps](1709758089_wrk.txt) | ||
|
|
||
| Локи в селекторах теперь незначительно, но тольку то ;) |
There was a problem hiding this comment.
Классно, что получилось соптимизировать локи. То что это не дало прироста производительности, означает, что очередь не являлась узким местом программы.
При этом полученный вами опыт довольно полезен. Например, если мы жестко привяжем один тред пул на один селектор тред, то в качестве очереди можно использовать single producer single consumer, которая скорее всего будет работать быстрее (но наше приложение опять же вряд ли ускорит). А еще такие очереди проще сделать wait-free, что может быть полезно для hard realtime систем.
Окей, перед парой в четверг сяду мерджить все проверенное. |
* add dao * fix dependencies * simple server * create all routes * fix cringe * tests * update dao to last version :) * add scripts * partly stage1 * vivods * мега вывод * code climate * new ideas * hw2 * fix codeclimate * code refactor * gg wp * refactor reports * hw2 --------- Co-authored-by: Alexey Shik <58121508+AlexeyShik@users.noreply.github.com>
Игорь Ковалев Константинович M33361