oxidecomputer / oxidecomputer/omicron

Fix write contention in silo crud

Open
#9,851 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Rust
Stars
572
Forks
97
Avg merge
2d 12h
Merged PRs (30d)
96

Description

We noticed that parallel silo updates occasionally break acceptance tests on the terraform provider, e.g. https://github.com/oxidecomputer/terraform-provider-oxide/actions/runs/21911763795/job/63267489398#step:16:265. Looking into the logs, we believe the issue is related to a few known issues in silo and dns management:

  • The dns_update_incremental function runs a series of queries in a transaction, rather than combining them into a CTE. This makes conflicts between concurrent queries more likely.
  • This may just be a restatement of the first point, but because we use nested transactions for dns updates, we can't retry the transactions if they fail due to a retryable error, using the transaction_non_retry_wrapper wrapper.

Given that we've only seen this come up during terraform tests, the quickest fix is to make sure that tests that create or delete silos don't run in parallel. But I'll note that we only run a small number of tests that create silos in the current suite, so real users may start to encounter this error in the wild before long. I think the correct fix would be either to implement application-side retries above the level of the transaction, or to refactor the DNS update to use a CTE such that it doesn't require the use of nested transactions. Low urgency for now—I'm mostly writing this up so I can link to the issue on the terraform side.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with nexus/db-queries/src/db/datastore/dns.rs, especially the dns_update_incremental area linked in the report, and reproduce the contention through the terraform provider acceptance tests that create or delete silos. Compare the nested-transaction behavior with the referenced CTE implementation and determine which proposed direction is appropriate. Done means the parallel silo-update failure is resolved and the relevant acceptance tests pass reliably.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
databases
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.