Skip to content

Refactor database selection in SidekiqServerMiddleware

Matthias Käppler requested to merge 332673-sidekiq-refactor-db-strategy into master

What does this MR do?

Closes #332673 (closed)

To allow for workers to balance reads over database replicas, we introduced logic in SidekiqServerMiddleware that makes decisions around whether to target the primary or replicas based on the job's data_consistency requirement. We also need to inject the database_chosen into the job hash so that we can label metrics with it in Sidekiq ServerMetrics.

These decisions were made in a single side-effecting getter, which made it difficult to reason about what this method is actually responsible for. I split up these two concerns so that we first select the appropriate database strategy, and only then act on it.

Other changes I made:

  • I addressed a related problem introduced in !63209 (merged) that resulted in the data_consistency setting being injected into the job hash twice: first in the client middleware, then again in the server middleware (using a different name: data_consistency instead of worker_data_consistency). I removed the redundant assignment and fixed ServerMetrics to read worker_data_consistency now. The original issue of this label missing from the metric should remain fixed.
  • I renamed database_chosen to load_balancing_strategy since this is not always a database node value. The new values are primary, replica, retry_primary and retry_replica. This label is not used in dashboards yet and just informational, so it's safe to rename. We also decided to expand on these in a follow-up in #333670 (closed).

This MR is just clean-up; nothing should change behaviorally. However, since we renamed a documented metric, we decided to highlight this via a changed changelog entry.

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Edited by Matthias Käppler

Merge request reports