Repository navigation
Replies: 8 comments 33 replies
|
Hi @ioemat , thanks for your interest! Additionally, and as a proof, we developed a detailed testing scenario: to validate that the setup is working as expected. cc. @vmuzikar |
I was talking about analysing issues like, pod b restarted, pod a became coordinator, pod a transferred all session data to pod b and ran into an out of memory exceptions. Issues you might have to analyse because Keycloak pods interact with each other compared to traditional 12 factor apps that put all the difficult stateful stuff into some backends.
So you are telling me I have to do a lot of extra work to get something I am getting for free with Stateful Sets? ;)
Once Keycloak is truly stateless (which I doubt ever happens for good reasons), I am fine with a Deployment. So far I can only see disadvantages and extra work I have to do. |
I fail to understand how having unique names for pods makes those analyses harder rather than simpler. If a pod gets restarted you have immediate evidence due to the new name instead of relying on more aggregated metrics.
Giving more work to do to our users is never going to be the objective. |
|
This is a great discussion and I wanted to chime in. Disclosure: currently our keycloak deployments are running Stateful Sets and they are the exception everything else we run is using Deployments.
One example where the Stateful Sets are needed at the current state in my opinion is:
I think many of the points I mentioned above depend on keycloak keeping state in infinispan. If that is not an issue which is the plan I believe, Deployment will be preferred. Of course there is a chance I am completely wrong a my assessment too. |
|
Hi @rh-iatanaso and thanks a lot for bringing those arguments to the attention! A general observation that we forgot to mention is that a Regarding your specific observations, I would say that configuring I agree that this discussion is strongly biased(for me) by the fact that we plan to remove the embedded Infinispan and give the opportunity to deploy it standalone (e.g. externally from Keycloak). Something that we haven't explored so far, is how much it will cost in terms of maintenance to push this decision to the users (at least until Infinispan is embedded) in the form of a field in the CRD. |
|
@andreaTP Is it a good practice to configure node affinity to pods? I always thought it is a bad practice and you should avoid as much as possible influencing the scheduler's decisions. |
|
@andreaTP What do you think about making this configurable? |
|
@andreaTP I would totally appreciate that. We could probably also support. |
|
IMO the Operator should be opinionated, so I don't think it should be configurable by the user. I tend to lean more towards the Deployment instead of SS as we should rather move KC towards stateless mindset (with Infinispan being external in the future) but I can imagine if there are solid reasons for SS in the current state, I'd go this path for now. |
|
@vmuzikar But you could also be opinionated by providing a solid default with the ability to override? |
|
My reasoning here is that if we decided to make it configurable, it'd add additional complexity and maintenance costs to the Operator. Not sure if the benefit of having it configurable would justify this. As I mentioned, maybe we should rather consider switching to SS if there are strong enough reasons for it. |
I think this is extremely correct, and, in this case, the complexity of the code to achieve it, is not huge. |
|
Follow up, I quickly hacked around a version of the operator capable of switching between I would be happy to hear feedback on this! |
|
cc. @ahus1 FYI you might be interested into trying this out |
|
Wow, that was quick! |
|
@andreaTP Infinispan operator seems to be using stateful set https://infinispan.org/docs/infinispan-operator/main/operator.html. Are there differences when running DG and embedding that imposes a specific workload object? Also, once we support zero-downtime upgrades, isn't ordered and automatic rolling upgrades a must? |
|
+1 for using a statefulset (at least as long as Infinispan is included in Keycloak) to prevent lost session information on restarting instances. This could happen on scaling down and on redeployments. I did a test of scaling from 1 instance to 10 instances of Keycloak running in clustered mode with the standard nightly build of the new operator: I see messages like "no physical address", so also scaling up seems to have some problems with the deployment. Here the detailed logs:
Those warnings seemed to stall the full start of the node. Could that work better with the statefulset? |
|
We discussed the eventuality of "data loss" in case of actual disruption, given @ryanemerson answer unless we:
This is going to happen anyhow. @ahus1 regarding:
let's separately discuss it, gut feeling is that this is not related, you can anyhow test out the |
|
I'll give the branch a try. To reply to the comment: I agree that Keycloak should not use a PV/PVC as those are a huge infrastructure request. I don't see the impact of a StatefulSet without PV/PVC, yet. |
yup. I too think the same, it does translate to bad user experience ( besies its the core use case of keycloak) |
|
I have to stress that, losing the sessions:
|
|
Thanks, @ryanemerson. Doesn't the #2 point (scaling up/down by one pod at a time) above also apply to embedded? I mean, considering that in the embedded mode we are also relying on distributable caches, scaling down too quickly might still result in data loss if a rebalance cannot complete before You're right that data loss is not so catastrophic but it has a direct impact on UX and user-facing availability. I'm not going to mention the impact on upgrades because that is something we are improving with the new store and there we won't have an embedded Infinispan anymore. I'm not opinionated about SS or Deployment and I agree we need feedback from others and take into account the feedback from them when managing a production-grade cluster. One important point you have though is that the operator should be able to solve some of the issues with using a deployment because in the end, it will be managing the workload. So perhaps using a deployment gives more flexibility to the operator in terms of the best approach for us for scaling. |
|
I strongly appreciate everyone's feedback! NOTE: Key takeaways(so far):
As an additional note, I want to share that the work done on Store.X will enable Keycloak to use an external Infinispan installation lowering, even more, the requirements for Last but not least, I want to remember that we are talking about the "opinionated default of the Keycloak operator" and there is absolutely no artificial barrier preventing from deploying Keycloak using |
|
Considering the long-term plan toward a stateless deployment, I agree a deployment makes sense. And I also agree that we should be considering statefulset if we find out it is more suitable for the legacy store and during the period the legacy store is not completely removed. If a deployment can work with both stores, even better. |
|
@andreaTP Correction on your point 2 takeaway. They agree that it is fine as long as you are able to have only one pod join and leave at a time and let Infinispan finish its rebalance.. This is something a stateful set does out of the box. It might be possible to get the same effect using a deployment with |
|
@sschu thanks for the comment! |
|
As I have written in #12447 (comment), the operator would also have to apply scaling changes incrementally to the deployment to get a safe behaviour similar to a stateful set. I am not sure it makes sense to do this yourself. |
|
I have tested and I can confirm that the settings This is a compelling argument that prevents us from fulfilling Infinispan requirements using a plain |
|
We finally reached the bottom line of this thread. You can track the development of this change here: #12700 |
|
Hello. What exactly are the consequences though? If you use a deployment instead of a statefulset and you have multiple pods leaving the cluster at once, what negative impact does that have? |
|
@paul-civitas see the doc note - scaling down too aggressively can be problematic given the replication factor, and there have been several issues noted with members joining simultaneously - however the infinispan team is working towards addressing that. So for now it's been easiest to just use a StatefulSet. In the future we may be able to use Deployments and pod disruption budgets. |
|
@shawkins yes but which data specifically can be lost this way? Dataloss isn't always a disaster depending on the data. In particular for cache data loss, the only consequence is cache misses. What are the several issues that have been noted? As you scale up it gets harder and harder to avoid multiple members joining or leaving concurrently. |
Session data. This should be a non-issue once persistent sessions are commonly in use.
https://issues.redhat.com/browse/ISPN-15432 There's also a concern about how expensive rebalancing can be - there again that's something persistent sessions may be able eventually address, but I'm not sure if it's there yet. Since this discussion is quite old, if you want to revisit this based upon specific concerns that would be relevant to the planning for KC 27 and later, it would be best to open a new discussion. |
|
In Keycloak 26, sessions are stored in the database and should be safe. However, some data is only stored in the caches:
Stateful sets reduce the risks above while deployment increases them. Improving the Infinispan handling of concurrent nodes leaving/joining is an outgoing process. Unfortunately, we are still finding issues with it: https://issues.redhat.com/browse/ISPN-16531 |
|
Hi everyone, thanks for the insightful discussion here! |
|
Yes, they are related to that issue. We still want to have a fix for that issue that concurrent starts work without problems. BTW, those watching this issue, there is now a Kubernetes stateful-set example linked in https://www.keycloak.org/getting-started/getting-started-kube |
Uh oh!
There was an error while loading. Please reload this page.
Hi keycloak community,
the keycloak operator currently creates the workloads as deployment (see) which means that the pods are replaced by a new ones when they are restarted.
In my understanding JGroups Cluster is designed to work with stable node identities like they are provided by statefulset.
Using a statefulset also ensures that pods are created sequentially, which probably lowers the risk of race conditions for the initial startup.
What are the advantages of deployment, why has this been chosen for the operator?
All reactions