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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix example, put defer close before ping #476

Closed
arnodenuijl opened this issue May 22, 2019 · 3 comments
Closed

fix example, put defer close before ping #476

arnodenuijl opened this issue May 22, 2019 · 3 comments

Comments

@arnodenuijl
Copy link

Hi,

I have a question. This library is used in telegraf to read sql server metrics (https://github.com/influxdata/telegraf). This works great. However, if we misconfigure the sql connection settings (and it can't connect correctly) we see an everlasting increase in memory used. There is an active issue on the telgraf github (influxdata/telegraf#3485).

It seems that when the sql connection isn't working good there are a lot of go routines that keep running for the connectionOpener and connectionResetter.

goroutine 5896 [select]:
database/sql.(*DB).connectionOpener(0xc003c22480, 0x26dbf40, 0xc000139b00)
	/usr/local/go/src/database/sql/sql.go:1001 +0xef
created by database/sql.OpenDB
	/usr/local/go/src/database/sql/sql.go:671 +0x164

goroutine 5897 [select]:
database/sql.(*DB).connectionResetter(0xc003c22480, 0x26dbf40, 0xc000139b00)
	/usr/local/go/src/database/sql/sql.go:1014 +0x102
created by database/sql.OpenDB
	/usr/local/go/src/database/sql/sql.go:672 +0x19a

Telegraf uses the following code to open and use the sql connection

	conn, err := sql.Open("mssql", server)
	if err != nil {
		return err
	}
	// verify that a connection can be made before making a query
	err = conn.Ping()
	if err != nil {
		// Handle error
		return err
	}
	defer conn.Close()

	// execute query
	rows, err := conn.Query(query.Script)
	if err != nil {
		return err
	}
	defer rows.Close()

https://github.com/influxdata/telegraf/blob/master/plugins/inputs/sqlserver/sqlserver.go#L150

This look a lot like one of the samples : https://github.com/denisenkom/go-mssqldb/blob/master/examples/tsql/tsql.go#L25

I was wondering if you shouldn't call defer conn.Close() before you call Ping?
I tried it and it seems to resolve the problem. But I was wondering if it is placed after the Ping for a specific reason.

Hope you can help!

Kind regards,

Arno den Uijl

@kardianos
Copy link
Collaborator

We should fix this example. Thanks.

@kardianos kardianos changed the title Memory issue with misconfigured sql connection (telegraf) fix example, put defer close before ping May 22, 2019
@kardianos
Copy link
Collaborator

I also commented on the telegraph thread.

@arnodenuijl
Copy link
Author

Thanks! Learned a lot today!

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

No branches or pull requests

3 participants