search

keycloak / keycloak-benchmark

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

Deploy Infinispan with maxCount=10000 and num_owners=1 #818

Closed ryanemerson closed 1 month ago

ryanemerson commented 2 months ago

Resolves #805

ahus1 commented 1 month ago

@ryanemerson - update to my previous comment: The PR https://github.com/keycloak/keycloak/pull/29597 might still allow for an external Infinispan with maxCount, let's continue the discussion next week.

mhajas commented 1 month ago

@ryanemerson Is ISPN operator able to update existing caches with maxCount and owners? I remember that some time ago when I tried to change something in CR Operator refused to change it.

ryanemerson commented 1 month ago

@ryanemerson Is ISPN operator able to update existing caches with maxCount and owners? I remember that some time ago when I tried to change something in CR Operator refused to change it.

Yes this is possible when the default strategy of spec.updates.strategy: retain is used on the Cache CR. The Operator will attempt to update the cache configuration on the server to equal that of the CR, however the operation will fail if the old and new configuration are incompatible for a runtime update.

https://infinispan.org/docs/infinispan-operator/main/operator.html#updating_cache_creating-caches

ahus1 commented 1 month ago

@ryanemerson - the issues to handle the expiry of sessions is now resolved as the other PR is merged.

Could you please now attend to the other issue: Your changes should only apply when persistent sessions are enabled, as limiting the number of entries is otherwise not supported.

Thanks!

ryanemerson commented 1 month ago

Suggestions added

ryanemerson commented 1 month ago

Your changes should only apply when persistent sessions are enabled, as limiting the number of entries is otherwise not supported.

It should be good now. If KC_PERSISTENT_SESSIONS is true when provisioning the Infinispan cluster then cache configs are updated accordingly.

kami619 commented 1 month ago

merging it, as its approved by @pruivo