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.

Sign up for GitHub

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

Jump to bottom

Use cache.compute() method to improve the replace retry loop #29073

Closed
pruivo opened this issue Apr 25, 2024 · 5 comments · Fixed by #29081
Closed

Use cache.compute() method to improve the replace retry loop #29073

pruivo opened this issue Apr 25, 2024 · 5 comments · Fixed by #29081
Assignees
@pruivo
Labels
kind/enhancement Categorizes a PR related to an enhancement release/24.0.5 release/25.0.0 team/cross-dc

Comments

@pruivo
Copy link
Contributor

pruivo commented Apr 25, 2024 •
edited by ahus1

Description

The Map.replace() returns a boolean to signal if the replaced succeed or not. This is inefficient because KC needs to perform a read operation to fetch the value again.

Using the compute() method, we can retrieve the most recent version and we can do the replacement based on version instead of equality.

Discussion / Motivation

This should fix the root cause for the symptoms observed in #28466
@pruivo pruivo added kind/enhancement Categorizes a PR related to an enhancement status/triage labels Apr 25, 2024
@pruivo pruivo self-assigned this Apr 25, 2024
pruivo added a commit to pruivo/keycloak that referenced this issue Apr 25, 2024
@pruivo
Use cache.compute() method to improve the replace retry loop
799fe55 
Closes keycloak#29073

Signed-off-by: Pedro Ruivo <pruivo@redhat.com>
pruivo added a commit to pruivo/keycloak that referenced this issue Apr 25, 2024
@pruivo
Use cache.compute() method to improve the replace retry loop
9a24d0d 
Closes keycloak#29073

Signed-off-by: Pedro Ruivo <pruivo@redhat.com>
@pruivo pruivo mentioned this issue Apr 25, 2024
Merged
pruivo added a commit to pruivo/keycloak that referenced this issue Apr 25, 2024
@pruivo
Use cache.compute() method to improve the replace retry loop
1b4b843 
Closes keycloak#29073

Signed-off-by: Pedro Ruivo <pruivo@redhat.com>
pruivo added a commit to pruivo/keycloak that referenced this issue Apr 27, 2024
@pruivo
Use cache.compute() method to improve the replace retry loop
257bd42 
Closes keycloak#29073

Signed-off-by: Pedro Ruivo <pruivo@redhat.com>
ahus1 pushed a commit that referenced this issue Apr 29, 2024
@pruivo @ahus1
Use cache.compute() method to improve the replace retry loop
17a700b 
Closes #29073

Signed-off-by: Pedro Ruivo <pruivo@redhat.com>
@ahus1
Copy link
Contributor

ahus1 commented Apr 29, 2024

I like to backport this change to KC24 as it IMHO fixed #28466. As this is a breaking change and it would

@pruivo - please create a first backport which would allow us in a two-step approach to get all necessary classes in a first patch release. Once this is merged and released, there would be a second PR to actually use these classes, IMHO that would be the changes to InfinispanChangelogBasedTransaction.

@ahus1 ahus1 mentioned this issue Apr 29, 2024
Closed
2 tasks
mhajas added a commit to mhajas/keycloak that referenced this issue Apr 29, 2024
@mhajas
Use computeIfPresent also for Persistent sessions
71e461e 
Follow-up-on keycloak#29073

Signed-off-by: Michal Hajas <mhajas@redhat.com>
@mhajas mhajas mentioned this issue Apr 29, 2024
Merged
mhajas added a commit to mhajas/keycloak that referenced this issue May 2, 2024
@mhajas
Use computeIfPresent also for Persistent sessions
702ec06 
Follow-up-on keycloak#29073

Signed-off-by: Michal Hajas <mhajas@redhat.com>
mhajas added a commit to mhajas/keycloak that referenced this issue May 2, 2024
@mhajas
Use computeIfPresent also for Persistent sessions
1ad1127 
Follow-up-on keycloak#29073

Signed-off-by: Michal Hajas <mhajas@redhat.com>
mhajas added a commit to mhajas/keycloak that referenced this issue May 2, 2024
@mhajas
Use computeIfPresent also for Persistent sessions
8b5b4da 
Follow-up-on keycloak#29073

Signed-off-by: Michal Hajas <mhajas@redhat.com>
ahus1 pushed a commit that referenced this issue May 2, 2024
@mhajas
Use computeIfPresent also for Persistent sessions
e93b7d4 
Follow-up-on #29073

Signed-off-by: Michal Hajas <mhajas@redhat.com>
pruivo added a commit to pruivo/keycloak that referenced this issue May 2, 2024
@pruivo
Use cache.compute() method to improve the replace retry loop
0caf155 
This commit only adds the ReplaceFunction to it can be backwards
compatible.

Closes keycloak#29073

Signed-off-by: Pedro Ruivo <pruivo@redhat.com>
@pruivo pruivo mentioned this issue May 2, 2024
Merged
@pruivo
Copy link
Contributor Author

pruivo commented May 2, 2024

Pull Request to KC24 available here: #29229

ahus1 pushed a commit that referenced this issue May 2, 2024
@pruivo @ahus1
Use cache.compute() method to improve the replace retry loop
2b6c331 
This commit only adds the ReplaceFunction to it can be backwards
compatible.

Closes #29073

Signed-off-by: Pedro Ruivo <pruivo@redhat.com>
@ahus1
Copy link
Contributor

ahus1 commented May 2, 2024

On hold for 24.0.4 to be released. Then the second half will be backported.

@ahus1 ahus1 added status/hold PR should not be merged. On hold for later. team/cross-dc and removed status/hold PR should not be merged. On hold for later. labels May 6, 2024
@ahus1
Copy link
Contributor

ahus1 commented May 8, 2024

@pruivo - KC 24.0.4 has been released today. Please provide the second part of the backport. Thanks!

pruivo added a commit to pruivo/keycloak that referenced this issue May 8, 2024
@pruivo
Use cache.compute() method to improve the replace retry loop
442e053 
Final commit to enable the new function.

Closes keycloak#29073

Signed-off-by: Pedro Ruivo <pruivo@redhat.com>
@pruivo pruivo mentioned this issue May 8, 2024
Merged
@pruivo
Copy link
Contributor Author

pruivo commented May 8, 2024

second part here: #29378

ahus1 pushed a commit that referenced this issue May 8, 2024
@pruivo @ahus1
Use cache.compute() method to improve the replace retry loop
36b7b8b 
Final commit to enable the new function.

Closes #29073

Signed-off-by: Pedro Ruivo <pruivo@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@pruivo pruivo

Labels
kind/enhancement Categorizes a PR related to an enhancement release/24.0.5 release/25.0.0 team/cross-dc
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use cache.compute() method to improve the replace retry loop pruivo/keycloak
2 participants
@pruivo @ahus1