Incident Review: The sidekiq_queueing SLI of the sidekiq service on shard low-urgency-cpu-bound has an apdex violating SLO

Key Information

Metric Value
Customers Affected During the 46 minute period, any user triggering an urgent-shard background job would have seen slowness
Requests Affected Any sidekiq urgent job that saw increased latency during this period. I'm not sure how to find this metric.
Incident Severity severity2
Start Time 2024-10-22 21:59:00 UTC
End Time 2024-10-22 22:45:00 UTC
Total Duration 46 minutes
Link to Incident Issue #18754 (closed)

Summary

Flipping the feature flag load_balancer_low_statement_timeout saturated the urgent sidekiq pgbouncer pool, leading to reduced job throughput and elevated queuing times for the 45 minutes that it was on.

Details

@stomlinson rolled out the feature flag load_balancer_low_statement_timeout ([Feature flag] Rollout of `load_balancer_low_st... (gitlab-org/gitlab#473429)) to production slowly over the course of about 3 hours. When the flag was flipped from 50% to 100%, the unknown-at-the-time higher consumption of pgbouncer connections was enough to saturate pgbouncer and reduce the rate of sidekiq job processing.

Flipping this feature flag caused the load balancer to checkout a fresh connection from the connection pool to perform health checks with a low statement timeout. While it was known that this would increase connection pressure, these checks were estimated at one every 2-3 minutes per replica, per puma worker. @stomlinson missed that a health check happens several times during sidekiq job execution to verify that the chosen replica is up-to-date with the queueing time of the job. This roughly doubled client connection pool sizes in sidekiq, since an extra connection was checked out concurrently for every job.

The flag was rolled out slowly with monitoring using a new feature flag type that turns entire kubernetes pods on and off (Feature.current_pod) since it couldn't be associated with any other feature flag actor (load balancer health checks mutate shared state across requests, so it seemed unsafe to roll any load balancer code out with a per-request flag).

There was no visible pgbouncer saturation when rolling the flag out from 1%, 5%, 20%, or 50% of pods, but saturation occurred when the flag was turned up to 100% of pods.

It was not initially clear that the feature flag was related, since the link between load balancer health checks and sidekiq queueing wasn't obvious. The SRE asked to disable the feature flag just to be sure it was unrelated, and disabling it immediately fixed the contention.

Outcomes/Corrective Actions

  1. Before re-enabling the load_balancer_low_statement_timeout feature flag as part of Improve application database load balancing log... (gitlab-org/gitlab#472161 - closed), @stomlinson will make sure it's not using extra connections, will roll it out with a change request, and will monitor for longer paying special attention to pgbouncer saturation metrics.

Learning Opportunities

What went well?

  1. The feature flag was quickly disabled, limiting the duration of the incident.
  2. A feature flag was used for this feature. This is non-trivial, specific work went in to making feature flags compatible with load balancer code without feature flag checks recursing infinitely as they go through the load balancer.
  3. The separate pgbouncer pool for sidekiq urgent jobs limited the blast radius. Without seperate pgbouncer pools for sidekiq and web, this could have been an severity1 outage. We've had split pgbouncer pools for a long time, and it continues to be a good safety measure. The additional split for sidekiq urgent jobs / sidekiq long-running jobs is recent, and further limited the blast radius to just urgent jobs.

What was difficult?

  1. While rolling out the feature flag, it wasn't possible (or at least @stomlinson didn't know how to) to monitor for increased pressure on pgbouncer client connections, without saturation. I explicitly looked for pgbouncer saturation while progressively rolling out the flag, but the pgbouncer pools seemed healthy because their queueing stays at 0 if they're keeping up, even with a higher workload.

  2. Having sidekiq and rails-web kibana logs in different places makes them harder to search at once, when this incident started @stomlinson checked the rails-web logs and saw zero related messages, forgetting that the sidekiq ones are in a different kibana shard.

Review Guidelines

This review should be completed by the team which owns the service causing the alert. That team has the most context around what caused the problem and what information will be needed for an effective fix. The EOC or IMOC may create this issue, but unless they are also on the service owning team, they should assign someone from that team as the DRI.

For the person opening the Incident Review

  • Set the title to Incident Review: (Incident issue name)
  • Assign a Service::* label (most likely matching the one on the incident issue)
  • Set a Severity::* label which matches the incident
  • In the Key Information section, make sure to include a link to the incident issue
  • Find and Assign a DRI from the team which owns the service (check their slack channel or assign the team's manager) The DRI for the incident review is the issue assignee.
  • Announce the incident review in the incident channel on Slack.
:mega: @here An incident review issue was created for this incident with <USER> assigned as the DRI.
If you have any review feedback please add it to <ISSUE_LINK>.

For the assigned DRI

  • Fill in the remaining fields in the Key Information section, using the incident issue as a reference. Feel free to ask the EOC or other folks involved if anything is difficult to find.
  • If there are metrics showing Customers Affected or Requests Affected, link those metrics in those fields
  • Create a few short sentences in the Summary section summarizing what happened (TL;DR)
  • Use the description section to write a few paragraphs explaining what happened
  • Link any corrective actions and describe any other actions or outcomes from the incident
  • Consider the implications for self-managed and Dedicated instances. For example, do any bug fixes need to be backported?
  • Add any appropriate labels based on the incident issue and discussions
  • Once discussion wraps up in the comments, summarize any takeaways in the details section
  • Close the review before the due date
Edited by Simon Tomlinson