You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by CNNJYB <gi...@git.apache.org> on 2019/01/08 09:18:19 UTC
[GitHub] activemq-artemis pull request #2492: ARTEMIS-2222 why the position remains u...
GitHub user CNNJYB opened a pull request:
https://github.com/apache/activemq-artemis/pull/2492
ARTEMIS-2222 why the position remains unchanged if ignored is set to true
I am a bit confused about this, When CursorIterator:next is called during queue depage, if ignored is set to true, why the position remains unchanged.
if (!ignored) {
position = message.getPosition();
}
For example, the client sends some messages to the topic subscriber ta (this topic has two subscribers ta and tb), every time tb depage continuous PagePositions that ignored are set to true will be traversed again.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/CNNJYB/activemq-artemis dev-CursorIterator-moveNext-ignored
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/2492.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 #2492
----
commit 49f8bcd99640209c825c22e54a32c872850cc000
Author: andytaylor <an...@...>
Date: 2018-10-24T10:21:52Z
ARTEMIS-2144 - tx begin failure in ra doesn't get cleaned up
https://issues.apache.org/jira/browse/ARTEMIS-2144
commit 7ae39f7d7218e7c5a93eaecb8f24af3d6186e6f3
Author: yb <17...@...>
Date: 2019-01-08T09:14:18Z
ARTEMIS-2222 why the position remains unchanged if ignored is set to true
----
---
[GitHub] activemq-artemis pull request #2492: ARTEMIS-2222 why the position remains u...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/2492#discussion_r246538095
--- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/PageSubscriptionImpl.java ---
@@ -1315,9 +1315,7 @@ private PagedReference moveNext() {
}
}
- if (!ignored) {
- position = message.getPosition();
- }
+ position = message.getPosition();
--- End diff --
I think this made sense at some point, but after a few fixes later it's harmless.
I don't think this needs a JIRA as there's no issue I think.
I will merge and keep the JIRA, but I will rename the commit from being a question into something more affirmative.
---
[GitHub] activemq-artemis issue #2492: ARTEMIS-2222 why the position remains unchange...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/2492
This had a specific purpose so we ignore deleted or completed pages.
As per comment in code.
// any deleted or complete page will be ignored on the moveNextPage, we will just keep going
---
[GitHub] activemq-artemis pull request #2492: ARTEMIS-2222 why the position remains u...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/2492
---
[GitHub] activemq-artemis issue #2492: ARTEMIS-2222 why the position remains unchange...
Posted by franz1981 <gi...@git.apache.org>.
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/2492
It is correct that the first commit is related to ARTEMIS-2144?
---
[GitHub] activemq-artemis issue #2492: ARTEMIS-2222 why the position remains unchange...
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/2492
Point being if this is a bug, please can there be a test that exposes the issues, so a) this can be reviewed b) we can validate the fix c) ensure there is no regression.
---
[GitHub] activemq-artemis issue #2492: ARTEMIS-2222 why the position remains unchange...
Posted by CNNJYB <gi...@git.apache.org>.
Github user CNNJYB commented on the issue:
https://github.com/apache/activemq-artemis/pull/2492
> It is correct that the first commit is related to ARTEMIS-2144?
Branch is not latest. Now updated.
---
[GitHub] activemq-artemis issue #2492: ARTEMIS-2222 why the position remains unchange...
Posted by CNNJYB <gi...@git.apache.org>.
Github user CNNJYB commented on the issue:
https://github.com/apache/activemq-artemis/pull/2492
> This had a specific purpose so we ignore deleted or completed pages, so we shouldnt be getting the message position.
>
> As per comment in code.
>
> ```
> // any deleted or complete page will be ignored on the moveNextPage, we will just keep going
> ```
> Is there visible bug that needs addressing? We shouldnt just be changing code, without a specific testable bug we're trying to resolve.
> This had a specific purpose so we ignore deleted or completed pages, so we shouldnt be getting the message position.
>
> As per comment in code.
>
> ```
> // any deleted or complete page will be ignored on the moveNextPage, we will just keep going
> ```
> Is there visible bug that needs addressing? We shouldnt just be changing code, without a specific testable bug we're trying to resolve.
If some pages have not been deleted (tb consumes these messages, ta has not consumed yet), CursorIterator:moveNext is called during queue depage. These messages will be traversed again. Especially during CursorProviderget:getPageCache, it takes time to readPage, if pages are not in the softCache collection.
---