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

bug: DB Connections not being reused #225

Closed
mr-karan opened this issue Nov 17, 2020 · 1 comment · Fixed by #226
Closed

bug: DB Connections not being reused #225

mr-karan opened this issue Nov 17, 2020 · 1 comment · Fixed by #226

Comments

@mr-karan
Copy link
Collaborator

Bug

In the sample config files provided for binary and docker installations, there's no configuration set for db.max_open and db.max_idle parameters.

Since these values are not set, they are by default passed as 0 when unmarshalled and configured here.

While having an unbounded connection is not desirable, it's not really that problematic but setting db.SetMaxIdleConns(0) means that every connection is established from scratch, thus not utilizing the connection pooling at all.

Here's the pg_stat_activity query results:

SELECT
  COUNT(*),
  state
FROM pg_stat_activity
WHERE datname = 'listmonk'
GROUP BY 2;
  • When max_idle isn't specified (0):
 count | state  
-------+--------
     1 | active
  • When max_idle is specified (25):
 count | state  
-------+--------
     1 | active
     2 | idle

Proposed Fix

I think we can provide the following options as "sane defaults" in the sample configs.

max_open 25
max_idle 25

Unrelated

In addition to this, and although this is unrelated and not a bug, but we should also set db.SetConnMaxLifetime to something like 5-10 minutes.

@knadh
Copy link
Owner

knadh commented Nov 17, 2020

Makes sense. Can you send a PR? We just need this added to the sample config + db init. Default for older config after the next upgrade will be 0 automatically, so nothing changes.

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 a pull request may close this issue.

2 participants