-
Notifications
You must be signed in to change notification settings - Fork 47
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
HW1 Чеботин Александр (ITMO DWS) #2
Conversation
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.
The PR diff size of 7192 lines exceeds the maximum allowed for the inline comments feature.
Тест один флакает почему-то |
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.
The PR diff size of 7155 lines exceeds the maximum allowed for the inline comments feature.
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.
The PR diff size of 7152 lines exceeds the maximum allowed for the inline comments feature.
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.
The PR diff size of 7152 lines exceeds the maximum allowed for the inline comments feature.
Значит, есть баг. Какие есть идеи? 11:06:44.653 [NIO Selector #11] ERROR one.nio.net.Session -- Cannot process session from 127.0.0.1
java.lang.IndexOutOfBoundsException: Out of bound access on segment MemorySegment{ heapBase: Optional[[B@7885006c] address:0 limit: 16 }; new offset = 128; new length = 8
at java.base/jdk.internal.foreign.AbstractMemorySegmentImpl.outOfBoundException(AbstractMemorySegmentImpl.java:426)
at java.base/jdk.internal.foreign.AbstractMemorySegmentImpl.apply(AbstractMemorySegmentImpl.java:407)
at java.base/jdk.internal.foreign.AbstractMemorySegmentImpl.apply(AbstractMemorySegmentImpl.java:69)
at java.base/jdk.internal.util.Preconditions.outOfBounds(Preconditions.java:98)
at java.base/jdk.internal.util.Preconditions.outOfBoundsCheckIndex(Preconditions.java:124)
at java.base/jdk.internal.util.Preconditions.checkIndex(Preconditions.java:448)
at java.base/jdk.internal.foreign.AbstractMemorySegmentImpl.checkBounds(AbstractMemorySegmentImpl.java:396)
at java.base/jdk.internal.foreign.AbstractMemorySegmentImpl.checkAccess(AbstractMemorySegmentImpl.java:356)
at java.base/java.lang.invoke.VarHandleSegmentAsLongs.checkAddress(VarHandleSegmentAsLongs.java:81)
at java.base/java.lang.invoke.VarHandleSegmentAsLongs.get(VarHandleSegmentAsLongs.java:108)
at ru.vk.itmo.test.chebotinalexandr.dao.MurmurHash.getBlock(MurmurHash.java:153)
at ru.vk.itmo.test.chebotinalexandr.dao.MurmurHash.hash64(MurmurHash.java:51)
at ru.vk.itmo.test.chebotinalexandr.dao.BloomFilter.sstableMayContain(BloomFilter.java:58)
at ru.vk.itmo.test.chebotinalexandr.dao.NotOnlyInMemoryDao.getFromDisk(NotOnlyInMemoryDao.java:107)
at ru.vk.itmo.test.chebotinalexandr.dao.NotOnlyInMemoryDao.get(NotOnlyInMemoryDao.java:100)
at ru.vk.itmo.test.chebotinalexandr.dao.NotOnlyInMemoryDao.get(NotOnlyInMemoryDao.java:32)
at ru.vk.itmo.test.chebotinalexandr.StorageServer.get(StorageServer.java:65)
at RequestHandler0_get.handleRequest(Unknown Source)
at one.nio.http.HttpServer.handleRequest(HttpServer.java:66)
at one.nio.http.HttpSession.handleParsedRequest(HttpSession.java:137)
at one.nio.http.HttpSession.processHttpBuffer(HttpSession.java:198)
at one.nio.http.HttpSession.processRead(HttpSession.java:77)
at one.nio.net.Session.process(Session.java:222)
at one.nio.server.SelectorThread.run(SelectorThread.java:70)
java.io.IOException: HTTP/1.1 header parser received no bytes
at java.net.http/jdk.internal.net.http.HttpClientImpl.send(HttpClientImpl.java:964)
at java.net.http/jdk.internal.net.http.HttpClientFacade.send(HttpClientFacade.java:133)
at ru.vk.itmo.ServiceInfo.get(ServiceInfo.java:38)
at ru.vk.itmo.SingleNodeTest.restart(SingleNodeTest.java:233)
at java.base/java.lang.reflect.Method.invoke(Method.java:580)
at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184)
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:179)
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184)
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:184)
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1708)
at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:276)
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:197)
at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1708)
at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
at java.base/java.util.stream.ReferencePipeline$7$1.accept(ReferencePipeline.java:276)
at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1708)
at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:509)
at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:499)
at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:151)
at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:174)
at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:596)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
Caused by: java.io.IOException: HTTP/1.1 header parser received no bytes
at java.net.http/jdk.internal.net.http.common.Utils.wrapWithExtraDetail(Utils.java:388)
at java.net.http/jdk.internal.net.http.Http1Response$HeadersReader.onReadError(Http1Response.java:590)
at java.net.http/jdk.internal.net.http.Http1AsyncReceiver.checkForErrors(Http1AsyncReceiver.java:302)
at java.net.http/jdk.internal.net.http.Http1AsyncReceiver.flush(Http1AsyncReceiver.java:268)
at java.net.http/jdk.internal.net.http.common.SequentialScheduler$LockingRestartableTask.run(SequentialScheduler.java:182)
at java.net.http/jdk.internal.net.http.common.SequentialScheduler$CompleteRestartableTask.run(SequentialScheduler.java:149)
at java.net.http/jdk.internal.net.http.common.SequentialScheduler$SchedulableTask.run(SequentialScheduler.java:207)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
at java.base/java.lang.Thread.run(Thread.java:1583)
Caused by: java.io.EOFException: EOF reached while reading
at java.net.http/jdk.internal.net.http.Http1AsyncReceiver$Http1TubeSubscriber.onComplete(Http1AsyncReceiver.java:601)
at java.net.http/jdk.internal.net.http.SocketTube$InternalReadPublisher$ReadSubscription.signalCompletion(SocketTube.java:648)
at java.net.http/jdk.internal.net.http.SocketTube$InternalReadPublisher$InternalReadSubscription.read(SocketTube.java:853)
at java.net.http/jdk.internal.net.http.SocketTube$SocketFlowTask.run(SocketTube.java:181)
at java.net.http/jdk.internal.net.http.common.SequentialScheduler$SchedulableTask.run(SequentialScheduler.java:207)
at java.net.http/jdk.internal.net.http.common.SequentialScheduler.runOrSchedule(SequentialScheduler.java:280)
at java.net.http/jdk.internal.net.http.common.SequentialScheduler.runOrSchedule(SequentialScheduler.java:233)
at java.net.http/jdk.internal.net.http.SocketTube$InternalReadPublisher$InternalReadSubscription.signalReadable(SocketTube.java:782)
at java.net.http/jdk.internal.net.http.SocketTube$InternalReadPublisher$ReadEvent.signalEvent(SocketTube.java:965)
at java.net.http/jdk.internal.net.http.SocketTube$SocketFlowEvent.handle(SocketTube.java:253)
at java.net.http/jdk.internal.net.http.HttpClientImpl$SelectorManager.handleEvent(HttpClientImpl.java:1467)
at java.net.http/jdk.internal.net.http.HttpClientImpl$SelectorManager.lambda$run$3(HttpClientImpl.java:1412)
at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
at java.net.http/jdk.internal.net.http.HttpClientImpl$SelectorManager.run(HttpClientImpl.java:1412) |
Благодаря тесту выявился баг в моей реализации Dao, о существовании которого в прошлом семестре я даже не знал. Исправлю |
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.
The PR diff size of 5752 lines exceeds the maximum allowed for the inline comments feature.
Нужно пофиксить замечания от CodeClimate. |
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.
The PR diff size of 5751 lines exceeds the maximum allowed for the inline comments feature.
А можно ли отключить замечание CodeClimate на fall-through? В реализации MurmurHash есть одно из редких применений fall-through внутри switch конструкции. |
Да, попробуете отключить в рамках этого PR через |
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.
The PR diff size of 5778 lines exceeds the maximum allowed for the inline comments feature.
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.
The PR diff size of 5777 lines exceeds the maximum allowed for the inline comments feature.
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.
The PR diff size of 5798 lines exceeds the maximum allowed for the inline comments feature.
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.
The PR diff size of 5803 lines exceeds the maximum allowed for the inline comments feature.
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.
The PR diff size of 5808 lines exceeds the maximum allowed for the inline comments feature.
Да, отключил |
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.
The PR diff size of 5808 lines exceeds the maximum allowed for the inline comments feature.
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.
Есть замечания к коду и вопросы к анализу результатов.
С другой стороны здорово, что поэкспериментировали с bloom filter и привели результаты.
В итоге 13 баллов.
function request() | ||
headers = { } | ||
headers["Host"] = "localhost:8080" | ||
return wrk.format("GET", "/hello", headers) |
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.
Это для чего?
} | ||
|
||
private static List<Integer> getRandomArray() { | ||
ArrayList<Integer> entries = new ArrayList<>(ENTRIES_IN_DB); |
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.
int[]
был бы сильно компактнее.
if (id == null || id.isEmpty()) { | ||
return new Response(Response.BAD_REQUEST, Response.EMPTY); | ||
} |
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.
Этот код дублируется 3 раза.
return new Response(Response.BAD_REQUEST, Response.EMPTY); | ||
} | ||
|
||
Entry<MemorySegment> entry = dao.get(fromString(id)); |
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.
Здесь и ниже не хватает обработки возможных исключений из DAO.
} | ||
|
||
@Path(PATH) | ||
public Response post(@Param("id") String id) { |
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.
Не только POST
же? Есть много других интересных HTTP методов...
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.
Это да) просто название метода неудачное, а так при любом http запросе отличном от GET/PUT/DELETE запрос обработается в этом методе, в том числе для OPTIONS, PATCH, HEAD…
|
||
``` | ||
|
||
Гистограмма, на которой видна деградация с течением времени: |
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.
Это же распределение времён ответа. Что значит "с течением времени"?
``` | ||
 | ||
