Skip to content

ClinVar control refresh can freeze the worker on a row lock #882

Description

@bencap

Summary

Two ClinVar refresh jobs running at the same time can freeze the whole worker process, and only a restart brings it back. This issue reorders the job so it never holds database locks while waiting on the network, with a lock timeout as a backstop.

refresh_clinvar_controls holds row locks from its upserts across an await for every allele. When two refresh jobs write the same ClinVar control row in the same release, the second blocks inside psycopg2 on the event-loop thread. The first job can then never resume to commit, and the worker process freezes. ARQ's job timeout, the stalled-job cleanup cron and the progress heartbeat all stop running. Postgres sees one session idle in transaction and one waiting on a lock, which isn't a deadlock, so nothing breaks the wait. The only recovery is restarting the worker.

Problem

In worker/jobs/external_services/clinvar.py, each ClinVar release iteration:

  1. commits through update_progress,
  2. awaits the TSV download, with no locks held yet,
  3. for each allele, awaits get_associated_clinvar_allele_id (an aiocache Redis lookup that falls back to requests.get in a thread), then upserts into clinvar_controls and inserts a ClinvarAlleleLink,
  4. flushes, and commits only at the start of the next iteration.

The upsert is ON CONFLICT DO UPDATE on uq_clinvar_controls_db_name_identifier_version, and it locks the conflicting row even when nothing changes. The lock window therefore runs from the first upsert of a release until the next release's commit, and it spans every per-allele await. ClinvarAlleleLink's partial unique index uq_clinvar_allele_links_live contends the same way, because alleles are shared across score sets.

Contention needs two refresh jobs in the same ClinVar release at once. With MAX_JOBS = 2, and with run_score_set_pipelines clustering same-gene score sets through cluster_cohort, overlapping controls are the common case. Jobs started together begin on the same release, and a faster job catches up with a slower one later.

The CAID → ClinVar allele ID lookup doesn't depend on the release, but it runs once per allele for every one of the 12 releases.

Proposed behavior

  • Resolve ClinVar allele IDs for all alleles once, before the release loop.
  • Within each release: commit, await the TSV, then do every write with no await in between, flush and commit.
  • Sort the upserts by ClinVar ID so that concurrent jobs take row locks in the same order.
  • As a backstop, issue SET LOCAL lock_timeout = '5s' at the start of each release's transaction, and again after every commit. A lock timeout then surfaces as a retryable failure through the existing classification in worker/lib/managers/utils.py.

Tests

In tests/worker/jobs/external_services/test_clinvar.py:

  • Ordering: SQLAlchemy event listeners record INSERT/UPDATE statements and commits, and a mocked lookup records each await. Assert that no await falls between a write and the next commit.
  • Contention, on the Postgres fixture: a second connection holds a lock on a control row the job will upsert. The job must fail with a lock timeout rather than hang. Set lock_timeout on the test's own connection, so a regression fails instead of hanging the suite.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    app: workerTask implementation touches the worker

    Type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions