[jira] [Commented] (ARTEMIS-1650) Improve paged message acknowledge

2018-02-09 Thread ASF subversion and git services (JIRA)

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1650?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16358709#comment-16358709
 ] 

ASF subversion and git services commented on ARTEMIS-1650:
--

Commit 63e0c0d310850fb59f800d2cc5cf9c5cfc0060ec in activemq-artemis's branch 
refs/heads/master from Clebert Suconic
[ https://git-wip-us.apache.org/repos/asf?p=activemq-artemis.git;h=63e0c0d ]

ARTEMIS-1650 Fixing Testsuite on PageReference

Transactions may initialize a PagedReference without a valid message yet
during load of prepared transactions.

Caching has to be lazy on this case and it should load on demand.


> Improve paged message acknowledge
> -
>
> Key: ARTEMIS-1650
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1650
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>  Components: Broker
>Reporter: shoukun huai
>Priority: Minor
> Attachments: jstack.txt
>
>
> PagedMessage may be GCed before it was acknowledged. In this case, server has 
> to reload the page and hold the page cache lock, then block other consumers 
> of the same queue.
> When do acknowledge, we need at least message id, transaction id, and is 
> large message.
> We believe the three property can be added to PagedReference, so we do not 
> rely on PagedMessage when acknowledge except LargeMessage.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (ARTEMIS-1650) Improve paged message acknowledge

2018-02-08 Thread ASF subversion and git services (JIRA)

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1650?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16356982#comment-16356982
 ] 

ASF subversion and git services commented on ARTEMIS-1650:
--

Commit 822445a717f943c64a84b2ac6a0af8ace9e5cd23 in activemq-artemis's branch 
refs/heads/master from [~huaishk]
[ https://git-wip-us.apache.org/repos/asf?p=activemq-artemis.git;h=822445a ]

ARTEMIS-1650 Improve paged message acknowledge

Cache `messageID`, `transactionID` and `isLargeMessage`
in PagedReference, so that when acknowledge, we do not have to
get PagedMessage which may be GCed and cause re-read entire page.


> Improve paged message acknowledge
> -
>
> Key: ARTEMIS-1650
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1650
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>  Components: Broker
>Reporter: shoukun huai
>Priority: Minor
> Attachments: jstack.txt
>
>
> PagedMessage may be GCed before it was acknowledged. In this case, server has 
> to reload the page and hold the page cache lock, then block other consumers 
> of the same queue.
> When do acknowledge, we need at least message id, transaction id, and is 
> large message.
> We believe the three property can be added to PagedReference, so we do not 
> rely on PagedMessage when acknowledge except LargeMessage.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (ARTEMIS-1650) Improve paged message acknowledge

2018-02-08 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1650?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16356983#comment-16356983
 ] 

ASF GitHub Bot commented on ARTEMIS-1650:
-

Github user asfgit closed the pull request at:

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


> Improve paged message acknowledge
> -
>
> Key: ARTEMIS-1650
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1650
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>  Components: Broker
>Reporter: shoukun huai
>Priority: Minor
> Attachments: jstack.txt
>
>
> PagedMessage may be GCed before it was acknowledged. In this case, server has 
> to reload the page and hold the page cache lock, then block other consumers 
> of the same queue.
> When do acknowledge, we need at least message id, transaction id, and is 
> large message.
> We believe the three property can be added to PagedReference, so we do not 
> rely on PagedMessage when acknowledge except LargeMessage.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (ARTEMIS-1650) Improve paged message acknowledge

2018-02-05 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1650?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16352308#comment-16352308
 ] 

ASF GitHub Bot commented on ARTEMIS-1650:
-

Github user shoukunhuai commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1827#discussion_r165955807
  
--- Diff: 
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/RefsOperation.java
 ---
