ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ilya Kasnacheev <ilya.kasnach...@gmail.com>
Subject Re: Problem with reading incomplete payload - IGNITE-7153
Date Thu, 10 Jan 2019 15:26:49 GMT
Hello!

I don't see why we can't. Just take it over, run tests, move to PA, etc.

Regards,
-- 
Ilya Kasnacheev


чт, 3 янв. 2019 г. в 17:00, Dmitriy Pavlov <dpavlov@apache.org>:

> Hi Igniters,
>
> I'm trying to reach the author of the fix because the ticket is still in In
> Progress.
>
> Could you please advice me how to handle it (because fix seems to be
> useful)? Can we set Patch Available status by lazy consensus and review
> possibly incomplete fix?
> https://issues.apache.org/jira/browse/IGNITE-7153
>
> Sincerely,
> Dmitriy Pavlov
>
> пт, 2 нояб. 2018 г. в 13:20, Michael Fong <mcfong.open@gmail.com>:
>
> > 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 <yzhdanov@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
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message