[Metricbeat] Fix Socket leak in postgresql module #11419
Conversation
| @@ -102,8 +102,15 @@ func ParseURL(mod mb.Module, rawURL string) (mb.HostData, error) { | |||
| return h, nil | |||
| } | |||
|
|
|||
| func QueryStats(db *sql.DB, query string) ([]map[string]interface{}, error) { | |||
| rows, err := db.Query(query) | |||
| func QueryStats(driver, uri, query string) ([]map[string]interface{}, error) { | |||
There was a problem hiding this comment.
exported function QueryStats should have comment or be unexported
| @@ -102,8 +102,15 @@ func ParseURL(mod mb.Module, rawURL string) (mb.HostData, error) { | |||
| return h, nil | |||
| } | |||
|
|
|||
| func QueryStats(db *sql.DB, query string) ([]map[string]interface{}, error) { | |||
| rows, err := db.Query(query) | |||
| func QueryStats(driver, uri, query string) ([]map[string]interface{}, error) { | |||
There was a problem hiding this comment.
exported function QueryStats should have comment or be unexported
This patch refactors the postgresql module to fix a socket leak caused by creating too many instances of sql.DB. Fixes elastic#11393
adriansr
added
bug
Metricbeat
|
|
||
| // DBCache keeps a cache of databases for different drivers and URIs. | ||
| var DBCache = dbCacheType{ | ||
| dbs: make(map[cacheKey]*sql.DB, 4), |
There was a problem hiding this comment.
Could you share some background on the size here?
|
|
||
| type dbCacheType struct { | ||
| dbs map[cacheKey]*sql.DB | ||
| lock sync.Mutex |
There was a problem hiding this comment.
Nit: What about making this just sync.Mutex to compose dbCacheType with it? Like this below it would be just cache.Lock().
|
|
||
| db, err = sql.Open(driver, uri) | ||
| if db != nil { | ||
| cache.dbs[key] = db |
There was a problem hiding this comment.
In case of err != nil we should not assign the cache value as otherwise during the next run, it will not return an error.
|
@adriansr thanks for investigating this. Did you try to upgrade the postgresql driver we use? It seems that at least a connection leak was fixed recently. I'd prefer if we can follow an approach as the one used in the MySQL module and avoid maintaining our own connections pool if possible. |
|
It's true that this was a bit of a shotgun approach to workaround the problem. I've submitted a fix to the lib/pq library (lib/pq#843) so will close this PR. |
adriansr
removed
the
needs_backport
This patch refactors the postgresql module to fix a socket leak caused by creating too many instances of sql.DB.
Fixes #11393