[jira] [Commented] (PROTON-961) messenger doesn't handle multi-frame transfers
[ https://issues.apache.org/jira/browse/PROTON-961?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14647929#comment-14647929 ] ASF subversion and git services commented on PROTON-961: Commit 092d41d4453a8f5df0379a3ef87512596153df39 in qpid-proton's branch refs/heads/master from Robert Gemmell [ https://git-wip-us.apache.org/repos/asf?p=qpid-proton.git;h=092d41d ] PROTON-961: fix check for partial delivery, allow multi-frame messages to be received by Messenger Change suggested by Gordon Sim messenger doesn't handle multi-frame transfers -- Key: PROTON-961 URL: https://issues.apache.org/jira/browse/PROTON-961 Project: Qpid Proton Issue Type: Bug Components: proton-c Affects Versions: 0.9.1, 0.10, 0.11 Reporter: Gordon Sim See QPID-6651 for a reproducer. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (PROTON-961) messenger doesn't handle multi-frame transfers
[ https://issues.apache.org/jira/browse/PROTON-961?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14647946#comment-14647946 ] Robbie Gemmell commented on PROTON-961: --- I've checked in Gordons suggested fix. It passes all the existing tests and I verified it fixed the problem by using the 'recv' Messenger example against the Java broker, sending it 2MB messages from the JMS client followed by small messges from the 'send' Messenger example, and checking that the frames on the wire were as expected using a protocol trace. As noted earlier, the incoming capacity starvation issue will only occur if a local max-frame-size is in effect, and Messenger doesnt do that or let you configure it yourself, so the 2MB messages worked fine despite the 1MB session capacity. I tried adding a new test and have given up. You cant use Messenger on both sides, because it wont fragment sent messages unless a remote max framse size is set, and you cant configure it to do that. I then tried using the Reactor/Container bits on one side to get the effect, and couldn't see how to do what I wanted there either. If someone can point a simple way of doing it I'll add one. messenger doesn't handle multi-frame transfers -- Key: PROTON-961 URL: https://issues.apache.org/jira/browse/PROTON-961 Project: Qpid Proton Issue Type: Bug Components: proton-c Affects Versions: 0.9.1 Reporter: Gordon Sim Fix For: 0.10 See QPID-6651 for a reproducer. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (PROTON-961) messenger doesn't handle multi-frame transfers
[ https://issues.apache.org/jira/browse/PROTON-961?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14645871#comment-14645871 ] Robbie Gemmell commented on PROTON-961: --- I'm sure it probably doesnt change the effect, but still worth noting that max frame size MUST be 512 bytes or higher. I don't think the problem you saw necessarily points to an issue with the fix Gordon sugested, but instead to an issue with the engine in general... The problem presumably happens at 1MB because the session defaults to a 1MB 'incoming bytes capacity'. If a remote max-frame-size is set, this capacity is used to calculate the incoming window size as the number of max-sized frames that could be accepted at the time without breaching the 'capacity'. If a remote max-frame-size is not specified, then a fixed window size of 2^31-1 is 'calculated' each time. Proton-j and by the looks of it proton-c both recalculate their session incoming window whenever they are issuing a flow, which amongst other cases they will look to do during a process of the transport if the session incoming window hits 0. The issue is presumably that if the incoming window hits 0 (because a remote max-frame-size was set, and the resulting window has now been used) and the session has also received its capacity of incoming bytes at the same time, then the re-cacluated window will still be 0, something that happens if you send messages over session-capacity in size. In short, if the remote max-frame-size is being set (as it was in the scenario this issue was noticed), then the incoming session window behaviour seems basically broken for messages over session-capacity. If so, this likely hasnt been noticed for the most part because few are setting max-frame-size. messenger doesn't handle multi-frame transfers -- Key: PROTON-961 URL: https://issues.apache.org/jira/browse/PROTON-961 Project: Qpid Proton Issue Type: Bug Components: proton-c Affects Versions: 0.10, 0.11 Reporter: Gordon Sim See QPID-6651 for a reproducer. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (PROTON-961) messenger doesn't handle multi-frame transfers
[ https://issues.apache.org/jira/browse/PROTON-961?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14645887#comment-14645887 ] Robbie Gemmell commented on PROTON-961: --- Took long enough writing my reply / looking at the behaviour that I missed Rafi had already responded :) I agree that the fix seems better than what occurs now, as per above comments the rest seems separate. I think we should apply it before the 0.10 release, and raise a further issue for the 'messages larger than session-capacity while remote max-frame-size is set' issue that can be done at some point after. (Also, would it be possible to reach into messenger and alter the session capacity currently, as a workaround?) messenger doesn't handle multi-frame transfers -- Key: PROTON-961 URL: https://issues.apache.org/jira/browse/PROTON-961 Project: Qpid Proton Issue Type: Bug Components: proton-c Affects Versions: 0.10, 0.11 Reporter: Gordon Sim See QPID-6651 for a reproducer. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (PROTON-961) messenger doesn't handle multi-frame transfers
[ https://issues.apache.org/jira/browse/PROTON-961?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14645998#comment-14645998 ] Ken Giusti commented on PROTON-961: --- Thanks for the analysis - I agree that Gordon's fix is a good start and should be comitted. Haven't thought this through entirely, but for 0.10 would it make sense to add a temp workaround in messenger that automagically bumps the session-incoming_capacity from 1M to some huge number if the max-frame is configured? Would that avoid the session stall for 'reasonable' message sizes? messenger doesn't handle multi-frame transfers -- Key: PROTON-961 URL: https://issues.apache.org/jira/browse/PROTON-961 Project: Qpid Proton Issue Type: Bug Components: proton-c Affects Versions: 0.9.1, 0.10, 0.11 Reporter: Gordon Sim See QPID-6651 for a reproducer. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (PROTON-961) messenger doesn't handle multi-frame transfers
[ https://issues.apache.org/jira/browse/PROTON-961?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14646063#comment-14646063 ] Robbie Gemmell commented on PROTON-961: --- I don't think there would be a need for that in hindsight. I overlooked that this should only have an effect if the Messenger has set a *local* maxFrameSize, which is what I think you changed it to do (with this also influencing the remote max frame size as seen by the other messenger instance). If it doesnt do that (as I don't think it does currently?) then I dont think it should have a problem (unless 2^31-1 transfer frames are recieved without a flow going in the opposite direction on the session), because the advertised incoming window will always be set to 2^31-1 independent of the frame size or number of transfer frames received. messenger doesn't handle multi-frame transfers -- Key: PROTON-961 URL: https://issues.apache.org/jira/browse/PROTON-961 Project: Qpid Proton Issue Type: Bug Components: proton-c Affects Versions: 0.9.1, 0.10, 0.11 Reporter: Gordon Sim See QPID-6651 for a reproducer. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (PROTON-961) messenger doesn't handle multi-frame transfers
[
https://issues.apache.org/jira/browse/PROTON-961?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14642741#comment-14642741
]
Gordon Sim commented on PROTON-961:
---
The pni_pump_in function in messenger.c proceeds to process incoming deliveries
even if they are partial. There *is* a check for pn_delivery_partial at the
start, but I'm not quite sure what it is trying to test there.
The following change seems to fix the problem with large incoming messages and
doesn't break any of the tests. Is it possible the original check was just
written incorrectly?
{noformat}
--- a/proton-c/src/messenger/messenger.c
+++ b/proton-c/src/messenger/messenger.c
@@ -987,7 +987,7 @@ static void pn_condition_report(const char *pfx,
pn_condition_t *condition)
int pni_pump_in(pn_messenger_t *messenger, const char *address, pn_link_t
*receiver)
{
pn_delivery_t *d = pn_link_current(receiver);
- if (!pn_delivery_readable(d) !pn_delivery_partial(d)) {
+ if (!pn_delivery_readable(d) || pn_delivery_partial(d)) {
return 0;
}
{noformat}
messenger doesn't handle multi-frame transfers
--
Key: PROTON-961
URL: https://issues.apache.org/jira/browse/PROTON-961
Project: Qpid Proton
Issue Type: Bug
Components: proton-c
Affects Versions: 0.10, 0.11
Reporter: Gordon Sim
See QPID-6651 for a reproducer.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (PROTON-961) messenger doesn't handle multi-frame transfers
[ https://issues.apache.org/jira/browse/PROTON-961?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14642853#comment-14642853 ] Ken Giusti commented on PROTON-961: --- From my understanding of the code, I'd think your change is the correct fix. There are tests for receiving fragmented messages and large messages in the engine unit tests, but not for messenger. messenger doesn't handle multi-frame transfers -- Key: PROTON-961 URL: https://issues.apache.org/jira/browse/PROTON-961 Project: Qpid Proton Issue Type: Bug Components: proton-c Affects Versions: 0.10, 0.11 Reporter: Gordon Sim See QPID-6651 for a reproducer. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (PROTON-961) messenger doesn't handle multi-frame transfers
[ https://issues.apache.org/jira/browse/PROTON-961?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanelfocusedCommentId=14643071#comment-14643071 ] Ken Giusti commented on PROTON-961: --- Let me retract that - this isn't the correct fix. I tried hacking a copy of messenger to set the max-frame on its connections to 128 bytes. A few of the unit tests start failing with that setting, so I applied Gordon's fix above. His fix prevents a few of the failures, but proton_tests.messenger.MessengerTest.testSendReceive1M fails. Turns out even with that patch Messenger will hang for messages = 1meg. This is caused by the incoming-window closing on the receiver. Since the transfers are partial, messenger never calls pn_link_advance or pn_link_recv, which replenishes the incoming window. messenger doesn't handle multi-frame transfers -- Key: PROTON-961 URL: https://issues.apache.org/jira/browse/PROTON-961 Project: Qpid Proton Issue Type: Bug Components: proton-c Affects Versions: 0.10, 0.11 Reporter: Gordon Sim See QPID-6651 for a reproducer. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
