[GitHub] activemq-artemis pull request #2362: ARTEMIS-2117 Add custom LVQ Key and Non...

classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #2362: ARTEMIS-2117 Add custom LVQ Key and Non...

spyrkob
GitHub user michaelandrepearce opened a pull request:

    https://github.com/apache/activemq-artemis/pull/2362

    ARTEMIS-2117 Add custom LVQ Key and Non Destructive Queue into Broker

    Implement custom LVQ Key and Non-Destructive in broker - protocol agnostic
    Make feature configurable via broker.xml, core apis and activemqservercontrol
    Add last-value-key test cases
    Add non-destructive with lvq test cases
    Add non-destructive with expiry-delay test cases
    Update documents
    Add new methods to support create, update with new attributes
    Refactor to pass through queue-attributes in client side methods to reduce further method changes for adding new attributes in future and avoid methods with endless parameters. (note: in future this should prob be done server side too)
   
   
    Update existing test cases and fake impls for new methods/attributes

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/michaelandrepearce/activemq-artemis qpid-lvq-feature

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/activemq-artemis/pull/2362.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 #2362
   
----
commit 2af8fc924cbf4c56746331bd98b7c1547162e905
Author: Michael André Pearce <michael.andre.pearce@...>
Date:   2018-10-10T00:07:02Z

    ARTEMIS-2117 Add custom LVQ Key and Non Destructive Queue into Broker
   
    Implement custom LVQ Key and Non-Destructive in broker - protocol agnostic
    Make feature configurable via broker.xml, core apis and activemqservercontrol
    Add last-value-key test cases
    Add non-destructive with lvq test cases
    Add non-destructive with expiry-delay test cases
    Update documents
    Add new methods to support create, update with new attributes
    Refactor to pass through queue-attributes in client side methods to reduce further method changes for adding new attributes in future and avoid methods with endless parameters. (note: in future this should prob be done server side too)
   
   
    Update existing test cases and fake impls for new methods/attributes

----


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2362: ARTEMIS-2117 Add custom LVQ Key and Non Destru...

spyrkob
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2362
 
    will merge if no comments


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2362: ARTEMIS-2117 Add custom LVQ Key and Non Destru...

spyrkob
In reply to this post by spyrkob
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2362
 
    What is Non Destructive? I had a hard time understanding the use case.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #2362: ARTEMIS-2117 Add custom LVQ Key and Non...

spyrkob
In reply to this post by spyrkob
Github user clebertsuconic commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/2362#discussion_r225527717
 
    --- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/Message.java ---
    @@ -186,10 +186,12 @@ default Message setRoutingType(RoutingType routingType) {
           return this;
        }
     
    +   @Deprecated
    --- End diff --
   
    Why deprecate this?
   
   
    AMQP has some properties within the Spec that could be used. So this could be Protocol Specific. Hence why we had this.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2362: ARTEMIS-2117 Add custom LVQ Key and Non Destru...

spyrkob
In reply to this post by spyrkob
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2362
 
    My colleagues educated me on the non-descructive.
   
   
    Only question I have now is the deprecated method.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #2362: ARTEMIS-2117 Add custom LVQ Key and Non...

spyrkob
In reply to this post by spyrkob
Github user michaelandrepearce commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/2362#discussion_r225585632
 
    --- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/Message.java ---
    @@ -186,10 +186,12 @@ default Message setRoutingType(RoutingType routingType) {
           return this;
        }
     
    +   @Deprecated
    --- End diff --
   
    No need ill remove


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2362: ARTEMIS-2117 Add custom LVQ Key and Non Destru...

spyrkob
In reply to this post by spyrkob
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2362
 
    I'm running a whole testsuite before we can merge this.. will have results in 3 hours.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #2362: ARTEMIS-2117 Add custom LVQ Key and Non...

spyrkob
In reply to this post by spyrkob
Github user michaelandrepearce commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/2362#discussion_r225695887
 
    --- Diff: artemis-core-client/src/main/java/org/apache/activemq/artemis/api/core/Message.java ---
    @@ -186,10 +186,12 @@ default Message setRoutingType(RoutingType routingType) {
           return this;
        }
     
    +   @Deprecated
    --- End diff --
   
    removed


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2362: ARTEMIS-2117 Add custom LVQ Key and Non Destru...

spyrkob
In reply to this post by spyrkob
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2362
 
    the testsuite is stalling... I will have to figure out where and put some context here.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2362: ARTEMIS-2117 Add custom LVQ Key and Non Destru...

spyrkob
In reply to this post by spyrkob
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2362
 
    @clebertsuconic if you could share, im a bit surprised its this, as the new features should only impact if enabled, so wan't expecting any issues in old cases. Obviously like always happy to look at, as something could have been overlooked


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2362: ARTEMIS-2117 Add custom LVQ Key and Non Destru...

spyrkob
In reply to this post by spyrkob
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2362
 
    I had to take kids to a piano lesson tonight.  I don’t have a computer now.  
   
   
    I will have to connect later tonight.
   
   
    I will do some debug before I post as well. (Will run it again )


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2362: ARTEMIS-2117 Add custom LVQ Key and Non Destru...

spyrkob
In reply to this post by spyrkob
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2362
 
    @michaelandrepearce I tested directly at your branch.. I just rebased on a new branch and ran it and it passed fine. There was a few fixes that you were behind I assume.
   
    I will merge it.


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis issue #2362: ARTEMIS-2117 Add custom LVQ Key and Non Destru...

spyrkob
In reply to this post by spyrkob
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2362
 
    I will merge this now.. if there's any issues we can review later


---
Reply | Threaded
Open this post in threaded view
|

[GitHub] activemq-artemis pull request #2362: ARTEMIS-2117 Add custom LVQ Key and Non...

spyrkob
In reply to this post by spyrkob
Github user asfgit closed the pull request at:

    https://github.com/apache/activemq-artemis/pull/2362


---