set worker_name compatible with k8s horizontal pod autoscaling #19285
Conversation
|
iops seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
| if self.worker_app: | ||
| self.worker_name = config.get("worker_name", environ.get("HOSTNAME")) | ||
| self.instance_name = self.worker_name or MAIN_PROCESS_INSTANCE_NAME | ||
|
|
There was a problem hiding this comment.
@gaelgatelement I'm curious on your opinion about this kind of change being familiar deploying Synapse with K8s
| self.worker_name = self.worker_app | ||
| if self.worker_app: | ||
| self.worker_name = config.get("worker_name", environ.get("HOSTNAME")) |
There was a problem hiding this comment.
Is this equivalent? It seems like this will always get overriden when self.worker_app is set.
| self.worker_name = self.worker_app | |
| if self.worker_app: | |
| self.worker_name = config.get("worker_name", environ.get("HOSTNAME")) | |
| if self.worker_app: | |
| self.worker_name = config.get("worker_name", environ.get("HOSTNAME")) |
| self.worker_replication_secret = worker_replication_secret | ||
|
|
||
| self.worker_name = config.get("worker_name", self.worker_app) | ||
| from os import environ |
There was a problem hiding this comment.
This should be organized into the imports at the top of the file
| from os import environ | ||
| self.worker_name = self.worker_app | ||
| if self.worker_app: | ||
| self.worker_name = config.get("worker_name", environ.get("HOSTNAME")) |
There was a problem hiding this comment.
This changes the previous behavior where we previously read from config.get("worker_name") and had a fallback to the self.worker_app.
Now it will config.get("worker_name") -> environ.get("HOSTNAME")
Overall, I'm not very convinced of the change yet.
There was a problem hiding this comment.
This for statically scaled worker, that registered in master worker. We use state full sets to have predictable names
| if self.worker_app: | ||
| self.worker_name = config.get("worker_name", environ.get("HOSTNAME")) | ||
| self.instance_name = self.worker_name or MAIN_PROCESS_INSTANCE_NAME | ||
|
|
There was a problem hiding this comment.
CLA needs to be signed
There was a problem hiding this comment.
CLA needs to be signed
I signed it as GH user, but git remembers my local Linux user as co-author . Should I rewrite PR and send it with correct .gitconfig?
There was a problem hiding this comment.
@shcherbak Perhaps easiest 🤷
Sorry for the hassle
3 participants
master worker needs to "know" names of workers by type. this patch allows to use kubernetes HPA because worker name is based on hostname which is equals to pod name in kubernetes