Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ARTEMIS-3761 Improve page cleanup to remove messages in the middle of the stream #4015

Merged
merged 3 commits into from Apr 7, 2022

Conversation

clebertsuconic
Copy link
Contributor

No description provided.

@clebertsuconic clebertsuconic force-pushed the page_cleaner branch 2 times, most recently from baf8ea5 to a6b91c7 Compare April 6, 2022 03:50
@clebertsuconic clebertsuconic marked this pull request as draft April 6, 2022 15:38
@clebertsuconic
Copy link
Contributor Author

I'm adding more tests...

but I don't plan to make any changes to the code... feel free to review it.

@clebertsuconic clebertsuconic force-pushed the page_cleaner branch 4 times, most recently from 0349245 to 99b4b17 Compare April 6, 2022 20:57
@clebertsuconic clebertsuconic marked this pull request as ready for review April 6, 2022 21:03
@clebertsuconic clebertsuconic force-pushed the page_cleaner branch 5 times, most recently from 988d885 to a51f0b3 Compare April 6, 2022 21:15
@clebertsuconic clebertsuconic marked this pull request as draft April 6, 2022 22:30
@clebertsuconic
Copy link
Contributor Author

converting to draft as there's a test failing

@clebertsuconic clebertsuconic marked this pull request as ready for review April 7, 2022 01:12
this test was relying on internal details such as number of credits on the link.
The test was flaky and eventually failing or hunging.
@clebertsuconic clebertsuconic force-pushed the page_cleaner branch 3 times, most recently from 93b2dd2 to 54fce9b Compare April 7, 2022 13:06
…ception from the Iteration

QueueImpl::browserIterator could throw NoSuchElementException and this is fixing the iterator
Found this while testing ARTEMIS-3761
… the stream as well

Paging only removes files at the beginning of the stream...
Say you have paged files 1 through 1000...
if all the messages are ack, but one message on file 1 is missing an ack, all the 999 subsequent files would not be removed until all the messages on file 1 is ack.
This was working as engineered, but sometimes devs don't have complete control on their app.
With this improvement we will now remove messages in the middle of the stream as well.

There is also some improvement to how browsing and page work with this
@clebertsuconic clebertsuconic merged commit cfdb710 into apache:main Apr 7, 2022
@clebertsuconic clebertsuconic deleted the page_cleaner branch April 7, 2022 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant