Skip to content

Commit

Permalink
Fix memory leak in SQL helper when database is not available (#26607)
Browse files Browse the repository at this point in the history
Explicitly close SQL connections when connectivity checks fail during their
creation, to avoid leaking resources when a connection is created but it
is not going to be used.

(cherry picked from commit e4d4f76)
  • Loading branch information
jsoriano authored and mergify-bot committed Jul 1, 2021
1 parent f23335c commit 9b5ab5d
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.next.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- Fix azure billing dashboard. {pull}25554[25554]
- Major refactor of system/cpu and system/core metrics. {pull}25771[25771]
- Fix GCP Project ID being ingested as `cloud.account.id` in `gcp.billing` module {issue}26357[26357] {pull}26412[26412]
- Fix memory leak in SQL module when database is not available. {issue}25840[25840] {pull}26607[26607]
- Fix aws metric tags with resourcegroupstaggingapi paginator. {issue}26385[26385] {pull}26443[26443]

*Packetbeat*
Expand Down
3 changes: 3 additions & 0 deletions metricbeat/helper/sql/sql.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ func NewDBClient(driver, uri string, l *logp.Logger) (*DbClient, error) {
}
err = dbx.Ping()
if err != nil {
if closeErr := dbx.Close(); closeErr != nil {
return nil, errors.Wrapf(err, "failed to close with %s, after connection test failed", closeErr)
}
return nil, errors.Wrap(err, "testing connection")
}

Expand Down
59 changes: 59 additions & 0 deletions metricbeat/helper/sql/sql_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,18 @@
package sql

import (
"context"
"database/sql"
"database/sql/driver"
"fmt"
"math"
"testing"
"time"

"github.com/stretchr/testify/require"

"github.com/elastic/beats/v7/libbeat/common"
"github.com/elastic/beats/v7/libbeat/tests/resources"
)

type kv struct {
Expand Down Expand Up @@ -186,3 +193,55 @@ func TestToDotKeys(t *testing.T) {
t.Fail()
}
}

func TestNewDBClient(t *testing.T) {
t.Run("create and close", func(t *testing.T) {
goroutines := resources.NewGoroutinesChecker()
defer goroutines.Check(t)

client, err := NewDBClient("dummy", "localhost", nil)
require.NoError(t, err)

err = client.Close()
require.NoError(t, err)
})

t.Run("unavailable", func(t *testing.T) {
goroutines := resources.NewGoroutinesChecker()
defer goroutines.Check(t)

_, err := NewDBClient("dummy", "unavailable", nil)
require.Error(t, err)
})
}

func init() {
sql.Register("dummy", dummyDriver{})
}

type dummyDriver struct{}

func (dummyDriver) Open(name string) (driver.Conn, error) {
if name == "error" {
return nil, fmt.Errorf("error")
}

return &dummyConnection{name: name}, nil
}

type dummyConnection struct {
name string
}

// Ensure that this dummy connection implements the pinger interface, used by the helper.
var _ driver.Pinger = &dummyConnection{}

func (*dummyConnection) Prepare(query string) (driver.Stmt, error) { return nil, nil }
func (*dummyConnection) Close() error { return nil }
func (*dummyConnection) Begin() (driver.Tx, error) { return nil, nil }
func (c *dummyConnection) Ping(context.Context) error {
if c.name == "unavailable" {
return fmt.Errorf("database unavailable")
}
return nil
}

0 comments on commit 9b5ab5d

Please sign in to comment.