[jboss-dev-forums] [Design of Messaging on JBoss (Messaging/JBoss)] - Re: Large message code review

[timfox]

Reviewing the latest state of trunk, it seems:

1) is done

2) is outstanding

3) is done

4) is done

5) is NOT done:

  |  sendMessageInChunks(true, msg);
  | 

It's always called with true!

6) tests outstanding

7) is done

8) is done

9) Still has issues:


  |    public synchronized void handleLargeMessage(final byte[] header) throws Exception
  |    {
  |       if (closing)
  |       {
  |          // This is ok - we just ignore the message
  |          return;
  |       }
  | 
  |       // Flow control for the first packet, we will have others
  |       flowControl(header.length); *** WRONG**
  | 
  |       currentChunkMessage = createFileMessage(header);
  |    }
  | 

The flow control number of bytes must be the packet size which is not the same as the header.length!!

I can also see you're not using the packet size for to reduce the credits in the serverconsumerimpl. Packet size MUST be used.

Flow control size also seems be calculated wrong in other places.

Other issues:


  |    /** Size used for FlowControl */
  |    int getFlowControlSize();
  |    
  |    /** Size used for FlowControl */
  |    void setFlowControlSize(int flowControlSize);
  | 

This methods should NOT be on the public API, they are for internal use only, not for use by users!!

Another issue:

ClientMessage::setFlowControlSize is only being called for large messages. So for normal messages it's always zero, meaning no credits are sent to the server............. ouch ouch

not good


View the original post : http://www.jboss.com/index.html?module=bb&op=viewtopic&p=4195606#4195606

Reply to the post : http://www.jboss.com/index.html?module=bb&op=posting&mode=reply&p=4195606