Skip to content

Conversation

JelteF
Copy link
Contributor

@JelteF JelteF commented Apr 8, 2024

Connection Lifetime can also be used to limit memory growth of
PostgreSQL connection. Certain caches only grow over time, the most
common example is the cache for table metadata: the "relcache". For
systems with many tables (often times due to partitioning) this relcache
slowly grows larger and larger. By putting a 1 hour limit on a
connection lifetime such excessive growth is limited, since this is
especially an issue with very long lived connections (multi-day).

This same 1 hour limit is used for PgBouncer its equivalent
server_lifetime config option. (I'm the maintainer of PgBouncer)

@JelteF JelteF requested review from roji and vonzshik as code owners April 8, 2024 07:33
Copy link

@paulmey paulmey left a comment

Choose a reason for hiding this comment

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

This is a better default, IMHO.

@vonzshik
Copy link
Contributor

vonzshik commented Apr 8, 2024

Certain caches only grow over time, the most
common example is the cache for table metadata: the "relcache". For
systems with many tables (often times due to partitioning) this relcache
slowly grows larger and larger.

Was this issue raised with postgres maintainers? Unbound cache is the main problem here, since even if we have a one hour limit on a connection lifetime, it can most likely be replicated with sufficient enough load.

@JelteF
Copy link
Contributor Author

JelteF commented Apr 8, 2024

It's a very long standing known issue that the maintainers are aware of. But changing to some expiration/LRU based approach hasn't happened yet. Some recent message mentioning this: https://www.postgresql.org/message-id/20230613.165512.2091685398843624399.horikyota.ntt%40gmail.com

At the moment it's simply not recommended to have connections open to postgres for an unbounded amount of time.

To be clear, I'm not saying that one hour is the perfect limit for everyone. I've seen users have a need to lower it to 10 minutes to keep memory in check. But the current default (no limit at all) is definitely wrong for a significant percentage of users. And the overhead of reconnecting once per hour is very tiny.

@vonzshik
Copy link
Contributor

vonzshik commented Apr 8, 2024

OK, Got it. I'm personally OK with changing the default of connection lifetime to one hour.
@roji what do you say?

Copy link
Member

@roji roji left a comment

Choose a reason for hiding this comment

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

Yeah, this conversation seems to make this point even more forcefully.

So I'm good with this. @NinoFloris any objection?

/// balancing between a running server and a server just brought online. It can also be useful to prevent
/// runaway memory growth of connections at the PostgreSQL server side, because in some cases very long lived
/// connections slowly consume more and more memory over time.
/// Defaults to 3600 seconds (1 hour).
Copy link
Member

Choose a reason for hiding this comment

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

Need to also fix the <value> doc comment just below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Connection Lifetime can also be used to limit memory growth of
PostgreSQL connection. Certain caches only grow over time, the most
common example is the cache for table metadata: the "relcache". For
systems with many tables (often times due to partitioning) this relcache
slowly grows larger and larger. By putting a 1 hour limit on a
connection lifetime such excessive growth is limited.

This same 1 hour limit is used for PgBouncer its equivalent
server_lifetime config option. (I'm the maintainer of PgBouncer)
@JelteF JelteF force-pushed the connection-lifetime-non-zero branch from c8341a0 to fbc4f3f Compare April 9, 2024 08:28
@NinoFloris
Copy link
Member

Yeah 1 hour sounds fine to me. Bedankt @JelteF :)

@roji roji added this to the 9.0.0 milestone Apr 13, 2024
@roji roji merged commit 5b380fb into npgsql:main Apr 13, 2024
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.

5 participants