@@ -152,14 +153,14 @@ protected void rollbackRedelivery(Transaction tx, 
MessageReference ref, long tim
@Override
public void afterCommit(final Transaction tx) {
   for (MessageReference ref : refsToAck) {
- synchronized (ref.getQueue()) {
--- End diff --

hi @michaelandrepearce I get the sync back, i think we can discuss this 
later, may be another PR.


> Improve paged message acknowledge
> -
>
> Key: ARTEMIS-1650
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1650
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>  Components: Broker
>Reporter: shoukun huai
>Priority: Minor
> Attachments: jstack.txt
>
>
> PagedMessage may be GCed before it was acknowledged. In this case, server has 
> to reload the page and hold the page cache lock, then block other consumers 
> of the same queue.
> When do acknowledge, we need at least message id, transaction id, and is 
> large message.
> We believe the three property can be added to PagedReference, so we do not 
> rely on PagedMessage when acknowledge except LargeMessage.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (ARTEMIS-1650) Improve paged message acknowledge

2018-02-05 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1650?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16352305#comment-16352305
 ] 

ASF GitHub Bot commented on ARTEMIS-1650:
-

Github user shoukunhuai commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1827#discussion_r165955062
  
--- Diff: 
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/MessageReference.java
 ---
@@ -38,6 +38,10 @@ public static MessageReference createReference(Message 
encode, final Queue queue
 
Message getMessage();
 
+   default long getMessageID() {
--- End diff --

I have removed default method.


> Improve paged message acknowledge
> -
>
> Key: ARTEMIS-1650
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1650
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>  Components: Broker
>Reporter: shoukun huai
>Priority: Minor
> Attachments: jstack.txt
>
>
> PagedMessage may be GCed before it was acknowledged. In this case, server has 
> to reload the page and hold the page cache lock, then block other consumers 
> of the same queue.
> When do acknowledge, we need at least message id, transaction id, and is 
> large message.
> We believe the three property can be added to PagedReference, so we do not 
> rely on PagedMessage when acknowledge except LargeMessage.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (ARTEMIS-1650) Improve paged message acknowledge

2018-02-02 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1650?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16350691#comment-16350691
 ] 

ASF GitHub Bot commented on ARTEMIS-1650:
-

Github user clebertsuconic commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1827#discussion_r165706644
  
--- Diff: 
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/MessageReference.java
 ---
@@ -38,6 +38,10 @@ public static MessageReference createReference(Message 
encode, final Queue queue
 
Message getMessage();
 
+   default long getMessageID() {
--- End diff --

I agree here...


> Improve paged message acknowledge
> -
>
> Key: ARTEMIS-1650
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1650
> Project: ActiveMQ Artemis
>  Issue Type: Improvement
>  Components: Broker
>Reporter: shoukun huai
>Priority: Minor
> Attachments: jstack.txt
>
>
> PagedMessage may be GCed before it was acknowledged. In this case, server has 
> to reload the page and hold the page cache lock, then block other consumers 
> of the same queue.
> When do acknowledge, we need at least message id, transaction id, and is 
> large message.
> We believe the three property can be added to PagedReference, so we do not 
> rely on PagedMessage when acknowledge except LargeMessage.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (ARTEMIS-1650) Improve paged message acknowledge

2018-02-01 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1650?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16348539#comment-16348539
 ] 

ASF GitHub Bot commented on ARTEMIS-1650:
-

Github user michaelandrepearce commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1827#discussion_r165348688
  
--- Diff: 
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/RefsOperation.java
 ---
@@ -152,14 +153,14 @@ protected void rollbackRedelivery(Transaction tx, 
MessageReference ref, long tim
@Override
public void afterCommit(final Transaction tx) {
   for (MessageReference ref : refsToAck) {
- synchronized (ref.getQueue()) {
--- End diff --

Makes sense. I think best way of getting confidence is a really good test 
case to prove it 


> Improve paged message acknowledge
> -
>
> Key: ARTEMIS-1650
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1650
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>  Components: Broker
>Reporter: shoukun huai
>Priority: Minor
> Attachments: jstack.txt
>
>
> PagedMessage may be GCed before it was acknowledged. In this case, server has 
> to reload the page and hold the page cache lock, then block other consumers 
> of the same queue.
> When do acknowledge, we need at least message id, transaction id, and is 
> large message.
> We believe the three property can be added to PagedReference, so we do not 
> rely on PagedMessage when acknowledge except LargeMessage.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)


[jira] [Commented] (ARTEMIS-1650) Improve paged message acknowledge

2018-02-01 Thread ASF GitHub Bot (JIRA)

[ 
https://issues.apache.org/jira/browse/ARTEMIS-1650?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16348473#comment-16348473
 ] 

ASF GitHub Bot commented on ARTEMIS-1650:
-

Github user shoukunhuai commented on a diff in the pull request:

https://github.com/apache/activemq-artemis/pull/1827#discussion_r16574
  
--- Diff: 
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/RefsOperation.java
 ---
@@ -152,14 +153,14 @@ protected void rollbackRedelivery(Transaction tx, 
MessageReference ref, long tim
@Override
public void afterCommit(final Transaction tx) {
   for (MessageReference ref : refsToAck) {
- synchronized (ref.getQueue()) {
--- End diff --

I do this because in QueueImpl::postAcknowledge, the only thing that 
changed queue's state is decrement queue's delivering count, which i believe is 
thread safe.

While QueueImpl:postRollback is different, it will not return messages if 
purge on no consumers and there is no consumer at that time, since add/remove 
consumer will lock the queue. So we must lock the queue when do post rollback.

If no body is 100% sure about this, i will revert this change, since other 
change has improved a lot.


> Improve paged message acknowledge
> -
>
> Key: ARTEMIS-1650
> URL: https://issues.apache.org/jira/browse/ARTEMIS-1650
> Project: ActiveMQ Artemis
>  Issue Type: Bug
>  Components: Broker
>Reporter: shoukun huai
>Priority: Minor
> Attachments: jstack.txt
>
>
> PagedMessage may be GCed before it was acknowledged. In this case, server has 
> to reload the page and hold the page cache lock, then block other consumers 
> of the same queue.
> When do acknowledge, we need at least message id, transaction id, and is 
> large message.
> We believe the three property can be added to PagedReference, so we do not 
> rely on PagedMessage when acknowledge except LargeMessage.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)