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: configure maximum db connections #32

Merged
merged 1 commit into from
Jul 11, 2024

Conversation

skatsaounis
Copy link
Collaborator

@skatsaounis skatsaounis commented Jul 5, 2024

The PR includes the following changes:

  • Set PgBouncer pool_mode to session since pg_advisory_lock cannot be supported by transaction mode
  • Set a predefined number for the maximum connections to the database server from each PgBouncer unit to be 50. Those are the maximum connections that MAAS up to 3.4 requires per region controller (40), plus a buffer of 10. PgBouncer units are subordinate units of maas-region units, so it has to be a 1-1 mapping. The extra 10, if not used they will be closed by PgBouncer.
  • Provide a mechanism to configure experimental_max_connections to PostgreSQL charm with three options:
    • default: It does not set experimental_max_connections at all. It will use the system defaults of PostgreSQL
    • dynamic: It configures experimental_max_connections based on the number of maas-region units. If chosen, it will use the maximum of 100 (default max connections) and 50 * number_of_region_units + 10 (required by PgBouncer). This option also includes a restart of the PostgreSQL server units, so it is up to the practitioner to use it.
    • exact number of connections (100<=n<=500): It statically configures the experimental_max_connections of the PostgreSQL server. If specified during bootstrap (e.g., the MAAS cluster topology is predefined and total number of regions is known), it will be set from the beginning.
  • A ReapplyPostgreSQLTerraformPlanStep that will re-configure the experimental_max_connections when new region nodes are joining/leaving the cluster and max_connections mechanism is set to dynamic
  • A small refactor on the functions that fetch all the Steps of a given role

Fixes: #11

SK1Y101
SK1Y101 previously approved these changes Jul 5, 2024
Copy link
Member

@SK1Y101 SK1Y101 left a comment

Choose a reason for hiding this comment

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

LGTM, with comment

@skatsaounis skatsaounis force-pushed the MAASENG-3382 branch 2 times, most recently from 4345fd9 to dbc3fe3 Compare July 5, 2024 12:07
@skatsaounis skatsaounis mentioned this pull request Jul 5, 2024
@skatsaounis skatsaounis force-pushed the MAASENG-3382 branch 7 times, most recently from 1472c86 to 136ad9d Compare July 8, 2024 15:18
SK1Y101
SK1Y101 previously approved these changes Jul 9, 2024
Copy link
Member

@SK1Y101 SK1Y101 left a comment

Choose a reason for hiding this comment

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

LGTM, nice!

@skatsaounis skatsaounis force-pushed the MAASENG-3382 branch 2 times, most recently from ffafa27 to b03f125 Compare July 10, 2024 09:02
Copy link
Member

@SK1Y101 SK1Y101 left a comment

Choose a reason for hiding this comment

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

Nice! Appreciate the README change too

@skatsaounis skatsaounis merged commit 423ea89 into canonical:main Jul 11, 2024
4 checks passed
@skatsaounis skatsaounis deleted the MAASENG-3382 branch July 11, 2024 10:01
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.

Postgresql connections
2 participants