keycloak / keycloak-benchmark

Keycloak Benchmark
https://www.keycloak.org/keycloak-benchmark/
Apache License 2.0
125 stars 68 forks source link

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

Closed mhajas closed 1 month ago

mhajas commented 1 month ago

…ted to the second site anyway

ahus1 commented 1 month ago

@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 commented 1 month ago

@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 commented 1 month ago

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 commented 1 month ago

@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 commented 1 month ago

@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 commented 1 month ago

@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: https://github.com/keycloak/keycloak/issues/29750

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

mhajas commented 1 month ago

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?

ahus1 commented 1 month ago

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