feat(helm): add support for pgbouncer (#818) #818
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #818 +/- ##
=======================================
Coverage 19.22% 19.22%
=======================================
Files 26 26
Lines 2512 2512
=======================================
Hits 483 483
Misses 2029 2029
|
| @@ -98,6 +98,12 @@ This Helm automatically prefixes all names using the release name to avoid colli | |||
| | `reana_hostname` | REANA hostname (e.g. reana.example.org) | None | | |||
| | `namespace_runtime` | Namespace in which the REANA runtime pods (workflow engines, jobs etc...) will run | `.Release.Namespace` | | |||
| | `naming_scheme` | REANA component naming scheme | None | | |||
| | `pgbouncer.enabled` | Instantiate PgBouncer inside the cluster to pool database connections | false | | |||
There was a problem hiding this comment.
When pgbouncer is enabled, shall we act on reana-job-controller's REANA_DB_CLOSE_POOL_CONNECTIONS configuration value?
There was a problem hiding this comment.
No, it's actually the opposite, if pgbouncer is not enabled then the two options are:
- increase the number of connection slots of the database
- close the connections as soon as they are returned to SQLAlchemy's connection pool by setting
REANA_DB_CLOSE_POOL_CONNECTIONSappropriately
I have kept the default value of REANA_DB_CLOSE_POOL_CONNECTIONS as to have the best performance possible, but it is customisable so that we can revert back the change in case of troubles with PgBouncer.
What do you think we should do regarding this? Shall we add this to the documentation?
There was a problem hiding this comment.
Yes, we can later improve the scalability-related documentation regarding PgBouncer.
| @@ -0,0 +1,17 @@ | |||
| apiVersion: v1 | |||
There was a problem hiding this comment.
Hmm, I wonder whether it would be possible to amend the setup to automatically delete / refresh reana-server and reana-workflow-controller pods when the admin changes PgBouncer from enabled to disabled and vice versa? (Currently in addition to helm diff upgrade... one has to do it manually.)
This comment is a sort of a "bonus", the things work nicely 👍 and I'm not sure whether admins would attempt to play with enabling/disabling PgBouncer dynamically in reality... Well we could always simply explain this in the documentation.
There was a problem hiding this comment.
I have updated the PR to also roll the deployments of pgbouncer/r-server/r-w-controller when the database config (configmap or secrets) change. This can be used as a blueprint to do the same for the other config/secret that might change over time.
Automatically restart deployed pods when the database configuration changes by setting the checksums of the ConfigMaps and Secrets as annotations of the pod.
Automatically restart deployed pods when the database configuration changes by setting the checksums of the ConfigMaps and Secrets as annotations of the pod.
Automatically restart deployed pods when the database configuration changes by setting the checksums of the ConfigMaps and Secrets as annotations of the pod.
Closes #819