Add node shard allocation awareness by default #3380
Conversation
anyasabo
added
>enhancement
There was a problem hiding this comment.
LGTM
I did a few tests, with several nodes deployed on a same K8S node, and it seems to be working as expected 👍
I was just wondering if it's worth updating our documentation to mention this default behavior ?
|
@alaudazzi if you can take a look at the docs changes I'd appreciate it |
| @@ -16,6 +17,8 @@ import ( | |||
| "github.com/elastic/cloud-on-k8s/pkg/controller/elasticsearch/volume" | |||
| ) | |||
|
|
|||
| var nodeAttrNodeName = fmt.Sprintf("%s.%s", esv1.NodeAttr, EnvNodeName) | |||
There was a problem hiding this comment.
I believe typically these node.attrs are lowercase (just a nit). Also should we try to make it clear that NODE in this context does not mean the Elasticsearch node but the k8s node by maybe using a slightly different name?
There was a problem hiding this comment.
Do you think just k8s_node_name? I went this way to minimize the amount of mappings you have to keep in mind between ES attr name -> env var name -> downward api value. I can see that that might not be a big advantage over your proposed approach though
There was a problem hiding this comment.
I think I'd be +1 with k8s_node_name.
Fix #2827
Should fix issues where multiple pods in the same ES cluster get scheduled to the same k8s node, but ES does not know so it allocates both the primary and replica shards to pods on the same k8s node. We already have docs for setting up AZ awareness, but that would require extra information that ECK does not have. We can depend on having the node name available, so we can do this by default.