Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consul registry performance improvements #928

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ddreier
Copy link
Contributor

@ddreier ddreier commented Mar 22, 2023

Hi, it's me again trying to improve route update performance for our absurd number of routes. 馃榿

There's two major changes here:

The first is adding "filtering" so that only Consul Health Checks with the applicable Service Tag Prefix (and serf/maintenance checks) are considered during the rest of the process. In our Production environments this brings down the number of checks that Fabio has to look at from ~40k down to ~12k.

The second change is simplifying the passingServices function from an $O(n^5)$ process to $O(n^2)$. Each of the "helper" functions (countChecks isAgentCritical isNodeInMaintenance isServiceInMaintenance) did their own loop over each of the Consul Health Checks which added up in time very quickly.

Both of these changes drastically reduced the amount of time spent processing responses from Consul. In a test using real Consul data but not serving traffic we saw a reduction from around 90 seconds to less than 10 seconds. We've been testing Fabio with these changes in a non-production environment for about a week now, and have not detected any issues.
image
(The increase in makeConfig shown on the graph is due to how I measured it. It includes time waiting to write on the channel back to the main Fabio goroutine. This wasn't an issue before because processing the Consul data took longer than the time needed to build the actual route table (~10s) but the scales have now flipped. Building the Route Table takes longer than processing the data from Consul.)

I also had profiling running during my tests. Here's stock Fabio:
Stock Fabio CPU profile

After adding the Check "filtering":
Consul Health Check filtering CPU profile

After simplifying passingServices:
Filtering plus simplified passingServices

I'm very interested in any feedback about these changes and will do what I can to help satisfy any concerns. Thanks!

Two changes:
1. Limit the initial list of Health Checks to those with the configured Tag Prefix
2. Reduce the amount of loops done in passingServices
@ddreier
Copy link
Contributor Author

ddreier commented Apr 3, 2023

Hi @nathanejohnson and @KTruesdellENA, just wanted to check in and see if you have any feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant