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

feat(db): exposed postgres connection pooling config #9603

Merged
merged 32 commits into from
Dec 16, 2022

Conversation

shettyh
Copy link
Contributor

@shettyh shettyh commented Oct 22, 2022

Summary

As of now there is no straight forward way to control the number of postgres connections created. There are some workarounds like setting lua_socket_pool_size, but that has it own downsides as it effects the global upstream connection pool also.

Because of this kong can bring down the postgres instance in case of high load and most of request are resulting in a cache miss. As part of this change exposed config to control this behaviour.

This change does not impact the default behaviour of kong

Full changelog

  • Added config pg_keepalive_timeout to control the postgres connection idle timeout
  • Added config pg_pool_size to control the postgres connection pool size
  • Added config pg_backlog to limit the open connections to postgres
  • Added relevant test cases

@hbagdi hbagdi requested review from bungle and dndx October 24, 2022 23:27
Copy link
Contributor

@mayocream mayocream left a comment

Choose a reason for hiding this comment

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

Related pgmoon PR:

This PR is indeed needed for improving the Postgres connections.

kong.conf.default Outdated Show resolved Hide resolved
kong.conf.default Outdated Show resolved Hide resolved
kong/db/strategies/postgres/connector.lua Outdated Show resolved Hide resolved
@shettyh shettyh requested review from mayocream and removed request for bungle and dndx October 26, 2022 09:38
kong.conf.default Outdated Show resolved Hide resolved
kong/db/strategies/postgres/connector.lua Outdated Show resolved Hide resolved
kong/db/strategies/postgres/connector.lua Outdated Show resolved Hide resolved
@hbagdi hbagdi requested a review from dndx October 26, 2022 17:04
@hbagdi
Copy link
Member

hbagdi commented Oct 26, 2022

@dndx Should we expose this level of detail? Can dynamic scaling of workers cause bad surprises?

@dndx
Copy link
Member

dndx commented Oct 31, 2022

@hbagdi As long as we provide reasonable defaults this should not be a bad thing to have. Most user don't need to touch it anyway.

@samugi
Copy link
Contributor

samugi commented Nov 22, 2022

@shettyh great job! Could you add a changelog entry in the Unreleased section please?

@chronolaw
Copy link
Contributor

Let's merge it after 3.1 release.

@shettyh
Copy link
Contributor Author

shettyh commented Nov 27, 2022

@samugi updated changelog, please check

@chronolaw chronolaw added the pending author feedback Waiting for the issue author to get back to a maintainer with findings, more details, etc... label Dec 6, 2022
@dndx dndx removed the pending author feedback Waiting for the issue author to get back to a maintainer with findings, more details, etc... label Dec 6, 2022
@mayocream
Copy link
Contributor

mayocream commented Dec 13, 2022

Hi @shettyh, the failing CI was fixed by #9949, please rebase.

@shettyh
Copy link
Contributor Author

shettyh commented Dec 14, 2022

@mayocream rebased, one of the test is still failing with config error

@kikito kikito added this to the 3.2 milestone Dec 14, 2022
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@hanshuebner hanshuebner merged commit f132418 into Kong:master Dec 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants