Support high availability for database load balancing

When a Postgres database replica degrades we see a major degradation across all GitLab services because application load balancing was designed for load distribution, not high availability. See https://docs.gitlab.com/ee/administration/database_load_balancing.html

We have previously discussed moving load balancing away from the application to a load balancing proxy but there were some points made that shows that our own load balancing code is necessary to ensure that reads go to the primary immediately after a write and other special logic.

We have seen in several high severity incidents like production#4874 (closed) and production#4820 (closed) where losing a replica will cause a large spike of errors for typically < 5 minutes. This spike, while short, can be quite disruptive.

• For the case where a replica goes offline, should we decrease the discovery interval from the default of 60seconds, so we can detect it sooner?
• Should we push more health logic into console? For example the application checks for replication delay but this could be done by the service status instead, we could also incorporate GCP maintenance events to mark the replica as unhealthy https://cloud.google.com/compute/docs/storing-retrieving-metadata#maintenanceevents and remove it from the pool, which has a 60 second lead time.

added 1 deleted label

mentioned in issue production#4874 (closed)

added corrective action label

marked this issue as related to production#4874 (closed)

marked this issue as related to production#4820 (closed)

changed the description

added teamReliability workflow-infraTriage labels and removed 1 deleted label

added priority2 label

removed teamReliability label

added workflow-infraReady label and removed workflow-infraTriage label

added severity2 workflow-infraTriage + 1 deleted label and removed workflow-infraReady label

@Finotto would love to get your input on this. As it stands, this issue has outstanding questions that are better answered a DBRE. Happy to partner with you on the implementation if needed.

@amoter Thank you for sharing this proposal with me. I think we need to consider some facts on our research during the last months about the degradation from the database load on the secondaries:

• the root cause that we understood is a long-running transaction on the primary and we keep an old txid(transaction id) open ( I mean keeping a reference with the txid from the primary in the secondary) generating an overflow on pg_lsru that start to degrade the performance on the secondaries. Please to read more about this we have this nice blog post from Stan: https://about.gitlab.com/blog/2021/09/29/why-we-spent-the-last-month-eliminating-postgresql-subtransactions/

• We have some epics, I have this one to share quickly: https://gitlab.com/groups/gitlab-com/gl-infra/-/epics/593#note_706977252

• we patched one instance with a custom parameter to increase the LSRU for subtransactions in one node patroni-v12-09, and it seems to be working nicely.

• We have already done some corrective actions to fix this problem but still seems that some corner cases are still happening.

Please these are some activities that are related to the proposal, please feel free to schedule a coffee chat and we can talk about it.

cc: @kwanyangu

@yorickpeterse I know you implemented recently a new load balancer for ci, this approach will impact the current code?

cc: @fzimmer @iroussos

@Finotto The changes we made were purely to support multiple database. The strategy for selecting what replicas to read from remains unchanged.

I think a starting point is to determine what metrics are/were useful for identifying the past incidents. Then we have to decide based on what conditions we should stop using a replica. One metric could be the database load, with the load balancer shifting queries away from an overloaded replica.

The requirement here is that obtaining these metrics should be cheap and easy, meaning they are preferably stored in the replica's database itself.

Another change we could make is to increase the frequency by which a replica status is checked. Right now the interval defaults to a random time between 60 and 120 seconds, but that might be too high for GitLab.com.

Basically the current "algorithm" is optimised largely for code simplicity, not for obtaining 15 nines of uptime. We have plenty of options there to improve this, provided the necessary metrics are easily obtainable.

Another option is to introduce a form of shared state between the processes that use load balancing. Right now every process decides for itself what replicas to pick. This can result in all processes picking the same replica, resulting in an uneven distribution of load. This is mitigated somewhat by randomising the order of replicas as stored in-memory, but it's not a perfect solution.

The shared state would be in Redis, and would track data such as how many users (in terms of processes) a replica has. When picking a replica at the start of the request, the process would pick the replica with the least amount of users. In addition, we could use Redis to track the online state of a replica. This way if one process detects a replica as being offline, all other processes will immediately disregard that replica.

Implementation wise I think this isn't actually all that difficult to implement. Basically we just have a counter per DB host to track the number of "users", and a separate "is the host online" flag; at least as a starting point. Redis in turn is fast enough that we can permit checking/modifying this state at the start of a request.

This also opens the door for more advanced load balancing strategies. For example, we could have an external monitoring system modify this Redis data based on various metrics. This way we don't need all sorts of (potentially) expensive checks in the Rails code.

I think in the original MR that introduced the load balancer I suggested something like this being a good option for the future. I guess that future is now

assigned to @amoter

added workflow-infraUnder Review label and removed workflow-infraTriage label

@Finotto @yorickpeterse to give a bit more context on my question, I'm the DRI for the CA squad (epics 562/621) and we are reviewing outstanding corrective actions. This issue strikes me as beyond the scope of the CA squad to take on because it seems to be epic-level work and at the very least should be done in partnership/at the direction of the DBRE team.

Do you feel that the core of reliability concerns that this issue captures are being addressed in other issues or epics? If not, I propose the creation of a new epic/issue and closing this one. Thoughts?

@amoter I do agree that this work will be epic and is not a Corrective Action. At the moment I suggest we can keep this issue in the backlog and raise it in further quarters goals/projects. I do agree also to keep the database team involved in the planning perhaps we change the priority due to the sharding goals.

Thanks @Finotto. I'm going to remove the corrective action label in that case, and leave the issue open.

added 1 deleted label and removed 1 deleted label

removed corrective action label

unassigned @amoter

removed workflow-infraUnder Review label

I am adding the labels to get visibility on the DB Squad backlog.

added Database label

mentioned in issue gitlab-com/www-gitlab-com#12796 (closed)

Removing the deprecated label ~"ca::refined".

removed 1 deleted label

removed priority2 label

removed severity2 label

marked this issue as related to production#8156 (closed)

mentioned in issue production#8156 (closed)

I'm awaking this issue from the backlog as a corrective action for production#8156 (closed), as we saw that if one of standby nodes are having issues, all standby nodes in Postgres cluster have a dip in traffic (particular thread where it was discussed: production#8156 (comment 1208214511))

Support HA for DB loadbalancing seems the best approach to speed up the overall reaction under such incidents.

• Replica health checks between "60 and 120 seconds" seem way to slow, it should be no more than 5 seconds in my opinion.
• Replicas health check should detect "stuck I/O", (lightweight to be executed frequently)

/cc @kwanyangu

added ReliabilityP2 boardautomate corrective action teamDatabase Reliability workincident workflow-infraReady labels

changed milestone to %DB Squad - February 23

mentioned in issue production#8423 (moved)

Hi @jarv,

The label severity4 has been applied due to missing severity:: label.

To ensure the necessary information is captured in this issue for a ::"corrective action", please apply the corrective_action issue template, which includes a quick action to apply the default severity label.

---

You are welcome to help improve this comment.

added severity4 label

Hi @jarv,

The label ServiceUnknown has been applied due to missing Service:: label.

Please update accordingly.

---

You are welcome to help improve this comment.

added ServiceUnknown label

changed milestone to %DB Squad - March 23

Setting a due date of 2023-05-22 based on corrective action SLOs

You are welcome to help improve this comment.

changed due date to May 22, 2023

assigned to @rhenchen.gitlab

changed due date to June 30, 2023

Setting a due date of 2023-05-22 based on corrective action SLOs

You are welcome to help improve this comment.

changed due date to May 22, 2023

closed

added workflow-infraDone label and removed workflow-infraReady label