|
||
Видим что среднее Latency - 0.81 ms. |
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.
Как мы обсуждали на занятиях, вместо среднего правильнее смотреть на перцентили.
|
||
Detailed Percentile spectrum тут не нужен, и так видно, что база просто моментально захлебнулась таким количеством бинарных поисков. | ||
|
||
Снижением rps была определена точка разладки, в этом случае она составляет 10k rps. Возьмем стабильную 7.5k rps: |
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.
Откуда 10К rps? В листинге выше выдержали только 7.2К rps.
|
||
 | ||
|
||
Видно что много аллокаций приходится на SSTablesStorage.write - это как раз сброс данных с memtable на диск. |
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.
На flamegraph видно, что почти все аллокации при этом возникают в SSTablesStorage.getPaths()
-- можно их устранить? Они действительно необходимы?
|
||
 | ||
|
||
Ярко выраженная затрата времени на JIT-компиляцию, что вызывает задержку. Также видим задержку увеличивает также flush (метод SSTablesStorage.write). |
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.
Здесь неправильно говорить о "задержке" -- мы видим, на что тратятся циклы процессора. Почему упоминается JIT, но нет ни слова про "слона" -- как и выше SSTablesStorage.getPaths()
. Можно его соптимизировать? Действительно ли необходимо на каждый запрос тратить столько CPU на построение путей, которые почти не меняются?
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.
The PR diff size of 5808 lines exceeds the maximum allowed for the inline comments feature.
* stage2 * fix * codeclimate * codeclimate #2 * fixes * report * graceful shutdown --------- Co-authored-by: Roman Mushchinskii <31738033+urbanchef@users.noreply.github.com>
* stage2 * fix * codeclimate * codeclimate polis-vk#2 * fixes * report * graceful shutdown --------- Co-authored-by: Roman Mushchinskii <31738033+urbanchef@users.noreply.github.com>
* stage2 * fix * codeclimate * codeclimate #2 * fixes * report * graceful shutdown * stage3 * style * style * style * style * style * style * style * fixes * report * style * stage 4 * style * style * style * report --------- Co-authored-by: Vadim Tsesko <incubos@users.noreply.github.com> Co-authored-by: atimofeyev <andrey.timofeyev@gmail.com>
No description provided.