Skip to content

fix(postgres): serialize concurrent metadata type upserts - #250

Closed
jeffspahr wants to merge 1 commit into
google:masterfrom
jeffspahr:codex/fix-postgres-type-creation-race
Closed

jeffspahr wants to merge 1 commit into
google:masterfrom
jeffspahr:codex/fix-postgres-type-creation-race

Conversation

@jeffspahr

Copy link
Copy Markdown

Concurrent first-time type upserts against PostgreSQL can both observe an absent (name, version, type_kind) and insert different IDs. A client can then create a context under one ID while GetContextByTypeAndName resolves the other and returns an empty result. Two unmodified KFP clients reproduced this as context not found immediately after insert against MLMD 1.14.0; the same read-then-insert path remains on master.

Acquire a transaction-scoped SHARE ROW EXCLUSIVE lock on Type before the upsert lookup. Under PostgreSQL's default READ COMMITTED isolation, a competing upsert waits and then reads the committed type instead of creating a duplicate. Ordinary type reads remain unblocked. The lock covers artifact, execution, and context types, including versioned and unversioned types, and requires no schema change.

Add a standalone PostgreSQL query-executor test target and deterministic two-connection regression coverage for contention, commit/readback, ordinary reads, and rollback. The tests use PostgreSQL's lock timeout to exercise overlap without scheduling-dependent thread sleeps. The standalone fixture now cleans up its database between cases.

This prevents new duplicate types among upgraded writers; it does not repair existing duplicate records. Existing duplicate IDs need a separate data-repair plan, and all metadata-server writers must be upgraded before relying on this guarantee during rollout.

Validation (PostgreSQL 16, Bazel 7.7.0):

  • //ml_metadata/metadata_store:postgresql_query_executor and //ml_metadata/metadata_store:standalone_postgresql_query_executor_test build successfully.
  • Full standalone PostgreSQL query-executor suite: 26 passed, including 12 new regression cases.
  • Same test suite with the production lock removed: 20 passed, 6 failed; every new contention case fails for the expected missing lock.
  • Changed-file pre-commit hooks and git diff --check pass.

Observed in the PostgreSQL MLflow lane of kubeflow/pipelines#14348. A KFP deployment will need a metadata-server image containing this fix before its first-use race is resolved.

Signed-off-by: Jeff Spahr <spahrj@gmail.com>
@google-cla

google-cla Bot commented Sep 9, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant