Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARTEMIS-856 Test fixes - Alternative #2211

Merged
merged 4 commits into from
Aug 2, 2018

Conversation

michaelandrepearce
Copy link
Contributor

@michaelandrepearce michaelandrepearce commented Aug 2, 2018

@clebertsuconic

Here's a slight alternative option to your pr (#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, but for the redistributor fixes many of the redistributor issues, essentially it was failing because it ended up in a spin.
#2203

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.

@michaelandrepearce michaelandrepearce changed the title Redistfix ARTEMIS-856 Test fixes - Alternative Aug 2, 2018
@michaelandrepearce
Copy link
Contributor Author

michaelandrepearce commented Aug 2, 2018

@clebertsuconic i figured out the last case and fixed it, as normal a one liner, so this should be good now if you'd rather take this one.

@clebertsuconic
Copy link
Contributor

@michaelandrepearce Let me run my internal CI.. if all good will merge it in 2 hours (time to have the whole run)

@franz1981
Copy link
Contributor

@michaelandrepearce @clebertsuconic If it is including the changes of #2203 please consider to add the test I have put there too (if it seems good) 👍

@asfgit asfgit merged commit de46517 into apache:master Aug 2, 2018
asfgit pushed a commit that referenced this pull request Aug 2, 2018
@michaelandrepearce
Copy link
Contributor Author

@franz1981 i dont think it made it before the merge. Could you pr it by itself and ill merge

@clebertsuconic
Copy link
Contributor

@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.

@michaelandrepearce
Copy link
Contributor Author

michaelandrepearce commented Aug 2, 2018

@clebertsuconic have the day off dude, i will look into tomorrow (its now nightime) during the day (UK), im sure i can sort, if any others just list.

@michaelandrepearce
Copy link
Contributor Author

@clebertsuconic see #2212.

Was a simple one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants