Make fetching nodes faster when getting health checks#365
Make fetching nodes faster when getting health checks#365aliciay64 wants to merge 3 commits intohashicorp:mainfrom
Conversation
Signed-off-by: xyang378 <xyang378@bloomberg.net>
|
|
|
Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement Learn more about why HashiCorp requires a CLA and what the CLA includes Have you signed the CLA already but the status is still pending? Recheck it. |
| } | ||
|
|
||
| nodes, _, err := a.client.Catalog().Nodes(&api.QueryOptions{NodeMeta: a.config.NodeMeta}) | ||
| nodes, _, err := a.client.Catalog().Nodes(&api.QueryOptions{ |
There was a problem hiding this comment.
This is still a partial solution. Instead of passing the NodeMeta, we should actually pass the pingNodes to the filter. Otherwise, we are still retrieving all the Nodes !
But, that would create an issue at scale. Lets say 200 number of Nodes.
So, the real fix would be Consul should expose a dedicated API to retrieve all the nodes corresponds to that specific ESM instance, as Consul Server can read from the KV store and filter only the relevant nodes. cc @shashankNandigama
This looks good for now.
There was a problem hiding this comment.
Cool, if you can implement this as a new API on the consul side, that would be more straightforward, also saves the retrieval from KV store.
One concern is that this new API will still put more load on consul leader, so we want to enable stale reads in ESM when switching to that API. I'll make the stale reads on nodes a configurable option
Can you also link the Issue/PR for that new Consul API here? Thanks!
2 participants
Description
During fetching health checks, currently ESM instances will the whole nodes catalogue, then filter down to the list of nodes it needs at the client side (ESM followers). With a large catalogue and many instances of ESM in a cluster, this becomes very inefficient for network I/O and other system resources.
Changing the nodes fetch call to pass a regex filter, so that the consul server only returns the list of nodes that each ESM instance is responsible for. Also enabled stale read for this call to distribute the filtering work among consul server followers, avoid overloading consul leader. The 'stale' read is actually not that stale, compared to how slow health check propagation was to ESM instances.
Overall this improves processing time and system's max capacity, below are results from testing this change vs the existing code locally. We also observed in our production environment that this change reduces both consul server cluster's and ESM cluster's machine's CPU, memory and network I/O by 50-80%.
There are no additional steps needed to revert this change, also no change to security controls from this PR.
PCI review checklist
I have documented a clear reason for, and description of, the change I am making.
If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.
If applicable, I've documented the impact of any changes to security controls.
Examples of changes to security controls include using new access control methods, adding or removing logging pipelines, etc.