You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by michaelandrepearce <gi...@git.apache.org> on 2017/08/11 20:28:24 UTC
[GitHub] activemq-artemis pull request #1461: ARTEMIS-1348 Support LVQ for AMQP
GitHub user michaelandrepearce opened a pull request:
https://github.com/apache/activemq-artemis/pull/1461
ARTEMIS-1348 Support LVQ for AMQP
Add support for LVQ, using the same property key as core "_AMQ_LVQ_NAME"
Add test case for AMQP LVQ.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/michaelandrepearce/activemq-artemis ARTEMIS-1348
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/1461.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 #1461
----
commit 502a77b6f9ba891fc467173e961f48b85bb2ae15
Author: Michael Andre Pearce <mi...@me.com>
Date: 2017-08-11T20:27:49Z
ARTEMIS-1348 Support LVQ for AMQP
Add support for LVQ, using the same property key as core "_AMQ_LVQ_NAME"
Add test case for AMQP LVQ.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #1461: ARTEMIS-1348 Support LVQ for AMQP
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1461
The reason I didn't implement it was the reason @tabish121 mentioned.. there's a proper way to do that with AMQPMessage (some property I don't remember now).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #1461: ARTEMIS-1348 Support LVQ for AMQP
Posted by tabish121 <gi...@git.apache.org>.
Github user tabish121 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1461
This approach has a couple of negatives going for it.
First the use of application properties for message handling directives now requires that every incoming message will have to decode that section of the message where previously the broker attempted to not do so unless needed as there is significant overhead in the decode.
Second the LVQ property applied to the sent message is now carried on the outbound message which may confuse those who aren't expecting that property in the messages received, I think the core JMS client filters these out in getPropertyNames JMS APIs but I could be wrong, Qpid JMS certainly would not. In any case it's odd that a routing directive is propagate all the way to the receiving client, these sorts of things are known to cause issues when the message is retransmitted or hops around a broker network.
This would be better slotted in as a Message Annotation or maybe even a Delivery Annotation, would need some more thought to determine which is the right place. In general the Application Properties of an AMQP message do not carry directives for the peer.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #1461: ARTEMIS-1348 Support LVQ for AMQP
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1461
There is the _properties section (but these are predefined in qpid (org.apache.qpid.proton.amqp.messaging.Properties). e.g. you cannot add to this section as far as i can tell.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #1461: ARTEMIS-1348 Support LVQ for AMQP
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1461
@tabish121
On point 1:
This only is queried (and parsed) if the address is configured for LVQ. I believe on QPID java broker it is set in the same place (application properties).
On point 2:
It it is carried over also on Core so id disagree a bit on this.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #1461: ARTEMIS-1348 Support LVQ for AMQP
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1461
So I can confirm the method on interface Message.getLastValueProperty is only invoked by LastValueQueue.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #1461: ARTEMIS-1348 Support LVQ for AMQP
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1461
I guess if the parse properties only happens inside the last value queue it will be ok. I had tried to only parse properties when needed. E.g. Filters.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #1461: ARTEMIS-1348 Support LVQ for AMQP
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1461
Added to test case to check the property in JMS is the consumed message.
Added to test case for it to check the behaviour is identical to Core protocol
Lastly also I have updated the test case to check cross compatibility with core protocol also, e.g. produce with core, consume with amqp and vice versa. So as a user using JMS clients what you get is identical either side.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis pull request #1461: ARTEMIS-1348 Support LVQ for AMQP
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1461
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] activemq-artemis issue #1461: ARTEMIS-1348 Support LVQ for AMQP
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1461
As i dig through this, it seems the only option if using QPID JMS wrapper without making that vendor specific (aka that i think should remain agnostic to artemis), is to use application properties. It seems there is no dynamic way for telling it that a specific property should be in message annotations instead of application properties. (unless I'm missing something)
So as it stands I'm struggling to see any better way for now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---