11 Replies Latest reply on Nov 4, 2009 11:17 AM by timfox

    InVM Packet.getPacketSize() != Netty Packet.getPacketSize()

    clebert.suconic

      getPacketSize() will differ between InVM and Netty. It will remove the 4 bytes use for the int size on the Netty, and it would include on Netty.


      That affected LargeMessage flow control.

      I'm changing LargeMessage to use the getRequiredBufferSize instead (the same counter used at the server side).

      I'm also extending ConsumerWindowSizeTest to a NettyConsumerWindowSizeTest. ConsumerWindowSizeTest validates flow control on both regular and large messages.

        • 1. Re: InVM Packet.getPacketSize() != Netty Packet.getPacketSiz
          timfox

          Can you explain in more detail please?

          Even if it differs between netty and invm, the same amount should be taken from server and given back to client, so I don't understand this explanation.

          • 2. Re: InVM Packet.getPacketSize() != Netty Packet.getPacketSiz
            timfox

            BTW the precalculate flow control credits, IIRC, was only added because of the old replication.

            Now we don't replicate sessions, it should no longer be necessary.

            If not, can you explain (in some detail) why not?

            • 3. Re: InVM Packet.getPacketSize() != Netty Packet.getPacketSiz
              clebert.suconic

               

              Can you explain in more detail please?

              Even if it differs between netty and invm, the same amount should be taken from server and given back to client, so I don't understand this explanation.


              I assumed packet.getPacketSize() would always give the.. errr... packet size.. but the Netty implementation differs from InVM. So, the flow control only worked for InVM.

              As soon it started using Netty it was using wrong values for the credit size.


              I''ve fixed by using another method that would be the same on both implementations.


              BTW the precalculate flow control credits, IIRC, was only added because of the old replication.

              Now we don't replicate sessions, it should no longer be necessary.

              If not, can you explain (in some detail) why not?



              No.. it's not...

              That's a Deja Vu actually... you already asked me that same question few months ago:

              http://www.jboss.org/index.html?module=bb&op=viewtopic&p=4238888#4238888

              On our design discussion you were concerned about credits arriving at the same time the delivery code was working, and that would make the channel being over used, so I've implemented the pre-calculation per your request back then.

              It wouldn't be a big deal if you decide to remove it now.



              • 4. Re: InVM Packet.getPacketSize() != Netty Packet.getPacketSiz
                clebert.suconic

                BTW: ConsumerWindowSizeTest was already doing those tests you added on LargeMessage. I've kept those commented out and added a NettyCoinsumerwindowSizeTest now, so this would be tested over both scenarios.

                • 5. Re: InVM Packet.getPacketSize() != Netty Packet.getPacketSiz
                  clebert.suconic

                  Actually, I've opened this thread to discuss why InVM.getPacketSize() != Netty Packet.getPacketSize().

                  That issue still here, and may get someone else later. If we decide to not fix it now (since it's not a big deal).. that's fine.. We all just need to be aware of that.

                  • 6. Re: InVM Packet.getPacketSize() != Netty Packet.getPacketSiz
                    timfox

                    That's not the question I was asking.

                    If we're using netty, then both the client and the server side will be using netty.

                    If we're using invm, then both the client and the server side will be using invm.

                    So.. even if the amount of bytes returned by getPacketSize() differs between netty and invm, then it should be the same on client and server side - so there should be no problem.

                    I suspect what's really happening is you were using a different method on client side to calculate the credits returned to what you were using on the server side.

                    • 7. Re: InVM Packet.getPacketSize() != Netty Packet.getPacketSiz
                      timfox

                      Ok from looking at the code and your changes, the real problem here is not that the getPacketSize() result differs between netty and invm, it's that that, before your fix, the client side was using getPacketSize(), but the server side, for large messages was using getRequiredPacketSize(), and you assumed the results would be the same, which they're not.

                      • 8. Re: InVM Packet.getPacketSize() != Netty Packet.getPacketSiz
                        timfox

                        If the tests are not needed in LargeMessageTest, please remove them, don't just leave them commented out!

                        • 9. Re: InVM Packet.getPacketSize() != Netty Packet.getPacketSiz
                          clebert.suconic

                           

                          "timfox" wrote:
                          Ok from looking at the code and your changes, the real problem here is not that the getPacketSize() result differs between netty and invm, it's that that, before your fix, the client side was using getPacketSize(), but the server side, for large messages was using getRequiredPacketSize(), and you assumed the results would be the same, which they're not.


                          Yes, that's exactly what I was saying..... getPacketSize() is not available before encode/decode so I couldn't use it on the server side, and I was assuming getPacketSize() would be the same on both Netty and InVM.

                          If the tests are not needed in LargeMessageTest, please remove them, don't just leave them commented out!


                          I'm just asking to see if you saw any other reason to keep them. I will remove them.

                          • 10. Re: InVM Packet.getPacketSize() != Netty Packet.getPacketSiz
                            clebert.suconic

                             

                            "timfox" wrote:
                            BTW the precalculate flow control credits, IIRC, was only added because of the old replication.



                            precalculate flow control was added because a conversation on this thread:

                            http://www.jboss.org/index.html?module=bb&op=viewtopic&t=146096&postdays=0&postorder=asc&start=0

                            From the thread:

                            ***

                            9.Flow control - for large message flow control should be called before adding adding to buffer APART from last chunk which should have flow control called before consumption, otherwise could flood the buffer.

                            ***


                            We talked about the code back then and we both agreed it wasn't good, but I couldn't find an easy math formula to the precalulation.



                            If we don't need the precalculation it would be much better anyway.

                            • 11. Re: InVM Packet.getPacketSize() != Netty Packet.getPacketSiz
                              timfox

                              Wow, don't you know when to stop?

                              BTW that comment is referring to the *client* side buffer, so is completely irrelevant.

                              In any case, like I said before I have re-implemented this in a much simpler way, which also guarantees that the delivery thread is not tied up for long periods.

                              Actually I wish I had implemented the whole damn large message code myself, it would have saved a *lot* of time.