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.

Sign up for GitHub

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 documentation for timeout parameters #535

Merged
merged 3 commits into from
Mar 27, 2017
Merged

Fix documentation for timeout parameters #535

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6ec775d
Fix documentation for timeout parameters
pschultz Dec 27, 2016
4416483
Add readme section about connection pool
pschultz Mar 27, 2017
43145eb
Add myself to AUTHORS
pschultz Mar 27, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,11 +260,11 @@ Default: false
##### `readTimeout`

```
Type: decimal number
Type: duration
Default: 0
```

I/O read timeout. The value must be a decimal number with an unit suffix ( *"ms"*, *"s"*, *"m"*, *"h"* ), such as *"30s"*, *"0.5m"* or *"1m30s"*.
I/O read timeout. The value must be a decimal number with a unit suffix ( *"ms"*, *"s"*, *"m"*, *"h"* ), such as *"30s"*, *"0.5m"* or *"1m30s"*.
Copy link
Member

Choose a reason for hiding this comment

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

Is this correct?

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, because "unit" does not start with a vowel, phonetically: /ˈjuː.nɪt/.


##### `strict`

Expand All @@ -283,11 +283,11 @@ By default MySQL also treats notes as warnings. Use [`sql_notes=false`](http://d
##### `timeout`

```
Type: decimal number
Type: duration
Default: OS default
```

*Driver* side connection timeout. The value must be a decimal number with an unit suffix ( *"ms"*, *"s"*, *"m"*, *"h"* ), such as *"30s"*, *"0.5m"* or *"1m30s"*. To set a server side timeout, use the parameter [`wait_timeout`](http://dev.mysql.com/doc/refman/5.6/en/server-system-variables.html#sysvar_wait_timeout).
Timeout for establishing connections, aka dial timeout. The value must be a decimal number with a unit suffix ( *"ms"*, *"s"*, *"m"*, *"h"* ), such as *"30s"*, *"0.5m"* or *"1m30s"*. To configure the duration after which connections are removed from the connection pool, use [*sql.DB.SetConnMaxLifetime](https://golang.org/pkg/database/sql/#DB.SetConnMaxLifetime).
Copy link
Member

Choose a reason for hiding this comment

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

I think second sentence ("To configure...") is redundant.
Both of wait_timeout sysvar and SetConnMaxLifetime are totally unrelated to this config variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That SetConnMaxLifetime is unrelated to the driver config is exactly what makes this link useful. It says "the driver doesn't do what you may be looking for; see here instead."

What's the harm in this link? The Internet works so well because of hyperlinks.

If you insist I'll remove it, but in my opinion it is perfectly adequate to mention SetConnMaxLifetime in the documentation for a parameter that is simply called "timeout", which by itself can mean a dozen different things.

Copy link
Member

Choose a reason for hiding this comment

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

If there are 1000 people, there may be 1000 "may be looking for".
Adding 1000 links for each option makes README unreadable.

So my question is why your case is so special?

Copy link
Member

Choose a reason for hiding this comment

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

If this is really FAQ, I'm OK to adding link. But I have seen only one question. (#488)

I think SetConnMaxLifetime is very useful (that's why I added it!), I feel there should be more appreciate section like "recommended configurations".

Copy link
Member

Choose a reason for hiding this comment

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

Maybe, adding "Configuring connection pool and timeout" section under the "Usage" section is more useufl.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough. Done in 4416483.


##### `tls`

Expand All @@ -302,11 +302,11 @@ Default: false
##### `writeTimeout`

```
Type: decimal number
Type: duration
Default: 0
```

I/O write timeout. The value must be a decimal number with an unit suffix ( *"ms"*, *"s"*, *"m"*, *"h"* ), such as *"30s"*, *"0.5m"* or *"1m30s"*.
I/O write timeout. The value must be a decimal number with a unit suffix ( *"ms"*, *"s"*, *"m"*, *"h"* ), such as *"30s"*, *"0.5m"* or *"1m30s"*.


##### System Variables
Expand Down