[jira] [Commented] (DISPATCH-1045) Sometimes close connetion after releasing partial multi-frame messsage
[ https://issues.apache.org/jira/browse/DISPATCH-1045?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16526305#comment-16526305 ] ASF subversion and git services commented on DISPATCH-1045: --- Commit 5d7304f2c4caf206c9304fcbd22be0a972116e3d in qpid-dispatch's branch refs/heads/master from [~ganeshmurthy] [ https://git-wip-us.apache.org/repos/asf?p=qpid-dispatch.git;h=5d7304f ] DISPATCH-1045 - Release the delivery only after the entire message has been received > Sometimes close connetion after releasing partial multi-frame messsage > -- > > Key: DISPATCH-1045 > URL: https://issues.apache.org/jira/browse/DISPATCH-1045 > Project: Qpid Dispatch > Issue Type: Bug > Components: Router Node >Affects Versions: 1.1.0 >Reporter: Alan Conway >Assignee: Ganesh Murthy >Priority: Major > Fix For: 1.2.0 > > > Since DISPATCH-1012 the router releases undeliverable deliveries. > In the case of a multi-frame delivery, it is possible for dispatch to release > it before the entire delivery has been received. Presently dispatch settles > such deliveries and advances its link. That means that if another transfer > for the same delivery arrives, dispatch regards it as a new message with an > invalid delivery-id and closes the connection. Since the release is > asynchronous, there's no way for the client to avoid this possibility. > Worth checking the spec but I suspect we should be dropping the extra data > rather than closing the connection. Need to investigate how this would work > in proton - I think we'd update the delivery without settling it and then > wait for the remote to settle before finally throwing it away. > -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (DISPATCH-1045) Sometimes close connetion after releasing partial multi-frame messsage
[ https://issues.apache.org/jira/browse/DISPATCH-1045?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16525497#comment-16525497 ] ASF GitHub Bot commented on DISPATCH-1045: -- Github user ganeshmurthy commented on a diff in the pull request: https://github.com/apache/qpid-dispatch/pull/328#discussion_r198613623 --- Diff: src/router_node.c --- @@ -312,12 +307,30 @@ static void AMQP_rx_handler(void* context, qd_link_t *link) pn_link_name(pn_link)); } +// +// The entire message has been received and we are ready to consume the delivery by calling pn_link_advance(). +// +pn_link_advance(pn_link); + +// +// The entire message has been received but this message needs to be discarded +// +if (qd_message_is_discard(msg)) { +pn_delivery_update(pnd, qdr_delivery_disposition(delivery)); --- End diff -- Agreed. I have pushed up a new commit to remedy your comment. Please take a quick look. Thanks. > Sometimes close connetion after releasing partial multi-frame messsage > -- > > Key: DISPATCH-1045 > URL: https://issues.apache.org/jira/browse/DISPATCH-1045 > Project: Qpid Dispatch > Issue Type: Bug > Components: Router Node >Affects Versions: 1.1.0 >Reporter: Alan Conway >Assignee: Ganesh Murthy >Priority: Major > Fix For: 1.2.0 > > > Since DISPATCH-1012 the router releases undeliverable deliveries. > In the case of a multi-frame delivery, it is possible for dispatch to release > it before the entire delivery has been received. Presently dispatch settles > such deliveries and advances its link. That means that if another transfer > for the same delivery arrives, dispatch regards it as a new message with an > invalid delivery-id and closes the connection. Since the release is > asynchronous, there's no way for the client to avoid this possibility. > Worth checking the spec but I suspect we should be dropping the extra data > rather than closing the connection. Need to investigate how this would work > in proton - I think we'd update the delivery without settling it and then > wait for the remote to settle before finally throwing it away. > -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (DISPATCH-1045) Sometimes close connetion after releasing partial multi-frame messsage
[ https://issues.apache.org/jira/browse/DISPATCH-1045?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16525462#comment-16525462 ] ASF GitHub Bot commented on DISPATCH-1045: -- Github user ted-ross commented on a diff in the pull request: https://github.com/apache/qpid-dispatch/pull/328#discussion_r198605509 --- Diff: src/router_node.c --- @@ -312,12 +307,30 @@ static void AMQP_rx_handler(void* context, qd_link_t *link) pn_link_name(pn_link)); } +// +// The entire message has been received and we are ready to consume the delivery by calling pn_link_advance(). +// +pn_link_advance(pn_link); + +// +// The entire message has been received but this message needs to be discarded +// +if (qd_message_is_discard(msg)) { +pn_delivery_update(pnd, qdr_delivery_disposition(delivery)); --- End diff -- Is the delivery disposition _always_ set when discard is set? What if the disposition was not set? I think the pn_delivery_update should not be called in this case. > Sometimes close connetion after releasing partial multi-frame messsage > -- > > Key: DISPATCH-1045 > URL: https://issues.apache.org/jira/browse/DISPATCH-1045 > Project: Qpid Dispatch > Issue Type: Bug > Components: Router Node >Affects Versions: 1.1.0 >Reporter: Alan Conway >Assignee: Ganesh Murthy >Priority: Major > Fix For: 1.2.0 > > > Since DISPATCH-1012 the router releases undeliverable deliveries. > In the case of a multi-frame delivery, it is possible for dispatch to release > it before the entire delivery has been received. Presently dispatch settles > such deliveries and advances its link. That means that if another transfer > for the same delivery arrives, dispatch regards it as a new message with an > invalid delivery-id and closes the connection. Since the release is > asynchronous, there's no way for the client to avoid this possibility. > Worth checking the spec but I suspect we should be dropping the extra data > rather than closing the connection. Need to investigate how this would work > in proton - I think we'd update the delivery without settling it and then > wait for the remote to settle before finally throwing it away. > -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (DISPATCH-1045) Sometimes close connetion after releasing partial multi-frame messsage
[ https://issues.apache.org/jira/browse/DISPATCH-1045?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16520743#comment-16520743 ] ASF GitHub Bot commented on DISPATCH-1045: -- Github user codecov-io commented on the issue: https://github.com/apache/qpid-dispatch/pull/328 # [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/328?src=pr=h1) Report > Merging [#328](https://codecov.io/gh/apache/qpid-dispatch/pull/328?src=pr=desc) into [master](https://codecov.io/gh/apache/qpid-dispatch/commit/2e766f35cf2b3a172aaa58cef6c996376e69f1c9?src=pr=desc) will **increase** coverage by `0.1%`. > The diff coverage is `92.85%`. [![Impacted file tree graph](https://codecov.io/gh/apache/qpid-dispatch/pull/328/graphs/tree.svg?src=pr=rk2Cgd27pP=650=150)](https://codecov.io/gh/apache/qpid-dispatch/pull/328?src=pr=tree) ```diff @@Coverage Diff@@ ## master #328 +/- ## = + Coverage 86.52% 86.63% +0.1% = Files 69 69 Lines 1547915494 +15 = + Hits1339313423 +30 + Misses 2086 2071 -15 ``` | [Impacted Files](https://codecov.io/gh/apache/qpid-dispatch/pull/328?src=pr=tree) | Coverage Δ | | |---|---|---| | [src/router\_node.c](https://codecov.io/gh/apache/qpid-dispatch/pull/328/diff?src=pr=tree#diff-c3JjL3JvdXRlcl9ub2RlLmM=) | `94.49% <100%> (+0.06%)` | :arrow_up: | | [src/router\_core/transfer.c](https://codecov.io/gh/apache/qpid-dispatch/pull/328/diff?src=pr=tree#diff-c3JjL3JvdXRlcl9jb3JlL3RyYW5zZmVyLmM=) | `89.96% <80%> (-0.05%)` | :arrow_down: | | [src/router\_core/router\_core.c](https://codecov.io/gh/apache/qpid-dispatch/pull/328/diff?src=pr=tree#diff-c3JjL3JvdXRlcl9jb3JlL3JvdXRlcl9jb3JlLmM=) | `99.25% <0%> (+0.37%)` | :arrow_up: | | [src/router\_core/agent\_link.c](https://codecov.io/gh/apache/qpid-dispatch/pull/328/diff?src=pr=tree#diff-c3JjL3JvdXRlcl9jb3JlL2FnZW50X2xpbmsuYw==) | `63.79% <0%> (+0.57%)` | :arrow_up: | | [src/message.c](https://codecov.io/gh/apache/qpid-dispatch/pull/328/diff?src=pr=tree#diff-c3JjL21lc3NhZ2UuYw==) | `87.34% <0%> (+1.48%)` | :arrow_up: | -- [Continue to review full report at Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/328?src=pr=continue). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://codecov.io/gh/apache/qpid-dispatch/pull/328?src=pr=footer). Last update [2e766f3...e40858d](https://codecov.io/gh/apache/qpid-dispatch/pull/328?src=pr=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments). > Sometimes close connetion after releasing partial multi-frame messsage > -- > > Key: DISPATCH-1045 > URL: https://issues.apache.org/jira/browse/DISPATCH-1045 > Project: Qpid Dispatch > Issue Type: Bug > Components: Router Node >Affects Versions: 1.1.0 >Reporter: Alan Conway >Assignee: Ganesh Murthy >Priority: Major > > Since DISPATCH-1012 the router releases undeliverable deliveries. > In the case of a multi-frame delivery, it is possible for dispatch to release > it before the entire delivery has been received. Presently dispatch settles > such deliveries and advances its link. That means that if another transfer > for the same delivery arrives, dispatch regards it as a new message with an > invalid delivery-id and closes the connection. Since the release is > asynchronous, there's no way for the client to avoid this possibility. > Worth checking the spec but I suspect we should be dropping the extra data > rather than closing the connection. Need to investigate how this would work > in proton - I think we'd update the delivery without settling it and then > wait for the remote to settle before finally throwing it away. > -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org
[jira] [Commented] (DISPATCH-1045) Sometimes close connetion after releasing partial multi-frame messsage
[ https://issues.apache.org/jira/browse/DISPATCH-1045?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16520737#comment-16520737 ] ASF GitHub Bot commented on DISPATCH-1045: -- GitHub user ganeshmurthy opened a pull request: https://github.com/apache/qpid-dispatch/pull/328 DISPATCH-1045 - Release the delivery only after the entire message ha… …s been received You can merge this pull request into a Git repository by running: $ git pull https://github.com/ganeshmurthy/qpid-dispatch DISPATCH-1045 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/qpid-dispatch/pull/328.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #328 commit e40858d7da687e427d624409ba60cb27c1fe0550 Author: Ganesh Murthy Date: 2018-06-22T20:01:51Z DISPATCH-1045 - Release the delivery only after the entire message has been received > Sometimes close connetion after releasing partial multi-frame messsage > -- > > Key: DISPATCH-1045 > URL: https://issues.apache.org/jira/browse/DISPATCH-1045 > Project: Qpid Dispatch > Issue Type: Bug > Components: Router Node >Affects Versions: 1.1.0 >Reporter: Alan Conway >Assignee: Ganesh Murthy >Priority: Major > > Since DISPATCH-1012 the router releases undeliverable deliveries. > In the case of a multi-frame delivery, it is possible for dispatch to release > it before the entire delivery has been received. Presently dispatch settles > such deliveries and advances its link. That means that if another transfer > for the same delivery arrives, dispatch regards it as a new message with an > invalid delivery-id and closes the connection. Since the release is > asynchronous, there's no way for the client to avoid this possibility. > Worth checking the spec but I suspect we should be dropping the extra data > rather than closing the connection. Need to investigate how this would work > in proton - I think we'd update the delivery without settling it and then > wait for the remote to settle before finally throwing it away. > -- This message was sent by Atlassian JIRA (v7.6.3#76005) - To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org For additional commands, e-mail: dev-h...@qpid.apache.org