From 8f4ea86c13384245231d734226efdfa85127f091 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Fri, 18 Aug 2017 17:09:50 +0200 Subject: [PATCH 01/10] Disable pre-fetching (not needed on exporter queries), limit conn pool size and re-use database connection to MongoDB --- collector/mongodb_collector.go | 29 +++++++++++++++++++++++++---- mongodb_exporter.go | 7 ++++--- shared/connection.go | 2 ++ 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/collector/mongodb_collector.go b/collector/mongodb_collector.go index e7f90ad13..4924ad96b 100644 --- a/collector/mongodb_collector.go +++ b/collector/mongodb_collector.go @@ -15,7 +15,6 @@ package collector import ( - "errors" "fmt" "time" @@ -57,6 +56,7 @@ type MongodbCollector struct { scrapesTotal prometheus.Counter lastScrapeError prometheus.Gauge lastScrapeDurationSeconds prometheus.Gauge + mongoSess *mgo.Session } // NewMongodbCollector returns a new instance of a MongodbCollector. @@ -87,6 +87,28 @@ func NewMongodbCollector(opts MongodbCollectorOpts) *MongodbCollector { return exporter } +// getSession returns the cached *mgo.Session (after a test ping) +// or creates a new session and returns it. The cached session is +// reconnected if the ping to it fails. +func (exporter *MongodbCollector) getSession() *mgo.Session { + if exporter.mongoSess != nil { + err := exporter.mongoSess.Ping() + if err == nil { + return exporter.mongoSess + } + exporter.mongoSess.Close() + } + exporter.mongoSess = shared.MongoSession(exporter.Opts.toSessionOps()) + return exporter.mongoSess +} + +// Close cleanly closes the mongo session if it exists. +func (exporter *MongodbCollector) Close() { + if exporter.mongoSess != nil { + exporter.mongoSess.Close() + } +} + // Describe sends the super-set of all possible descriptors of metrics collected by this Collector // to the provided channel and returns once the last descriptor has been sent. // Part of prometheus.Collector interface. @@ -139,12 +161,11 @@ func (exporter *MongodbCollector) scrape(ch chan<- prometheus.Metric) { } }(time.Now()) - mongoSess := shared.MongoSession(exporter.Opts.toSessionOps()) + mongoSess := exporter.getSession() if mongoSess == nil { - err = errors.New("can't create mongo session") + log.Errorf("can't create mongo session") return } - defer mongoSess.Close() var serverVersion string serverVersion, err = shared.MongoSessionServerVersion(mongoSess) diff --git a/mongodb_exporter.go b/mongodb_exporter.go index 018f2fdf6..15dc136e9 100644 --- a/mongodb_exporter.go +++ b/mongodb_exporter.go @@ -154,8 +154,8 @@ func startWebServer() { } handler := prometheusHandler() - - registerCollector() + collector := registerCollector() + defer collector.Close() if (*sslCertFileF == "") != (*sslKeyFileF == "") { log.Fatal("One of the flags -web.ssl-cert-file or -web.ssl-key-file is missing to enable HTTPS/TLS") @@ -210,7 +210,7 @@ func startWebServer() { } } -func registerCollector() { +func registerCollector() *collector.MongodbCollector { mongodbCollector := collector.NewMongodbCollector(collector.MongodbCollectorOpts{ URI: *uriF, TLSConnection: *tlsF, @@ -220,6 +220,7 @@ func registerCollector() { TLSHostnameValidation: !(*tlsDisableHostnameValidationF), }) prometheus.MustRegister(mongodbCollector) + return mongodbCollector } func main() { diff --git a/shared/connection.go b/shared/connection.go index 1aaa5acde..91eee774f 100644 --- a/shared/connection.go +++ b/shared/connection.go @@ -76,6 +76,8 @@ func MongoSession(opts MongoSessionOpts) *mgo.Session { return nil } session.SetMode(mgo.Eventual, true) + session.SetPoolLimit(2) + session.SetPrefetch(0.00) session.SetSyncTimeout(syncMongodbTimeout) session.SetSocketTimeout(0) return session From 0faf0015bbc57a00fcb757402ddd82368cd131b1 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Fri, 18 Aug 2017 19:04:35 +0200 Subject: [PATCH 02/10] Remove .Ping() on exporter.mongoSess, mgo.Session will recreate the conn for us --- collector/mongodb_collector.go | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/collector/mongodb_collector.go b/collector/mongodb_collector.go index 4924ad96b..db6587188 100644 --- a/collector/mongodb_collector.go +++ b/collector/mongodb_collector.go @@ -87,16 +87,10 @@ func NewMongodbCollector(opts MongodbCollectorOpts) *MongodbCollector { return exporter } -// getSession returns the cached *mgo.Session (after a test ping) -// or creates a new session and returns it. The cached session is -// reconnected if the ping to it fails. +// getSession returns the cached *mgo.Session or creates a new session and returns it. func (exporter *MongodbCollector) getSession() *mgo.Session { if exporter.mongoSess != nil { - err := exporter.mongoSess.Ping() - if err == nil { - return exporter.mongoSess - } - exporter.mongoSess.Close() + return exporter.mongoSess } exporter.mongoSess = shared.MongoSession(exporter.Opts.toSessionOps()) return exporter.mongoSess @@ -163,7 +157,7 @@ func (exporter *MongodbCollector) scrape(ch chan<- prometheus.Metric) { mongoSess := exporter.getSession() if mongoSess == nil { - log.Errorf("can't create mongo session") + log.Errorf("can't create mongo session to %s", exporter.Opts.URI) return } From 654f1684b38bc96e41a6edcca3c6422f2a72901d Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Fri, 18 Aug 2017 19:56:46 +0200 Subject: [PATCH 03/10] avoid race condition with sync.Mutex --- collector/mongodb_collector.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/collector/mongodb_collector.go b/collector/mongodb_collector.go index db6587188..9b11b438b 100644 --- a/collector/mongodb_collector.go +++ b/collector/mongodb_collector.go @@ -16,6 +16,7 @@ package collector import ( "fmt" + "sync" "time" "github.com/prometheus/client_golang/prometheus" @@ -57,6 +58,7 @@ type MongodbCollector struct { lastScrapeError prometheus.Gauge lastScrapeDurationSeconds prometheus.Gauge mongoSess *mgo.Session + mongoSessLock sync.Mutex } // NewMongodbCollector returns a new instance of a MongodbCollector. @@ -89,6 +91,10 @@ func NewMongodbCollector(opts MongodbCollectorOpts) *MongodbCollector { // getSession returns the cached *mgo.Session or creates a new session and returns it. func (exporter *MongodbCollector) getSession() *mgo.Session { + // avoid race condition around session creation + exporter.mongoSessLock.Lock() + defer exporter.mongoSessLock.Unlock() + if exporter.mongoSess != nil { return exporter.mongoSess } From 37aa2f1643497b116b3b4f37f3985e91644b7c89 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Fri, 18 Aug 2017 20:06:49 +0200 Subject: [PATCH 04/10] Use lock on close of mgo session also --- collector/mongodb_collector.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/collector/mongodb_collector.go b/collector/mongodb_collector.go index 9b11b438b..9c3fd3996 100644 --- a/collector/mongodb_collector.go +++ b/collector/mongodb_collector.go @@ -105,6 +105,8 @@ func (exporter *MongodbCollector) getSession() *mgo.Session { // Close cleanly closes the mongo session if it exists. func (exporter *MongodbCollector) Close() { if exporter.mongoSess != nil { + exporter.mongoSessLock.Lock() + defer exporter.mongoSessLock.Unlock() exporter.mongoSess.Close() } } From 20f6bee01ea083c64a82ac890f5a69be10b9cf24 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Fri, 18 Aug 2017 20:10:40 +0200 Subject: [PATCH 05/10] Use lock on close of mgo session also #2 --- collector/mongodb_collector.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/collector/mongodb_collector.go b/collector/mongodb_collector.go index 9c3fd3996..4d107ce95 100644 --- a/collector/mongodb_collector.go +++ b/collector/mongodb_collector.go @@ -106,8 +106,8 @@ func (exporter *MongodbCollector) getSession() *mgo.Session { func (exporter *MongodbCollector) Close() { if exporter.mongoSess != nil { exporter.mongoSessLock.Lock() - defer exporter.mongoSessLock.Unlock() exporter.mongoSess.Close() + exporter.mongoSessLock.Unlock() } } From 240a0868eaf8c283fdd7c2ab70c678fb57f71013 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Fri, 18 Aug 2017 20:27:53 +0200 Subject: [PATCH 06/10] make pool limit a flag instead of hard-code --- collector/mongodb_collector.go | 2 ++ mongodb_exporter.go | 3 +++ shared/connection.go | 3 ++- 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/collector/mongodb_collector.go b/collector/mongodb_collector.go index 4d107ce95..bb2886831 100644 --- a/collector/mongodb_collector.go +++ b/collector/mongodb_collector.go @@ -38,6 +38,7 @@ type MongodbCollectorOpts struct { TLSPrivateKeyFile string TLSCaFile string TLSHostnameValidation bool + DbPoolLimit int } func (in MongodbCollectorOpts) toSessionOps() shared.MongoSessionOpts { @@ -48,6 +49,7 @@ func (in MongodbCollectorOpts) toSessionOps() shared.MongoSessionOpts { TLSPrivateKeyFile: in.TLSPrivateKeyFile, TLSCaFile: in.TLSCaFile, TLSHostnameValidation: in.TLSHostnameValidation, + PoolLimit: in.DbPoolLimit, } } diff --git a/mongodb_exporter.go b/mongodb_exporter.go index 15dc136e9..df9237da5 100644 --- a/mongodb_exporter.go +++ b/mongodb_exporter.go @@ -63,6 +63,8 @@ var ( " \tIf not provided: System default CAs are used.") tlsDisableHostnameValidationF = flag.Bool("mongodb.tls-disable-hostname-validation", false, "Do hostname validation for server connection.") + dbPoolLimit = flag.Int("mongodb.max-connections", 2, "Max number of pooled connections to the database.") + // FIXME currently ignored enabledGroupsFlag = flag.String("groups.enabled", "asserts,durability,background_flushing,connections,extra_info,global_lock,index_counters,network,op_counters,op_counters_repl,memory,locks,metrics", "Comma-separated list of groups to use, for more info see: docs.mongodb.org/manual/reference/command/serverStatus/") ) @@ -218,6 +220,7 @@ func registerCollector() *collector.MongodbCollector { TLSPrivateKeyFile: *tlsPrivateKeyF, TLSCaFile: *tlsCAF, TLSHostnameValidation: !(*tlsDisableHostnameValidationF), + DbPoolLimit: *dbPoolLimit, }) prometheus.MustRegister(mongodbCollector) return mongodbCollector diff --git a/shared/connection.go b/shared/connection.go index 91eee774f..7f25359f7 100644 --- a/shared/connection.go +++ b/shared/connection.go @@ -52,6 +52,7 @@ type MongoSessionOpts struct { TLSPrivateKeyFile string TLSCaFile string TLSHostnameValidation bool + PoolLimit int } func MongoSession(opts MongoSessionOpts) *mgo.Session { @@ -76,7 +77,7 @@ func MongoSession(opts MongoSessionOpts) *mgo.Session { return nil } session.SetMode(mgo.Eventual, true) - session.SetPoolLimit(2) + session.SetPoolLimit(opts.PoolLimit) session.SetPrefetch(0.00) session.SetSyncTimeout(syncMongodbTimeout) session.SetSocketTimeout(0) From ead27f7395235548c2e3fdae6f8ca0b4da80d118 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Fri, 18 Aug 2017 20:46:07 +0200 Subject: [PATCH 07/10] default to 1 connection, it seems to work fine even at high volume --- mongodb_exporter.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mongodb_exporter.go b/mongodb_exporter.go index df9237da5..ed7f3589f 100644 --- a/mongodb_exporter.go +++ b/mongodb_exporter.go @@ -63,7 +63,7 @@ var ( " \tIf not provided: System default CAs are used.") tlsDisableHostnameValidationF = flag.Bool("mongodb.tls-disable-hostname-validation", false, "Do hostname validation for server connection.") - dbPoolLimit = flag.Int("mongodb.max-connections", 2, "Max number of pooled connections to the database.") + dbPoolLimit = flag.Int("mongodb.max-connections", 1, "Max number of pooled connections to the database.") // FIXME currently ignored enabledGroupsFlag = flag.String("groups.enabled", "asserts,durability,background_flushing,connections,extra_info,global_lock,index_counters,network,op_counters,op_counters_repl,memory,locks,metrics", "Comma-separated list of groups to use, for more info see: docs.mongodb.org/manual/reference/command/serverStatus/") From c2be7bee58c2f51755f919548b1527eec934b7ed Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Sat, 19 Aug 2017 18:02:07 +0200 Subject: [PATCH 08/10] Must return .Copy() of mgo.Session to avoid mgo deadlocking under concurrent calls. Moved session SocketTimeout to no longer be forever --- collector/mongodb_collector.go | 6 +++--- shared/connection.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/collector/mongodb_collector.go b/collector/mongodb_collector.go index bb2886831..995780c3b 100644 --- a/collector/mongodb_collector.go +++ b/collector/mongodb_collector.go @@ -92,16 +92,16 @@ func NewMongodbCollector(opts MongodbCollectorOpts) *MongodbCollector { } // getSession returns the cached *mgo.Session or creates a new session and returns it. +// Use sync.Mutex to avoid race condition around session creation. func (exporter *MongodbCollector) getSession() *mgo.Session { - // avoid race condition around session creation exporter.mongoSessLock.Lock() defer exporter.mongoSessLock.Unlock() if exporter.mongoSess != nil { - return exporter.mongoSess + return exporter.mongoSess.Copy() } exporter.mongoSess = shared.MongoSession(exporter.Opts.toSessionOps()) - return exporter.mongoSess + return exporter.mongoSess.Copy() } // Close cleanly closes the mongo session if it exists. diff --git a/shared/connection.go b/shared/connection.go index 7f25359f7..ccb8fd96d 100644 --- a/shared/connection.go +++ b/shared/connection.go @@ -80,7 +80,7 @@ func MongoSession(opts MongoSessionOpts) *mgo.Session { session.SetPoolLimit(opts.PoolLimit) session.SetPrefetch(0.00) session.SetSyncTimeout(syncMongodbTimeout) - session.SetSocketTimeout(0) + session.SetSocketTimeout(dialMongodbTimeout) return session } From 832957829804a87fcf178ad51090e138513dc227 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Sat, 19 Aug 2017 18:33:42 +0200 Subject: [PATCH 09/10] Must defer .Close() on copied mgo.Session from .getSession --- collector/mongodb_collector.go | 1 + 1 file changed, 1 insertion(+) diff --git a/collector/mongodb_collector.go b/collector/mongodb_collector.go index 995780c3b..2c6f3d0c2 100644 --- a/collector/mongodb_collector.go +++ b/collector/mongodb_collector.go @@ -170,6 +170,7 @@ func (exporter *MongodbCollector) scrape(ch chan<- prometheus.Metric) { log.Errorf("can't create mongo session to %s", exporter.Opts.URI) return } + defer mongoSess.Close() var serverVersion string serverVersion, err = shared.MongoSessionServerVersion(mongoSess) From cedab6ba8817b8c5196ccfb0e88254dffc287fd7 Mon Sep 17 00:00:00 2001 From: Tim Vaillancourt Date: Sat, 19 Aug 2017 22:44:49 +0200 Subject: [PATCH 10/10] must check if != nil a 2nd time in getSession() --- collector/mongodb_collector.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/collector/mongodb_collector.go b/collector/mongodb_collector.go index 2c6f3d0c2..79f1f726b 100644 --- a/collector/mongodb_collector.go +++ b/collector/mongodb_collector.go @@ -101,7 +101,10 @@ func (exporter *MongodbCollector) getSession() *mgo.Session { return exporter.mongoSess.Copy() } exporter.mongoSess = shared.MongoSession(exporter.Opts.toSessionOps()) - return exporter.mongoSess.Copy() + if exporter.mongoSess != nil { + return exporter.mongoSess.Copy() + } + return nil } // Close cleanly closes the mongo session if it exists.