Bug 1283652 - Broker ignores parameters of Modified outcome received from a consumer
Broker ignores parameters of Modified outcome received from a consumer
Status: NEW
Product: Red Hat Enterprise MRG
Classification: Red Hat
Component: qpid-cpp (Show other bugs)
3.2
x86_64 Linux
medium Severity medium
: ---
: ---
Assigned To: messaging-bugs
Messaging QE
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2015-11-19 09:07 EST by Pavel Moravec
Modified: 2015-11-22 16:42 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed:
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)


External Trackers
Tracker ID Priority Status Summary Last Updated
Apache JIRA QPID-6870 None None None Never

  None (edit)
Description Pavel Moravec 2015-11-19 09:07:54 EST
Description of problem:
When a consumer sends modified outcome of a message with undeliverable-here=true and delivery-failed=false, the broker
- sends the message to the same consumer again
- increases delivery-count of the message

First violates AMQP 1.0 spec:

3.4.5 Modified:

"If the undeliverable-here is set, then any messages released MUST NOT be redelivered to the modifying link endpoint."

Second is questionable per:

"If the delivery-failed flag is set, any messages modified MUST have their delivery-count incremented."

(that implies delivery-failed=false SHOULD NOT increase delivery-count)

Please fix the first flaw and consider changing behaviour of the second one.


Version-Release number of selected component (if applicable):
qpid-cpp-server-0.34-5.el6.x86_64


How reproducible:
100%


Steps to Reproduce:
to be added


Actual results:
See Description (and future reproducer)


Expected results:
See Description (and future reproducer)

Additional info:
qpid/cpp/src/qpid/broker/amqp/Outgoing.cpp:

..
              case PN_MODIFIED:
                if (preAcquires()) queue->release(r.cursor, pn_disposition_is_failed(pn_delivery_remote(delivery)));
                //TODO: handle undeliverable-here and message-annotations
                outgoingMessageRejected();//TODO: not quite true...
                break;
..
Comment 1 Gordon Sim 2015-11-19 10:46:25 EST
I've committed a fix upstream that causes undeliverable-here messages to be rejected: https://svn.apache.org/r1715199. I believe this avoids any violation of the spec.

If delivery-failed is false (and the message is not rejected) then the delivery count does not appear to get incremented as things stand.
Comment 2 Pavel Moravec 2015-11-22 16:39:20 EST
Reproducer:

#!/usr/bin/python
import optparse
from proton import Delivery, symbol
from proton.handlers import MessagingHandler
from proton.reactor import Container



def dump(obj):
  for attr in dir(obj):
    print "obj.%s = %s" % (attr, getattr(obj, attr))


class Recv(MessagingHandler):
    def __init__(self, url, failed):
        super(Recv, self).__init__(auto_accept=False)
        self.url = url
        self.failed = failed

    def on_start(self, event):
        event.container.create_receiver(self.url)

    def on_message(self, event):
        print event._attrs
        event.delivery.local.failed = self.failed
        event.delivery.local.undeliverable = True
        event.delivery.update(Delivery.MODIFIED)
        event.delivery.settle()
#        event.connection.close()

parser = optparse.OptionParser(usage="usage: %prog [options]")
parser.add_option("-a", "--address", default="localhost:5672/examples",
                  help="address from which messages are received (default %default)")
parser.add_option("-f", "--failed", action="store_true", help="delivery-failed")
opts, args = parser.parse_args()

try:
    Container(Recv(opts.address, opts.failed != None)).run()
except KeyboardInterrupt: pass


1) service qpidd restart; qpid-send -a "examples; {create:always}" -m1
2) python modified.py
3) service qpidd restart; qpid-send -a "examples; {create:always}" -m1
4) python modified.py -f

Expected behaviour:
2) prints one message like:
{'message': Message{priority=0, first_acquirer=1, user_id="", content_type="text/plain", properties={"sn"=1, "ts"=1448228051627021916}}}

4) prints the same

Observed behaviour:
2) prints same message like:
{'message': Message{priority=0, first_acquirer=1, user_id="", content_type="text/plain", properties={"sn"=1, "ts"=1448228301965840558}}}
..

forever

4) prints messages like:
{'message': Message{priority=0, first_acquirer=1, user_id="", content_type="text/plain", properties={"sn"=1, "ts"=1448228254722139242}}}
{'message': Message{priority=0, delivery_count=1, user_id="", content_type="text/plain", properties={"sn"=1, "ts"=1448228254722139242}}}
{'message': Message{priority=0, delivery_count=2, user_id="", content_type="text/plain", properties={"sn"=1, "ts"=1448228254722139242}}}
..

forever
Comment 3 Pavel Moravec 2015-11-22 16:42:54 EST
(In reply to Gordon Sim from comment #1)
> I've committed a fix upstream that causes undeliverable-here messages to be
> rejected: https://svn.apache.org/r1715199. I believe this avoids any
> violation of the spec.

Although it is not stated there that undeliverable-here=true MODIFIED messages should be offered to other consumers, I expect that is the intention behind the spec. But I agree implementing it would be a bigger task - every message would have to keep a list of consumers setting undeliverable-here in past..

I.e. I am not happy from the implementation but dont see much better.

> 
> If delivery-failed is false (and the message is not rejected) then the
> delivery count does not appear to get incremented as things stand.

Agreed from my experiments, clarifying with original reporter their reproducer/scenario.

Note You need to log in before you can comment on or make changes to this bug.