Hi Yakov, Thanks so much for your analysis.
Parser expects chunks to be complete and has all the data to read entire > message, but this is not guaranteed and single message can arrive in > several chunks. This is indeed the the assumption to my implementation. I have not come up a another parsing algorithm to handle this rainy day case. Perhaps, it would require more refactoring on existing code. In addition, I might need to check how Redis dev implements the parser state machine. I would be interested to see how current implementation (based on 2.6/master) behaves if we intentionally split the message into chunks as you suggested for the reproducer. Regards, Michael On Wed, Oct 31, 2018 at 7:08 PM Yakov Zhdanov <yzhda...@apache.org> wrote: > Hi Mike! > > Thanks for reproducer. Now I understand the problem. NIO worker reads > chunks from the network and notifies the parser on data read. Parser > expects chunks to be complete and has all the data to read entire message, > but this is not guaranteed and single message can arrive in several chunks. > Which is demostrated by your test. > > The problem is inside GridRedisProtocolParser. We should add ability to > store the parsing context if we do not have all the data to complete > message parsing, as it is done, for example in GridBufferedParser. So, it > is definitely an issue and should be fixed by adding parsing state. I see > you attempted to do so in PR > https://github.com/apache/ignite/pull/5044/files. I did not do a formal > review, so let's ask community to review your patch. > > Couple of comments about your reproducer. > > 1. Let's dump a proper Redis message bytes sent by Jedis. > 2. Let's split this dump into 5 chunks and send them with 100 ms delays. > > This should fail before fix is applied, and should pass with proper message > parsed after we have the issue fixed. > > Thanks! > > --Yakov >
