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/07/28 16:17:03 UTC
[GitHub] activemq-artemis pull request #1428: ARTEMIS-1308: Delegate acknowlegde to C...
GitHub user michaelandrepearce opened a pull request:
https://github.com/apache/activemq-artemis/pull/1428
ARTEMIS-1308: Delegate acknowlegde to ClientMessage
ActiveMQMessage acknowledge should delegate to ClientMessage acknowledge
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/michaelandrepearce/activemq-artemis ARTEMIS-1308
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/1428.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 #1428
----
commit a1887fb8271e1e660d16683f4dc6a62309feeaf4
Author: Michael Andre Pearce <mi...@me.com>
Date: 2017-07-28T13:27:29Z
ARTEMIS-1308: Delegate acknowlegde to ClientMessage
ActiveMQMessage acknowledge should delegate to ClientMessage acknowledge
----
---
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 #1428: ARTEMIS-1308: Delegate acknowlegde to ClientMe...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1428
@franz1981 looks like its sporadic. just done two builds and it didn't occur again
---
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 #1428: ARTEMIS-1308: Make acknowlegde in AcitveMQMess...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1428
mvn test -Ptests
it takes about 2 hours
---
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 #1428: ARTEMIS-1308: Make acknowlegde in AcitveMQMess...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1428
has finally completed with build success locally using that maven profile :)
---
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 #1428: ARTEMIS-1308: Make acknowlegde in AcitveMQMess...
Posted by franz1981 <gi...@git.apache.org>.
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1428
@michaelandrepearce I've made a fix for that test here: https://github.com/apache/activemq-artemis/pull/1444
---
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 #1428: ARTEMIS-1308: Make acknowlegde in AcitveMQMess...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1428
@clebertsuconic which profile i need to run on maven for that (obviously i can run just that test but if i look to fix anything i would want to rerun the whole suite/profile of tests)? locally I'm running the same build as the PR does atm.
---
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 #1428: ARTEMIS-1308: Make acknowlegde in AcitveMQMess...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1428
@michaelandrepearce I wish apache had better server so we could run these in in a public fashion. I have a CI Box that I can run jobs passing a github account.
---
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 #1428: ARTEMIS-1308: Delegate acknowlegde to ClientMe...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1428
@clebertsuconic @franz1981 i believe i have this PR working except i am getting one test failure, see below.
https://builds.apache.org/job/ActiveMQ-Artemis-PR-Build/3570/testReport/junit/org.apache.activemq.artemis.tests.unit.core.journal.impl/TimedBufferTest/testTimeOnTimedBuffer/
Is this related at all to:
https://github.com/apache/activemq-artemis/pull/1435
Or the other TimedBuffer work? If not any ideas what this is, i haven't touched this area at all for this PR so doesn't make sense to me.
Cheers
Mike
---
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 #1428: ARTEMIS-1308: Delegate acknowlegde to ClientMe...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1428
This is def improvement over the first patch I saw.
---
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 #1428: ARTEMIS-1308: Delegate acknowlegde to ClientMe...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1428
would be too much to ask you to send a PR on the 1.x branch?
---
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 #1428: ARTEMIS-1308: Delegate acknowlegde to ClientMe...
Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on the issue:
https://github.com/apache/activemq-artemis/pull/1428
PR builds don't "merge with master" in any kind of automatic way. The PR build simply builds the PR. It's up to the submitter to rebase on master if necessary.
---
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 #1428: ARTEMIS-1308: Delegate acknowlegde to ClientMe...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1428
Have added a test case to ensure perf is at least double when many message, we expect it to be more as it should no longer block at all and it is, but was a good way to test its giving the improvement expected, e.g. the prior to the change or reverting the test fails as the perf is equal between the two states.
Here is a local output of the improvement in consume time of 10,000 when we make acknowledge non-blocking.
[main] 06:13:57,987 INFO [org.apache.activemq.artemis.jms.tests] BlockOnAcknowledge=false MessageCount=10000 TimeToConsume=462980029
[main] 06:13:57,987 INFO [org.apache.activemq.artemis.jms.tests] BlockOnAcknowledge=true MessageCount=10000 TimeToConsume=38748489101
---
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 #1428: ARTEMIS-1308: Delegate acknowlegde to ClientMe...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1428
This is causing a few failures on the testsuite:
Test Name | Duration | Age
-- | -- | --
org.apache.activemq.artemis.jms.tests.MessageConsumerTest.testRedeliveredDifferentSessions | 54 ms | 1
org.apache.activemq.artemis.jms.tests.MessageConsumerTest.testClientAcknowledgmentOnClosedConsumer | 1 sec | 1
org.apache.activemq.artemis.jms.tests.MessageConsumerTest.testRedel7 | 31 ms | 1
org.apache.activemq.artemis.jms.tests.message.BytesMessageTest.testRedelivery | 52 ms | 1
org.apache.activemq.artemis.jms.tests.message.MapMessageTest.testRedelivery | 38 ms | 1
org.apache.activemq.artemis.jms.tests.message.ObjectMessageTest.testRedelivery | 33 ms | 1
org.apache.activemq.artemis.jms.tests.message.StreamMessageTest.testRedelivery | 26 ms | 1
org.apache.activemq.artemis.jms.tests.message.TextMessageTest.testRedelivery | 23 ms | 1
org.apache.activemq.artemis.jms.tests.message.foreign.ForeignBytesMessageTest.testRedelivery | 30 ms | 1
org.apache.activemq.artemis.jms.tests.message.foreign.ForeignMapMessageTest.testRedelivery | 0.18 sec | 1
org.apache.activemq.artemis.jms.tests.message.foreign.ForeignMessageTest.testRedelivery | 37 ms | 1
org.apache.activemq.artemis.jms.tests.message.foreign.ForeignObjectMessageTest.testRedelivery | 30 ms | 1
org.apache.activemq.artemis.jms.tests.message.foreign.ForeignStreamMessageTest.testRedelivery | 29 ms | 1
org.apache.activemq.artemis.jms.tests.message.foreign.ForeignTextMessageTest.testRedelivery | 34 ms | 1
---
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 #1428: ARTEMIS-1308: Delegate acknowlegde to ClientMe...
Posted by franz1981 <gi...@git.apache.org>.
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1428
@michaelandrepearce try to see if you version include this: https://github.com/apache/activemq-artemis/commit/74f243cc4d9ec46f041e765148bdcb8be5af8b10
It's a fix for that test
---
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 #1428: ARTEMIS-1308: Delegate acknowlegde to ClientMe...
Posted by jbertram <gi...@git.apache.org>.
Github user jbertram commented on the issue:
https://github.com/apache/activemq-artemis/pull/1428
How will the ack get to the broker if you don't commit the session?
---
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 #1428: ARTEMIS-1308: Make acknowlegde in AcitveMQMess...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1428
@franz1981 thanks, we've got a succesfull build post my rebase, and squash anyhow :).
@ all this i think is in a state to look to merge, tests all passing in PR build.
---
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 #1428: ARTEMIS-1308: Delegate acknowlegde to ClientMe...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1428
Yeah seems that we need to somehow ensure when redelivery or consumer.close is called to ensure any waiting batched packets are send and confirmed. It seems some acks are still in batches waiting to be sent. (i tried also doing the other option you suggested Clebert with an async ack, instead of using auto ackcommit on the session it has same problem) ill look to see what options there are
---
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 #1428: ARTEMIS-1308: Make acknowlegde in Acitv...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1428
---
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 #1428: ARTEMIS-1308: Make acknowlegde in AcitveMQMess...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1428
There are 3 tests failures on org.apache.activemq.artemis.tests.integration.jms.consumer.JmsConsumerTest
something around individual ack.
---
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 #1428: ARTEMIS-1308: Make acknowlegde in AcitveMQMess...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1428
nice job on this one... no regressions whatsoever.. I even ran some outer tests on this.. compatibility kit and other stuff.. nice job!
---
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 #1428: ARTEMIS-1308: Make acknowlegde in AcitveMQMess...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1428
@jbertram thanks for clarifying my incorrect assumption on how pr build works.
Have as such took the opportunity to rebase, squash and correct the commit message to reflect now current solution.
---
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 #1428: ARTEMIS-1308: Make acknowlegde in AcitveMQMess...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1428
@clebertsuconic thanks, ill run that.
re the individual acks on that test, simple moved the close method before checking state in the test, as on close state is actual, before that with block on ack it is just eventual. have pushed this change.
---
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 #1428: ARTEMIS-1308: Delegate acknowlegde to ClientMe...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1428
This is def a WIP for a possible solution, but made some progress in IRC chat. don't think we are there yet entirely.
---
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 #1428: ARTEMIS-1308: Delegate acknowlegde to ClientMe...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1428
@franz1981 i assume it would i thought PR builds merge with master, as such should pick up, I've forced it to rebuild by doing a whitespace push to see if its sporadic or not.
---
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 #1428: ARTEMIS-1308: Make acknowlegde in AcitveMQMess...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1428
@clebertsuconic 2 hours? what machine do you have? My laptop is still running tests atm :).
---
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.
---