From 630a67e7ee29c102b307f753313260fdc1637ca7 Mon Sep 17 00:00:00 2001
From: Frank Schroeder <frank.schroeder@gmail.com>
Date: Sat, 27 Oct 2018 21:45:48 +0200
Subject: [PATCH] consul: fetch route updates concurrently

The code which updates the routing table from consul was using a
single go routine to fetch data from consul. This can be a slow
process if consul has lots of registered services.

This patch adds an option `registry.consul.serviceMonitors`
to increase the concurrency for the route updates.
---
 config/config.go           |  1 +
 config/default.go          |  1 +
 config/load.go             |  5 +++++
 config/load_test.go        |  7 +++++++
 fabio.properties           | 10 ++++++++++
 registry/consul/service.go | 20 ++++++++++++++++++--
 6 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/config/config.go b/config/config.go
index 098985aa5..3dcc30067 100644
--- a/config/config.go
+++ b/config/config.go
@@ -146,6 +146,7 @@ type Consul struct {
 	CheckTLSSkipVerify                  bool
 	CheckDeregisterCriticalServiceAfter string
 	ChecksRequired                      string
+	ServiceMonitors                     int
 }
 
 type Tracing struct {
diff --git a/config/default.go b/config/default.go
index dd1e9df6c..183f7e9d0 100644
--- a/config/default.go
+++ b/config/default.go
@@ -57,6 +57,7 @@ var defaultConfig = &Config{
 			ServiceAddr:                         ":9998",
 			ServiceName:                         "fabio",
 			ServiceStatus:                       []string{"passing"},
+			ServiceMonitors:                     1,
 			CheckInterval:                       time.Second,
 			CheckTimeout:                        3 * time.Second,
 			CheckScheme:                         "http",
diff --git a/config/load.go b/config/load.go
index ec92d81cc..c5a022c4b 100644
--- a/config/load.go
+++ b/config/load.go
@@ -179,6 +179,7 @@ func load(cmdline, environ, envprefix []string, props *properties.Properties) (c
 	f.BoolVar(&cfg.Registry.Consul.CheckTLSSkipVerify, "registry.consul.register.checkTLSSkipVerify", defaultConfig.Registry.Consul.CheckTLSSkipVerify, "service check TLS verification")
 	f.StringVar(&cfg.Registry.Consul.CheckDeregisterCriticalServiceAfter, "registry.consul.register.checkDeregisterCriticalServiceAfter", defaultConfig.Registry.Consul.CheckDeregisterCriticalServiceAfter, "critical service deregistration timeout")
 	f.StringVar(&cfg.Registry.Consul.ChecksRequired, "registry.consul.checksRequired", defaultConfig.Registry.Consul.ChecksRequired, "number of checks which must pass: one or all")
+	f.IntVar(&cfg.Registry.Consul.ServiceMonitors, "registry.consul.serviceMonitors", defaultConfig.Registry.Consul.ServiceMonitors, "concurrency for route updates")
 	f.IntVar(&cfg.Runtime.GOGC, "runtime.gogc", defaultConfig.Runtime.GOGC, "sets runtime.GOGC")
 	f.IntVar(&cfg.Runtime.GOMAXPROCS, "runtime.gomaxprocs", defaultConfig.Runtime.GOMAXPROCS, "sets runtime.GOMAXPROCS")
 	f.StringVar(&cfg.UI.Access, "ui.access", defaultConfig.UI.Access, "access mode, one of [ro, rw]")
@@ -244,6 +245,10 @@ func load(cmdline, environ, envprefix []string, props *properties.Properties) (c
 		cfg.Registry.Consul.CheckScheme = "https"
 	}
 
+	if cfg.Registry.Consul.ServiceMonitors <= 0 {
+		cfg.Registry.Consul.ServiceMonitors = 1
+	}
+
 	if gzipContentTypesValue != "" {
 		cfg.Proxy.GZIPContentTypes, err = regexp.Compile(gzipContentTypesValue)
 		if err != nil {
diff --git a/config/load_test.go b/config/load_test.go
index 29fb9682a..c8f938359 100644
--- a/config/load_test.go
+++ b/config/load_test.go
@@ -618,6 +618,13 @@ func TestLoad(t *testing.T) {
 				return cfg
 			},
 		},
+		{
+			args: []string{"-registry.consul.serviceMonitors", "5"},
+			cfg: func(cfg *Config) *Config {
+				cfg.Registry.Consul.ServiceMonitors = 5
+				return cfg
+			},
+		},
 		{
 			args: []string{"-log.access.format", "foobar"},
 			cfg: func(cfg *Config) *Config {
diff --git a/fabio.properties b/fabio.properties
index f57ef95a7..c86c14c47 100644
--- a/fabio.properties
+++ b/fabio.properties
@@ -750,6 +750,16 @@
 # registry.consul.checksRequired = one
 
 
+# registry.consul.serviceMonitors configures the concurrency for
+# route updates. Fabio will make up to the configured number of
+# concurrent calls to Consul to fetch status data for route
+# updates.
+#
+# The default is
+#
+# registry.consul.serviceMonitors = 1
+
+
 # glob.matching.disabled disables glob matching on route lookups
 # If glob matching is enabled there is a performance decrease
 # for every route lookup.  At a large number of services (> 500) this
diff --git a/registry/consul/service.go b/registry/consul/service.go
index f7d703307..f9ad1067e 100644
--- a/registry/consul/service.go
+++ b/registry/consul/service.go
@@ -71,9 +71,25 @@ func (w *ServiceMonitor) makeConfig(checks []*api.HealthCheck) string {
 		m[name][id] = true
 	}
 
-	var config []string
+	n := w.config.ServiceMonitors
+	if n <= 0 {
+		n = 1
+	}
+
+	sem := make(chan int, n)
+	cfgs := make(chan []string, len(m))
 	for name, passing := range m {
-		cfg := w.serviceConfig(name, passing)
+		name, passing := name, passing
+		go func() {
+			sem <- 1
+			cfgs <- w.serviceConfig(name, passing)
+			<-sem
+		}()
+	}
+
+	var config []string
+	for i := 0; i < len(m); i++ {
+		cfg := <-cfgs
 		config = append(config, cfg...)
 	}