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

Postgres connection leak #235

Open
grebneerg opened this issue Aug 12, 2022 · 2 comments
Open

Postgres connection leak #235

grebneerg opened this issue Aug 12, 2022 · 2 comments
Labels
bug Something isn't working

Comments

@grebneerg
Copy link
Member

The postgres dataaccess implementation seems to still have a connection leak. I reached too many connections by executing the following commands:

./gort bootstrap --allow-insecure -F 127.0.0.1:4000
!whoami
./gort group add admin theprogrammerjack
./gort config set -b incidents -k YEXT_PAGERDUTY_API_TOKEN value
./gort config set -b incidents -k YEXT_PAGERDUTY_API_TOKEN value
./gort config get -b incidents
./gort group (shouldn't hit server, just helptext)
./gort group list
./gort group list (first failure)
!config set -b incidents -k YEXT_PAGERDUTY_API_TOKEN value (also failure)

Those preceded by ! were executed through slack, while the rest were executed in the terminal locally. It's worth noting that all of the commands that hit the server were unauthorized due to #232.

@grebneerg grebneerg added the bug Something isn't working label Aug 12, 2022
@vfxcode
Copy link

vfxcode commented Sep 30, 2022

We had the same "issue" I think with the latest master branch.

The default parameters for the postgres database do not expire the database connections. I tested the following (which may or may not be too aggressive for your use case)

  # The maximum amount of time a connection may be idle. Expired connections
  # may be closed lazily before reuse. If <= 0, connections are not closed due
  # to a connection's idle time. Defaults to 1m.
  connection_max_idle_time: 1m

  # The maximum amount of time a connection may be reused. Expired connections
  # may be closed lazily before reuse. If <= 0, connections are not closed due
  # to a connection's age. Defaults to 10m
  connection_max_life_time: 30s

  # Sets the maximum number of connections in the idle connection pool. If
  # max_open_connections is > 0 but < max_idle_connections, then this value
  # will be reduced to match max_open_connections.
  # If n <= 0, no idle connections are retained.
  # Defaults to 2
  max_idle_connections: 2

  # The maximum number of open connections to the database. If
  # max_idle_connections is > 0 and the new this is less than
  # max_idle_connections, then max_idle_connections will be reduced to match
  # this value. If n <= 0, then there is no limit on the number of open
  # connections. The default is 0 (unlimited).
  max_open_connections: 150

Overall I think that the database/sql is not used the way it was meant to. It opens/closes database connections too eagerly while the library recommends limited open/close connections that span the duration of the process.

@clockworksoul
Copy link
Member

You're almost certainly right. Thank you for pointing this out: database utilization is one of my weak points and this will be very helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants