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

Set a lock_wait_timeout on the MySQL connection #252

Merged
merged 1 commit into from
Jan 5, 2018

Conversation

SuperQ
Copy link
Member

@SuperQ SuperQ commented Dec 19, 2017

Set a session level lock_wait_timeout to avoid stuck metadata locks.

Closes: #167

Set a session level `lock_wait_timeout` to avoid stuck metadata locks.
@SuperQ
Copy link
Member Author

SuperQ commented Dec 19, 2017

/cc @rtreffer

Copy link
Contributor

@AlekSi AlekSi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@SuperQ SuperQ merged commit c52f281 into master Jan 5, 2018
@SuperQ SuperQ deleted the superq/lock_timeout branch January 5, 2018 14:51
@arvenil
Copy link
Contributor

arvenil commented Jan 5, 2018

I'm surprised if this really works because you can't guarantee that db.Query() will be executed on the same session. Considering that this exporter limits number of open sessions, there is a chance it actually works... but this is still not guaranteed.

db is a session pool, and even with db.SetMaxOpenConns(1) you don't have guarantee that next db.Query() will use the same session as previous db.Query() call. It could always got close in background and new session could get open.

The only way to ensure you are using the same session is to use transactions.

go-sql-driver/mysql#208

@SuperQ
Copy link
Member Author

SuperQ commented Jan 5, 2018

db objects are created per scrape, it's not global for the exporter. But, as was pointed out in another PR, there are better ways to deal with this via the DSN string. I will probably refactor this later for better control.

@arvenil
Copy link
Contributor

arvenil commented Jan 10, 2018

@SuperQ just to clarify my position. Yes, you are right db is per scrape, but during scrape we are running multiple queries, and each query is picking a connection from connection pool. Yes, the connection pool is limited to just one connection, but this only means it's one connection at a time, not the same connection always.
For example here
https://golang.org/src/database/sql/sql.go?s=34389:34668#L1267
you can see we try to get connection few times, and if you dig deeper you will see it tries to grab cached one or if it doesn't work then it will create new one.

I just noticed new func, looks like recently added, and doc for this func explicitly says "Queries run on the same Conn will be run in the same database session. "
https://golang.org/pkg/database/sql/#DB.Conn
golang/go@d234f9a
Never used that, so can't say how well it works, but could be an option for you.
(well, I don't agree with this 1-connection pool so maybe I shouldn't suggest it ;))

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

Successfully merging this pull request may close these issues.

3 participants