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 2018/08/02 10:49:22 UTC

[GitHub] activemq-artemis pull request #2211: Redistfix

GitHub user michaelandrepearce opened a pull request:

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

    Redistfix

    @clebetsuconic 
    
    Here's a slight alternative option to your pr (https://github.com/apache/activemq-artemis/pull/2209) i took the fixes for the server and control, but in the queueimpl I wanted to keep away from having the redistributor in the list (reason being later where want to try make the dispatch policies more pluggable it makes it very hard)
    
    Also interestingly applying the same change that Franz has done for CPU here:
    https://github.com/apache/activemq-artemis/pull/2203
    
    But for the redistributor fixes many of the redistributor issues. 
    
    One case in the messageredistributiontest though still fails (testRedistributionWithMessageGroups), im looking into why it might, but if you can spot please do tell.
    
    If you need master fixed asap, maybe merge your PR, and i can pr this over the top once i figure it out.
    


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

    $ git pull https://github.com/michaelandrepearce/activemq-artemis REDISTFIX

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

    https://github.com/apache/activemq-artemis/pull/2211.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 #2211
    
----
commit 59520b9018d4a44fffa0bf05c49970d50ed69ab5
Author: Clebert Suconic <cl...@...>
Date:   2018-08-02T02:04:31Z

    ARTEMIS-856 Fixing QueueCommandTest

commit f0aca5c953cb3712e668bdfa72dd3fceb7c164e1
Author: Michael André Pearce <mi...@...>
Date:   2018-08-02T09:51:27Z

    ARTEMIS-856 Fixing MessageRedistributionTest

----


---

[GitHub] activemq-artemis issue #2211: ARTEMIS-856 Test fixes - Alternative

Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2211
  
    @franz1981 i dont think it made it before the merge. Could you pr it by itself and ill merge


---

[GitHub] activemq-artemis issue #2211: ARTEMIS-856 Test fixes - Alternative

Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2211
  
    @clebertsuconic see https://github.com/apache/activemq-artemis/pull/2212.
    
    Was a simple one. 


---

[GitHub] activemq-artemis issue #2211: ARTEMIS-856 Test fixes - Alternative

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2211
  
    @michaelandrepearce Let me run my internal CI.. if all good will merge it in 2 hours (time to have the whole run)


---

[GitHub] activemq-artemis issue #2211: ARTEMIS-856 Test fixes - Alternative

Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2211
  
    @clebertsuconic i figured out the last case and fixed it, as normal a one liner.


---

[GitHub] activemq-artemis issue #2211: ARTEMIS-856 Test fixes - Alternative

Posted by franz1981 <gi...@git.apache.org>.
Github user franz1981 commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2211
  
    @michaelandrepearce @clebertsuconic If it is including the changes of https://github.com/apache/activemq-artemis/pull/2203 please consider to add the test I have put there too (if it seems good) :+1: 


---

[GitHub] activemq-artemis pull request #2211: ARTEMIS-856 Test fixes - Alternative

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


---

[GitHub] activemq-artemis issue #2211: ARTEMIS-856 Test fixes - Alternative

Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2211
  
    @michaelandrepearce I still have the following tests failing with these changes:
    
    - org.apache.activemq.artemis.tests.integration.cluster.distribution.AMQPMessageLoadBalancingTest.testLoadBalanceAMQP
    - org.apache.activemq.artemis.tests.integration.cluster.failover.ReplicatedPagingFailoverTest.testPageFailBeforeConsume
    - org.apache.activemq.artemis.tests.integration.openwire.cluster.TemporaryQueueClusterTest.testClusteredQueue
    - org.apache.activemq.artemis.tests.integration.server.ScaleDownTest.testBasicScaleDown[useScaleDownGroupName=true]
    - org.apache.activemq.artemis.tests.integration.server.ScaleDownTest.testStoreAndForward[useScaleDownGroupName=true]
    - org.apache.activemq.artemis.tests.integration.server.ScaleDownTest.testBasicScaleDown[useScaleDownGroupName=false]
    - org.apache.activemq.artemis.tests.integration.server.ScaleDownTest.testStoreAndForward[useScaleDownGroupName=false]
    
    
    These all passed 100% with my other fix. (Not saying my fix was correct.. but there's something still around the routing logic.
    
    I'm taking today's off... I may come back into this tomorrow.. but any help is appreciated.


---

[GitHub] activemq-artemis issue #2211: ARTEMIS-856 Test fixes - Alternative

Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:

    https://github.com/apache/activemq-artemis/pull/2211
  
    @clebertsuconic have the day off dude, i will look into tomorrow during the day, im sure i can sort, if any others just list.


---