[GitHub] qpid-dispatch pull request #328: DISPATCH-1045 - Release the delivery only a...

2018-06-27 Thread ganeshmurthy
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.


---

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[GitHub] qpid-dispatch pull request #328: DISPATCH-1045 - Release the delivery only a...

2018-06-27 Thread ted-ross
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.


---

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org



[GitHub] qpid-dispatch pull request #328: DISPATCH-1045 - Release the delivery only a...

2018-06-22 Thread ganeshmurthy
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




---

-
To unsubscribe, e-mail: dev-unsubscr...@qpid.apache.org
For additional commands, e-mail: dev-h...@qpid.apache.org