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:
[email protected]
With regards,
Apache Git Services
