Update database connections defaults #5853
Conversation
jananiarunachalam
requested review from
evan2645,
amartinezfayo,
sorindumitru,
MarcosDY and
rturner3
as code owners
Signed-off-by: jananiarunachalam <[email protected]>
jananiarunachalam
force-pushed
the
issue-5802-fix-db-conns
branch
from
b0a5681 to
9b46080
Compare
There was a problem hiding this comment.
Thank you @jananiarunachalam for working on this contribution!
| defaultConnMaxLifetime, err := time.ParseDuration("30s") | ||
| db.DB().SetConnMaxLifetime(defaultConnMaxLifetime) // default value | ||
| if err != nil { | ||
| return nil, "", false, nil, fmt.Errorf("failed to parse conn_max_lifetime %q: %w", defaultConnMaxLifetime, err) | ||
| } |
There was a problem hiding this comment.
The SetConnMaxLifetime function is already called later, based on the ConnMaxLifetime setting.
We don't need to make any change there. What we need to adjust is the ConnMaxIdleTime property calling SetConnMaxIdleTime, maybe with a value of 30s.
Also note that you will not need to make any parsing for that (i.e. time.ParseDuration("30s")), you can just call SetConnMaxIdleTime(time.Second * 30). We may define this as a constant in the function, and probably the MaxOpenConns and MaxIdleConns numbers too.
|
Also, could you update the documentation in https://github.com/spiffe/spire/blob/main/doc/plugin_server_datastore_sql.md with the new defaults? |
Pull Request check list
Affected functionality
Description of change
Which issue this PR fixes
Fixes #5802