You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by gaol <gi...@git.apache.org> on 2016/04/28 11:09:57 UTC
[GitHub] activemq-artemis pull request: [ARTEMIS-506] PagingTest#testDelete...
GitHub user gaol opened a pull request:
https://github.com/apache/activemq-artemis/pull/495
[ARTEMIS-506] PagingTest#testDeletePhysicalPages fails
Jira: https://issues.apache.org/jira/browse/ARTEMIS-506
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/gaol/jboss-activemq-artemis ARTEMIS-506
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/495.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 #495
----
commit 452a16f9a04d17a53f10aff51926e19acf18c823
Author: Lin Gao <lg...@redhat.com>
Date: 2016-04-28T05:35:56Z
[ARTEMIS-506] PagingTest#testDeletePhysicalPages fails
----
---
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: [ARTEMIS-506] PagingTest#testDelete...
Posted by gaol <gi...@git.apache.org>.
Github user gaol closed the pull request at:
https://github.com/apache/activemq-artemis/pull/495
---
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: [ARTEMIS-506] PagingTest#testDelete...
Posted by gaol <gi...@git.apache.org>.
Github user gaol commented on the pull request:
https://github.com/apache/activemq-artemis/pull/495#issuecomment-218395699
Yes, sure. :-)
---
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: [ARTEMIS-506] PagingTest#testDelete...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/495#issuecomment-215426173
I'm running the whole testsuite first.
---
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: [ARTEMIS-506] PagingTest#testDelete...
Posted by gaol <gi...@git.apache.org>.
Github user gaol commented on the pull request:
https://github.com/apache/activemq-artemis/pull/495#issuecomment-218057390
Would you please tell which test it breaks if any?
This test actually reveals a failure in paging IMHO:
When `PageSubscriptionImpl.cleanupEntries(boolean)` is called in PagingStore's executor, and one page is considered completed, then a `PAGE_CURSOR_COMPLETE` record is appended to message journal for this page in a transactional operation.
Normally, the page will be deleted after the transaction is committed by the cleanup process(`PageCursorProviderImpl.cleanup()`), then a `DeleteRecord` record is appended to the message journal for this page. Sometimes, the page won't get deleted when the PagingStore is stopped during the cleanup process. But it is fine too because the server's restart will clean up the page anyway.
Failure happens on conditions:
* The completed page is not deleted by the cleanup process, and the `PAGE_CURSOR_COMPLETE` record survives.
* The completed page is deleted manually after the server is stopped
* Server is restarted, the `PAGE_CURSOR_COMPLETE` record still survivals, because there is no such page file in the paging store, and the clean up process does nothing about it.
* Client sends many messages to the server so that the server starts to page messages to paging store, and the new page file has the same page number(like page number: 120 in this test failure) with the survival `completed` page above.
In such conditions, the new page file will be considered completed, which leads to the messages in this page file won't get consumed. It is kind of missing messages.
The fix in this PR tries to append the `DeleteRecord` record for the associated `PAGE_CURSOR_COMPLETE` record when a message is stored in a paging file and clear the `completed` flag in the `PageSubscriptionImpl.PageCursorInfo`. I think at this moment, the page should **NOT** be considered `completed`, and the completed flag should be cleared if any. I am a little confused why it breaks the implementation?
This is my first try on Artemis issue, there must be many misunderstandings, it will be very appreciated if you can give me any light on that. Thank you. :-)
---
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: [ARTEMIS-506] PagingTest#testDelete...
Posted by gaol <gi...@git.apache.org>.
Github user gaol commented on the pull request:
https://github.com/apache/activemq-artemis/pull/495#issuecomment-215409661
The CI build was aborted because of execution timeout, could someone please restart the 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 pull request: [ARTEMIS-506] PagingTest#testDelete...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/495#issuecomment-220460874
@gaol Can you review my latest changes on paging, and this can probably be closed?
---
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: [ARTEMIS-506] PagingTest#testDelete...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/495#issuecomment-217851455
It seems this will break paging.. it's fixing a test but breaking the implementation...
I couldn't get to the bottom of it as I'm working into another possible paging issue, hence I left it in the back burners while I'm looking at this other one. (I could then find something related here)
---
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: [ARTEMIS-506] PagingTest#testDelete...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/495#issuecomment-218182737
Let me work on this in 2 days? I need to finish something else on paging before I can merge that.
---
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: [ARTEMIS-506] PagingTest#testDelete...
Posted by gaol <gi...@git.apache.org>.
Github user gaol commented on the pull request:
https://github.com/apache/activemq-artemis/pull/495#issuecomment-217789365
@clebertsuconic any news?
---
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: [ARTEMIS-506] PagingTest#testDelete...
Posted by gaol <gi...@git.apache.org>.
Github user gaol commented on the pull request:
https://github.com/apache/activemq-artemis/pull/495#issuecomment-220559790
@clebertsuconic I think the 3 lines starts from: [PageSubscriptionImpl#reloadPageCompletion()](https://github.com/apache/activemq-artemis/commit/3e2adf123b96c3dfade3d1584ea0ddf65a876941#diff-0f3a0fc675330f6da3b510a9428b6470R196) in your last changes fixed the issue.
I thought there maybe something to do within this method but not realized that the PagingStore can be used that way. :+1:
I think this PR can be closed now.
---
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: [ARTEMIS-506] PagingTest#testDelete...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/495#issuecomment-220460780
This is exactly the issue I was working against \U0001f44d I wish I had read your PR more properly..
I actually did but I didn't realize it was the same issue until I hit it.
Can you evaluate my latest changes and close this? upon restart I check if there is a page complete, and I then move towards the next page. we shouldn't ever add to a page once completed.
---
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: [ARTEMIS-506] PagingTest#testDelete...
Posted by mtaylor <gi...@git.apache.org>.
Github user mtaylor commented on the pull request:
https://github.com/apache/activemq-artemis/pull/495#issuecomment-220273890
@clebertsuconic any update on this?
---
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: [ARTEMIS-506] PagingTest#testDelete...
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the pull request:
https://github.com/apache/activemq-artemis/pull/495#issuecomment-215585334
Please don't merge this.. let me the one merging this as I'm doing tests.
---
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.
---