Expand PgBouncer and Consul HA support to handle multiple databases in the same cluster

As it stands today HA failover support, achieved through a combination of PgBouncer and Consul, in omnibus is supported only for the main GitLab database - gitlabhq_production.

With the addition of Gitaly Cluster and Praefect however it requires a separate database. For efficiency and maintenance reasons the desire will always be to have the databases on the same database cluster. However this isn't currently possible as failover is only handled by PgBouncer and Consul for the gitlabhq_production database and there doesn't appear to be a way to get it to track other databases on the same cluster.

Without this capability we can't use Praefect on the same database cluster, which is a notable inconvenience for Customers as well as ourselves. From previous interactions with customers I believe the general consensus will strongly be that they'll want to use the same cluster where possible.

Specifically adding this ability to make it so PgBouncer and Consul can track other databases will unlock this and any other future components to be fully managed in Omnibus.

added Reference Architecture database devopssystems groupdistribution + 1 deleted label

cc @ibaum @zj-gitlab

If possible the earlier this could be tackled the better but I appreciate the distribution team are super busy!

cc @toon as Gitaly Database expert.

I wasn't aware that was an issue. But I agree @grantyoung raises a good point here.

marked this issue as related to #5897

marked this issue as related to gitaly#2476

marked this issue as related to gitaly#3398 (closed)

marked this issue as related to gitlab-org/quality/reference-architectures#1

marked this issue as related to gitaly#3356 (closed)

mentioned in issue gitaly#3356 (closed)

mentioned in issue gitlab-org/quality/reference-architectures#1

added to epic &5145

added sectioncore platform label

mentioned in issue #5897

changed the description

mentioned in issue gitlab-org/quality/reference-architectures#28

To provide some context here: this is mostly an issue with PgBouncer notification script and to some extent the way that gitlab-ctl pgb-notify is implemented.

Consul watches PostgreSQL and when it goes down it triggers the notification script with the new leader's host name. At this point Consul hands it over to the handler script and it is up to this script to decide what to do with it. Currently it calls gitlab-ctl pgb-notify. This command accepts a database name but currently it is not used which falls back to the default value.

So the scaffolding for the fix is there but must be revisited with the new requirement in mind. That being said I know that one of the enterprise customers actually looking into something similar.

@pursultani We did have this working at one point with the pgbouncer['databases'] attribute. It wasn't an intentional feature at the time, and the workflow wasn't great so it was never documented. But I believe currently if you add entries to that without host information, pgb-notify will track those when the leader changes, alongside the gitlabhq_production entry. It has been a while so my memory is a little off on how it worked.

Subsequent changes may have also broken this, but it could be a good starting point.

Thanks @ibaum - this is becoming pretty important for a large opportunity. Would you be able to take a quick look and see if it's something that still works? We'll have a larger meeting on this early next week.

FYI @cdu1 @mendeni

I can spin up a cluster real quick and test it out. I'll try and spend some time on it today.

/cc @nhxnguyen

@joshlambert I tested the following, and it looks like it works

On a pgbouncer node

1. Update /etc/gitlab/gitlab.rb

   ...
   pgbouncer['databases'] = {
     gitlabhq_production: {},
     testdb: {}
   }
   ...

2. Run gitlab-ctl reconfigure
3. Run gitlab-ctl restart consul # Consul will regenerate the databases.ini follow on startup
4. Confirm /var/opt/gitlab/consul/databases.ini should now contain entries for both databases, pointing to the Leader of the patroni node
5. Shutdown the current patroni leader and wait for a new leader to be elected
6. Confirm /var/opt/gitlab/consul/databases.ini is updated for both databases and pgbouncer has picked up the changes

Awesome, thank you so much @ibaum. @ibaum @pursultani - should we do this automatically if Gitaly Cluster is enabled? Do we know if it is or not?

@francispotter - it looks like we have a way to solve this

@ibaum will this work for failovers as well? I raised this issue in part as I found there to be at least one case of a hardcoded value for gitlabhq_production.

@joshlambert I'll probably defer to @pursultani on that. I'm not fully up to speed on the current state of, or plans for, spinning up a Praefect cluster in omnibus-gitlab alongside a DB cluster. I do think if we're automating that, we should include this.

I think we should also do some work to make sure this continues to work. This wasn't really a planned feature, so at a minimum we may want to add some specs to ensure this setup generates the expected configuration. Also, the line that @grantyoung found should be fixed before we recommend using this in a production environment.

@grantyoung This definitely needs some more rigorous testing, but it should work for failovers.

Discussed this with the Distribution team today, the core issue here is that Omnibus does not currently manage the Praefect database by design: gitaly#2476. Praefect assumes a "Bring Your Own DB" model, which means you manage failures, etc.

@zj-gitlab @mjwood - should Omnibus manage this database? Is this something the Gitaly team would prioritize?

@joshlambert at the moment the amount of automated configuration that Omnibus does for Gitaly Cluster database is almost non-existent. This requires some work on not only Consul/PgBouncer but also Praefect recipes as well.

Discussed this with the Distribution team today, the core issue here is that Omnibus does not currently manage the Praefect database by design: gitaly#2476. Praefect assumes a "Bring Your Own DB" model, which means you manage failures, etc.

Right now that's the case as we'd have no way to have LISTEN/NOTIFY support and HA with omnibus. It feels we can finally resolve this with #6039 (closed).

Other than that, we'd love native Omnibus support Praefect databases!

added For Scheduling label

mentioned in issue gitlab-org/distribution/team-tasks#776 (closed)

mentioned in merge request gitlab!51928 (merged)