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

Test delete entry in remote Infinispan and check removals are replica… #823

Merged
merged 3 commits into from
May 22, 2024

Conversation

mhajas
Copy link
Contributor

@mhajas mhajas commented May 20, 2024

…ted to the second site anyway

  • Test logouts
  • Test refreshes

mhajas and others added 2 commits May 21, 2024 10:22
…ted to the second site anyway

Signed-off-by: Michal Hajas <[email protected]>
Signed-off-by: Alexander Schwartz <[email protected]>
@ahus1 ahus1 force-pushed the reproduce-cache-discrepancies branch from d556770 to 804bfa0 Compare May 21, 2024 10:37
@ahus1
Copy link
Contributor

ahus1 commented May 21, 2024

@mhajas - I tested this a bit today, and it works nicely for the logouts.

I verified this also with my local setup where I tested last week: The remove event is propagated to the client for a remote Infinispan 15.0.3, even if the key doesn't exist any more.

When using 14.0.27 which I did locally, I found that the remove events was propagated to the caller only when the entries existed. So it seems that there is a change in behavior between the major versions. Good that we now have a test for this.

So I assume we want people to use ISPN 15.x when running with KC 25?

cc: @ryanemerson @pruivo

UPDATE: I verified that behavior by deploying xsite with 14.0.27 and seeing the the tests testRemoteStoreDiscrepancyMissingSessionInBackupRemoteISPN and testRemoteStoreDiscrepancyMissingSessionInAllRemoteISPN fail.

@pruivo
Copy link
Contributor

pruivo commented May 21, 2024

@ahus1 ISPN 14 will be EOL very soon (except for bug fixes from products, EAP/SSO/RHBK). The main branch is now 15.1 and 15.0 is the supported version.

@ryanemerson
Copy link
Contributor

So I assume we want people to use ISPN 15.x when running with KC 25?

100%

Let's keep the support matrix as small as possible. This will allow us to spend less time on verifying different combinations and more time enhancing the Keycloak experience for users with new improvements/features.

@ahus1
Copy link
Contributor

ahus1 commented May 21, 2024

@ryanemerson / @pruivo - I agree with your comments. Could please also think in the direction if the change in behavior could be seen as a regression by others, and that it would be fixed (to the disadvantage of Keycloak) in an upcoming ISPN release?

@ryanemerson
Copy link
Contributor

ryanemerson commented May 21, 2024

@ahus1 I believe the change in behaviour was introduced by ISPN-15211 and is documented in the Infinispan upgrading guide:

== Client listeners remove events

Client listeners remove events will now be propagated even if the remove did not remove a value.
This is required to properly support new changes around the new `includeOldValue` method on `CacheEventConverter`.
NOTE: Remote events do not include any values by default

@ahus1
Copy link
Contributor

ahus1 commented May 21, 2024

@ryanemerson - thank you for the note that this is an expected behavior.

Given that we now depend on ISPN 15 specific behavior, this should be reflected in the docs and the migration guide. I created an issue for that: keycloak/keycloak#29750

Could you please pick that one up and create a [minimal] docs PR for this? Thanks!

Signed-off-by: Michal Hajas <[email protected]>
@mhajas
Copy link
Contributor Author

mhajas commented May 22, 2024

As agreed we will not provide a test for updates currently so this PR should be ready for reviewing and merging. @ryanemerson @ahus1 Could you please re-review?

@mhajas mhajas marked this pull request as ready for review May 22, 2024 15:12
@ryanemerson ryanemerson self-requested a review May 22, 2024 15:46
@ahus1 ahus1 merged commit 07e7182 into keycloak:main May 22, 2024
2 checks passed
@ahus1
Copy link
Contributor

ahus1 commented May 22, 2024

Thank you for these tests, they really helped us to understand Infinispan's behavior around this!

@mhajas mhajas deleted the reproduce-cache-discrepancies branch May 23, 2024 07:32
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.

None yet

4 participants