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
ISPN-10975 Transactional cache will slow down the expiration reaper #7610
ISPN-10975 Transactional cache will slow down the expiration reaper #7610
Conversation
CompletableFuture<Void> future; | ||
if (transactional) { | ||
// Transactional is still blocking - to be removed later | ||
future = CompletableFuture.supplyAsync(() -> cacheToUse.removeLifespanExpired(key, value, lifespan), asyncExecutor) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a NullPointer with this fix if the reaper is executed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be fixed now; added the missing @Inject
c965ec9
to
916ad8c
Compare
916ad8c
to
362593b
Compare
CompletableFuture<Void> future = cacheToUse.removeLifespanExpired(key, value, lifespan); | ||
CompletableFuture<Void> future; | ||
if (transactional) { | ||
// Transactional is still blocking - to be removed later |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO we could make cache.removeLifespanExpired()
submit a task to an executor internally when it's not part of a transaction, but the only way to make it realy non-blocking is to avoid any interaction with JTA, at which point we might as well remove it from the AdvancedCache
API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think we may have to remove it from the API to more cleanly fix this in the future. This is mostly just a stop gap until that can be done.
Merged, thanks |
https://issues.redhat.com/browse/ISPN-10975