You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by GitBox <gi...@apache.org> on 2020/04/14 00:30:22 UTC

[GitHub] [activemq-artemis] clebertsuconic opened a new pull request #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages

clebertsuconic opened a new pull request #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages
URL: https://github.com/apache/activemq-artemis/pull/3079
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] clebertsuconic commented on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages
URL: https://github.com/apache/activemq-artemis/pull/3079#issuecomment-613162170
 
 
   @gemmellr This is fixing an issue you raised.. if you could review it please?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] tabish121 commented on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages

Posted by GitBox <gi...@apache.org>.
tabish121 commented on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages
URL: https://github.com/apache/activemq-artemis/pull/3079#issuecomment-613676103
 
 
   I think worst case on Transfer without accounting for delivery tag and delivery state is something like 31 bytes (quick back of napkin calc).  broker tag uses a long counter I think so max 8 bytes plus extra couple for Binary encoding.  I don't recall any case where the broker adds a delivery state so guess you don't need to add padding for that.  Safe gut estimate would be 64 bytes if you didn't want to try and squeeze it down to optimistic encoding size.  

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] michaelandrepearce commented on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages

Posted by GitBox <gi...@apache.org>.
michaelandrepearce commented on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages
URL: https://github.com/apache/activemq-artemis/pull/3079#issuecomment-613867488
 
 
   50 seems to be a bit of a magic number... it be good to understand the calculation of why 50.  I feel uneasy that this is just a number picked out the air. I assume we know the tags and length and thus be more scientific. If anything 64 sounded better because at least its a power of 2...

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages

Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages
URL: https://github.com/apache/activemq-artemis/pull/3079#discussion_r408038172
 
 

 ##########
 File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/ProtonServerSenderContext.java
 ##########
 @@ -862,7 +862,9 @@ void deliver() {
 
                   sender.send(buf.array(), 0, size);
 
-                  connection.instantFlush();
+                  if (protonSession.session.getOutgoingBytes() >= protonSession.session.getConnection().getTransport().getOutboundFrameSizeLimit()) {
+                     connection.instantFlush();
+                  }
 
 Review comment:
   I dont think this makes the most sense as there isnt anything to say there arent bytes for other messages in the session, so using that to govern behaviour for a particular delivery isnt a reliable way to achieve the desired behaviour.
   
   The delivery has its own buffer, using that to govern behaviour would seem the most appropriate, see Delivery.pending() and available() (which both do the same thing hehe, https://issues.apache.org/jira/browse/PROTON-1409).
   
   Just checking the brokers outbound size limit doesnt allow for the fact the receivers frame size limit may be lower than it, so it may be worth calculating the lower of the two up front, otherwise it may never flush here. Similarly, the return slightly higher up may mean it returns without flushing following a change like this.
   
   The existing flush here will presumably also be why ARTEMIS-2707 occurred, since the delivery isnt indicated complete (sender.advance()) or settled (which implicitly completes) until further down just before another flush, yet would have already had all its payload sent at this point. With the addition of an 'if outstanding is over frame size' guard here it would become less certain whether the flush and subsquent empty transfer occurred however, depending then on whether the threshhold was exceeded after the final send.
   
   I'd note that by creating such a small buffer, you are forcing a similarly small allocation on every send call as the payload is copied into the delivery. Using a bigger buffer would seem sensible when appropriate. Perhaps the message size can be discerned and something suitable selected based on that and the frame sizing. Or also using NettyReadable and sendNoCopy to avoid the copy.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] clebertsuconic commented on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages
URL: https://github.com/apache/activemq-artemis/pull/3079#issuecomment-613676799
 
 
   ok, I will do what Robbie suggested me.. 50 + tag.length
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] gemmellr commented on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages

Posted by GitBox <gi...@apache.org>.
gemmellr commented on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages
URL: https://github.com/apache/activemq-artemis/pull/3079#issuecomment-613669789
 
 
   There isnt a constant, as the encoded size of the transfer performative varies depending on what fields are populated in it, their precise values (since types can vary in size depending on specific value being encoded), etc.
   
   For a different-but-related case, rhea simply deducts 50 bytes and the length of the delivery tag when calculating a max transfer payload size (https://github.com/amqp/rhea/blob/1.0.20/lib/session.js#L137), you could do something similar.
   
   As I noted before, you are just looking at the brokers configured outgoing limit (which is just set the same as its local max frame size). The receivers max frame size may be lower. That mattered with the earlier changes as it could have stopped it flushing, but here the engine will just frame the content however it needs to.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] clebertsuconic commented on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages
URL: https://github.com/apache/activemq-artemis/pull/3079#issuecomment-613561286
 
 
   Pr is updated.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages

Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages
URL: https://github.com/apache/activemq-artemis/pull/3079#discussion_r408038172
 
 

 ##########
 File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/ProtonServerSenderContext.java
 ##########
 @@ -862,7 +862,9 @@ void deliver() {
 
                   sender.send(buf.array(), 0, size);
 
-                  connection.instantFlush();
+                  if (protonSession.session.getOutgoingBytes() >= protonSession.session.getConnection().getTransport().getOutboundFrameSizeLimit()) {
+                     connection.instantFlush();
+                  }
 
 Review comment:
   I dont think this makes the most sense as there isnt anything to say there arent bytes for other messages in the session, so using that to govern behaviour for a particular delivery isnt a reliable way to achieve the desired behaviour.
   
   The delivery has its own buffer, using that to govern behaviour would seem the most appropriate, see Delivery.pending() and available() (which both do the same thing hehe, https://issues.apache.org/jira/browse/PROTON-1409).
   
   Just checking the brokers outbound size limit doesnt allow for the fact the receivers frame size limit may be lower than it, so it may be worth calculating the lower of the two up front, otherwise it may never flush here. Similarly, the return slightly higher up may mean it returns without flushing following a change like this.
   
   The existing flush here will presumably also be why ARTEMIS-2707 occurred, since the delivery isnt indicated complete (sender.advance()) or settled (which implicitly completes) until further down just before another flush, yet would have already had all its payload sent at this point. With the addition of an 'if outstanding is over frame size' guard here it would become less certain whether the flush and subsquent empty transfer occurred however, depending then on whether the threshhold was exceeded after the final send.
   
   I'd note that by creating such a small buffer, you are forcing a similarly small allocation on every send call as the payload is copied into the delivery. Using a bigger buffer would seem sensible when appropriate. Perhaps the message size can be discerned and something suitable selected based on that and the frame sizing. Or also using NettyReadable and sendNoCopy to avoid the copy (note though that per its API, sendNoCopy can/will still copy if there is outstanding data from a prior send call, hence passing a more appropriately sized buffer makes sense there too).
   
   In terms of sizing on send, it may be worth allowing for some room for the frame header and transfer encoding, such that by supplying a little less than [a multiple of] the max you end up avoiding emitting a 'full' frame followed by a tiny one just to complete sending some small remaining content, then another full one and tiny one for subsequent send call content.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] tabish121 commented on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages

Posted by GitBox <gi...@apache.org>.
tabish121 commented on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages
URL: https://github.com/apache/activemq-artemis/pull/3079#issuecomment-613656867
 
 
   The change doesn't account for the Transfer performative encoded in front of the payload as was mentioned in Robbie's comment so you end up now with an odd distribution of frames where you get one big on and then one tiny one which can be observed in the AMQP large message test by turning on frame tracing.  
   
   > [269870574:0] <- Transfer{handle=0, deliveryId=199, deliveryTag=\x00, messageFormat=0, settled=false, more=true, rcvSettleMode=null, state=null, resume=false, aborted=false, batchable=false} (32744) "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"...(truncated)
   > [269870574:0] <- Transfer{handle=0, deliveryId=199, deliveryTag=\x00, messageFormat=0, settled=false, more=true, rcvSettleMode=null, state=null, resume=false, aborted=false, batchable=false} (23) "AAAAAAAAAAAAAAAAAAAAAAA"
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] gemmellr commented on a change in pull request #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages

Posted by GitBox <gi...@apache.org>.
gemmellr commented on a change in pull request #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages
URL: https://github.com/apache/activemq-artemis/pull/3079#discussion_r408038172
 
 

 ##########
 File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/ProtonServerSenderContext.java
 ##########
 @@ -862,7 +862,9 @@ void deliver() {
 
                   sender.send(buf.array(), 0, size);
 
-                  connection.instantFlush();
+                  if (protonSession.session.getOutgoingBytes() >= protonSession.session.getConnection().getTransport().getOutboundFrameSizeLimit()) {
+                     connection.instantFlush();
+                  }
 
 Review comment:
   I dont think this makes the most sense as there isnt anything to say there arent bytes for other messages in the session, so using that to govern behaviour for a particular delivery isnt a reliable way to achieve the desired behaviour.
   
   The delivery has its own buffer, using that to govern behaviour would seem the most appropriate, see Delivery.pending() and available() (which both do the same thing hehe, https://issues.apache.org/jira/browse/PROTON-1409).
   
   Just checking the brokers outbound size limit doesnt allow for the fact the receivers frame size may be lower than it, so it may be worth calculating the lower of the two, otherwise it may never flush here. Similarly, the return slightly higher up may mean it returns without flushing.
   
   The flush here will presumably also be why ARTEMIS-2707 occurred, since the delivery isnt indicated complete (sender.advance()) or settled (which implicitly completes) until further down just before another flush, yet would have already had all its payload sent at this point. With the addition of an 'if outstanding is over frame size' guard here it would become less certain whether the flush and subsquent empty transfer occurred however, depending then on whether the threshhold was exceeded after the last send.
   
   I'd note that by creating such a small buffer, you are forcing a similarly small allocation on every send call as the payload is copied into the delivery. Using a bigger buffer would seem sensible when appropriate. Perhaps the message size can be discerned and something suitable selected based on that and the frame sizing. Or also using NettyReadable and sendNoCopy to avoid the copy.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] clebertsuconic commented on a change in pull request #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on a change in pull request #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages
URL: https://github.com/apache/activemq-artemis/pull/3079#discussion_r408140958
 
 

 ##########
 File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/proton/ProtonServerSenderContext.java
 ##########
 @@ -862,7 +862,9 @@ void deliver() {
 
                   sender.send(buf.array(), 0, size);
 
-                  connection.instantFlush();
+                  if (protonSession.session.getOutgoingBytes() >= protonSession.session.getConnection().getTransport().getOutboundFrameSizeLimit()) {
+                     connection.instantFlush();
+                  }
 
 Review comment:
   
   I wanted to avoid a bigger buffer. I will use the frame size as the buffer size...
   
   Making an update

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] michaelandrepearce edited a comment on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages
URL: https://github.com/apache/activemq-artemis/pull/3079#issuecomment-613867488
 
 
   50 seems to be a bit of a magic number... it be good to understand the calculation of why 50.  I feel uneasy that this is just a number picked out the air. I assume we know the tags and length and thus be more scientific. If anything and it does have to be a estimated @tabish121 suggested 64 sounded better because at least its a power of 2... 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] asfgit merged pull request #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages

Posted by GitBox <gi...@apache.org>.
asfgit merged pull request #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages
URL: https://github.com/apache/activemq-artemis/pull/3079
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] michaelandrepearce edited a comment on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages
URL: https://github.com/apache/activemq-artemis/pull/3079#issuecomment-613867488
 
 
   50 seems to be a bit of a magic number... it be good to understand the calculation of why 50.  I feel uneasy that this is just a number picked out the air. I assume we know the tags and length and thus be more scientific. If anything and it does have to be a estimated @tabish121 suggested 64 sounded better because at least its a power of 2...  

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] clebertsuconic commented on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages
URL: https://github.com/apache/activemq-artemis/pull/3079#issuecomment-613669693
 
 
   I can discount 23 (or make 25) from the body size I'm writing... 
   it should be fine as long as I'm not writing more than one frame.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] clebertsuconic commented on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages

Posted by GitBox <gi...@apache.org>.
clebertsuconic commented on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages
URL: https://github.com/apache/activemq-artemis/pull/3079#issuecomment-613662778
 
 
   @tabish121 so, what should I do, create the buffer size - than the payload?
   
   is there a constant I can use?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] gemmellr commented on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages

Posted by GitBox <gi...@apache.org>.
gemmellr commented on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages
URL: https://github.com/apache/activemq-artemis/pull/3079#issuecomment-613671058
 
 
   23 isnt going to be enough, its fitting here as its the very start of the delivery activity, but the transfer size will increase once the delivery tag grows beyond the 1 byte value used in this case, or the deliveryId grows and needs more bytes to encode, or more fields are used in the transfer, etc.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] gemmellr commented on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages

Posted by GitBox <gi...@apache.org>.
gemmellr commented on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages
URL: https://github.com/apache/activemq-artemis/pull/3079#issuecomment-613995896
 
 
   50 (+tag length) is a essentially magic round number, 'suitably greater' than the expected max size the frame and performative encodings will be (which can change, and cant be determined exactly without information unavailable at that point).
   
   Using a power of 2 doesnt really make a difference here (except being bigger and so making the resulting frame further from max possible). The value its being deducted from may or may not be, and the result almost always wont be. If we suitably cross-referenced the usage of expected tag lenghts it could be used to avoid checking that though.
   
   To be clear, theres no scope for a correctness issue becuase its not big enough or a power of 2, the only consideration is that if too small a number is used, the frame+performative may exceed that and cause an additional very small frame to be sent.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] michaelandrepearce edited a comment on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages

