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

Added libpq option target_session_attrs for cluster support #101

Merged

Conversation

moonrail
Copy link
Contributor

@moonrail moonrail commented Jun 8, 2022

Since PostgreSQL 10 the C library libpq, that is used by psycopg, that itself is used in Django and therefore NetBox, supports specifying multiple PostgreSQL hosts and a kind of selector.

See these links:

Basically one is able to specify multiple hosts and what a session to a host should support.
libpq will then try each host and use first matching one.
In the case of an aborted connection or if a failover happens, the application will still get an error, but is able to retry and a new host will be searched for automatically.

The added option in this PR is compatible to single hosts as well.
And it makes sense to set as default, because NetBox only really supports ReadWrite-connections.

Without this session attr NetBox will allow a readonly database connection.
When NetBox is using a readonly session, it will throw exceptions for each write action (e.g. user login) and reconnect to the database.
So this would be a behaviour change in that a readonly session is not allowed anymore. Currently NetBox would somewhat work for users being already logged in and only issuing readonly requests. But this I would not call a feature of NetBox.

What this PR will & can not fix, is that NetBox itself does only reconnect to the database, when an exception is thrown.
As netbox-docker uses nginx-unit with multiple app processes, hypothetically all processes (e.g. 4 pods with 4 processes = 16 processes) will throw an exception individually before all db connections are refreshed.
I've omitted factors like max db connection age & load-scaling of application processes in this simple example, but you get the idea.

@moonrail moonrail changed the base branch from master to develop June 8, 2022 18:15
@bootc bootc merged commit 37bd70a into netbox-community:develop Jul 17, 2022
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 this pull request may close these issues.

2 participants