Skip to content

Conversation

lowang-bh
Copy link
Contributor

@lowang-bh lowang-bh commented May 24, 2021

Description

I am confuse with the important setting. The code shows if MaxIdleConns is large than MaxOpen, will be reduce to MaxOpen

Checklist

  • Code compiles correctly
  • Created tests which fail without the change (if possible)
  • All tests passing
  • Extended the README / documentation, if necessary
  • Added myself / the copyright holder to the AUTHORS file

@lowang-bh
Copy link
Contributor Author

The document says db.SetMaxIdleConns() is recommended to be set same to (or greater than) db.SetMaxOpenConns().
But from the code, I find that if MaxIdleConns is large than MaxOpen, will be reduce to MaxOpen.
I am confused.

@shogo82148
Copy link
Contributor

related with #1141

README.md Outdated
`db.SetMaxOpenConns()` is highly recommended to limit the number of connection used by the application. There is no recommended limit number because it depends on application and MySQL server.

`db.SetMaxIdleConns()` is recommended to be set same to (or greater than) `db.SetMaxOpenConns()`. When it is smaller than `SetMaxOpenConns()`, connections can be opened and closed very frequently than you expect. Idle connections can be closed by the `db.SetConnMaxLifetime()`. If you want to close idle connections more rapidly, you can use `db.SetConnMaxIdleTime()` since Go 1.15.
It's confuse here! The code shows if MaxIdleConns is large than MaxOpen, will be reduce to MaxOpen
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this line need?

It seems that removing "(or greater than)" is enough to resolve this issue.
#1141 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes,I will fix as that. This line is just used to open a MR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! @shogo82148

Copy link
Contributor

@shogo82148 shogo82148 left a comment

Choose a reason for hiding this comment

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

LGTM

CC: @methane
I'll merge it because you said that:

#1141 (comment)
I wrote "same to (or greater than)" because greater value become same value automatically.
If it is confusing, I'm OK to remove "(or greater than)" part.

@shogo82148 shogo82148 merged commit 9942e21 into go-sql-driver:master May 26, 2021
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.

2 participants