[GitHub] activemq-artemis pull request #1827: ARTEMIS-1650 Improve paged message ackn...
Github user asfgit closed the pull request at: https://github.com/apache/activemq-artemis/pull/1827 ---
[GitHub] activemq-artemis pull request #1827: ARTEMIS-1650 Improve paged message ackn...
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. ---
[GitHub] activemq-artemis pull request #1827: ARTEMIS-1650 Improve paged message ackn...
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. ---
[GitHub] activemq-artemis pull request #1827: ARTEMIS-1650 Improve paged message ackn...
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... ---
[GitHub] activemq-artemis pull request #1827: ARTEMIS-1650 Improve paged message ackn...
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 ---
[GitHub] activemq-artemis pull request #1827: ARTEMIS-1650 Improve paged message ackn...
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. ---