Posted by GitBox <gi...@apache.org>.
michaelandrepearce edited a comment on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages
URL: https://github.com/apache/activemq-artemis/pull/3079#issuecomment-613867488
 
 
   50 seems to be a bit of a magic number... it be good to understand the calculation of why 50.  I feel uneasy that this is just a number picked out the air. I assume we know the tags and length and thus be more scientific. If anything and it does have to be a estimated 64 sounded better because at least its a power of 2... 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [activemq-artemis] gemmellr edited a comment on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages

Posted by GitBox <gi...@apache.org>.
gemmellr edited a comment on issue #3079: ARTEMIS-2706 Use FrameSize to decide when to flush large messages
URL: https://github.com/apache/activemq-artemis/pull/3079#issuecomment-613995896
 
 
   50 (+tag length) is a essentially magic round number, 'suitably greater' than the expected max size the frame and performative encodings will be (which can change, and cant be determined exactly without information unavailable at that point).
   
   Using a power of 2 doesnt really make a difference here (except being bigger and so making the resulting frame further from max possible). The value its being deducted from may or may not be, and the result almost always wont be.
   
   If we suitably cross-referenced the usage of expected tag lengths in the code / tag generator, a larger value here could perhaps be used to avoid even inspecting that though.
   
   To be clear, theres no scope for a correctness issue becuase its not big enough or a power of 2, the only consideration is that if too small a number is used, the frame+performative may exceed that and cause an additional very small frame to be sent with the remainder of the payload given.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services