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-5305 Improving performance of paging with multiple producers #5498

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

clebertsuconic
Copy link
Contributor

This is improving how locking works by using a single threaded executor on paging and depaging
as well as batching writes instead of directly writing on the files from the producers.

@clebertsuconic clebertsuconic force-pushed the page branch 10 times, most recently from 3ce16c6 to fbf0910 Compare February 12, 2025 02:41
@clebertsuconic clebertsuconic force-pushed the page branch 9 times, most recently from d8e2767 to 483f513 Compare February 12, 2025 17:57
@tabish121
Copy link
Contributor

Seems to be a heap dump attached to the second commit which is probably not meant to be there

@clebertsuconic
Copy link
Contributor Author

I have done a lot of review myself, trying to pretend I was someone else (like Robbie) and review my own stuff.. and I reach a limit ... I can't find anything else :)

Tests are extremely stable.. done a lot of testing...

@clebertsuconic clebertsuconic force-pushed the page branch 2 times, most recently from 574dc22 to b01f91a Compare February 18, 2025 16:46
@clebertsuconic clebertsuconic force-pushed the page branch 2 times, most recently from 0752f44 to 0cdd5fb Compare February 18, 2025 20:35
@clebertsuconic clebertsuconic marked this pull request as draft February 19, 2025 05:59
@clebertsuconic
Copy link
Contributor Author

Right now there are two operation ways.. one for unit tests (PagingStoreImpl).

I will make changes on PagingStoreImpl to account for a SCheduledExecutor.

@clebertsuconic
Copy link
Contributor Author

that way there's only going to be one way to lock this... by being single threaded through the PagingStore::executor (the PageTimedWriter also uses the same executor).

@clebertsuconic clebertsuconic force-pushed the page branch 15 times, most recently from e2f4d4f to 88ca17b Compare February 19, 2025 18:15
@clebertsuconic clebertsuconic marked this pull request as ready for review February 19, 2025 18:15
@clebertsuconic
Copy link
Contributor Author

I reworked the tests in a way to not require two types of locking. Basically it's pretty much single threaded now (through pageExecutor).

This is improving how locking works by using a single threaded executor on paging and depaging
as well as batching writes instead of directly writing on the files from the producers.
@clebertsuconic
Copy link
Contributor Author

@gemmellr can we merge this?

I wanted to merge this before the javadoc huge change by Justin... besides I will have to bring this to an internal branch of mine based on a previous version and I need to start working on that....
the reason I wanted to do before the javadoc also is that it would make my life easier I guess on the cherry-manual-pick.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants