[GitHub] activemq-artemis pull request #1461: ARTEMIS-1348 Support LVQ for AMQP

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis pull request #1461: ARTEMIS-1348 Support LVQ for AMQP

franz1981
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 <[hidden email]>
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1461: ARTEMIS-1348 Support LVQ for AMQP

franz1981
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1461: ARTEMIS-1348 Support LVQ for AMQP

franz1981
In reply to this post by franz1981
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1461: ARTEMIS-1348 Support LVQ for AMQP

franz1981
In reply to this post by franz1981
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1461: ARTEMIS-1348 Support LVQ for AMQP

franz1981
In reply to this post by franz1981
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1461: ARTEMIS-1348 Support LVQ for AMQP

franz1981
In reply to this post by franz1981
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1461: ARTEMIS-1348 Support LVQ for AMQP

franz1981
In reply to this post by franz1981
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1461: ARTEMIS-1348 Support LVQ for AMQP

franz1981
In reply to this post by franz1981
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis issue #1461: ARTEMIS-1348 Support LVQ for AMQP

franz1981
In reply to this post by franz1981
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] activemq-artemis pull request #1461: ARTEMIS-1348 Support LVQ for AMQP

franz1981
In reply to this post by franz1981
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 [hidden email] or file a JIRA ticket
with INFRA.
---
Loading...