Skip to content

feat(db) resource version cas - #1292

Merged
derekwaynecarr merged 1 commit into
NVIDIA:mainfrom
derekwaynecarr:feat/db-resource-version-cas
May 18, 2026
Merged

feat(db) resource version cas#1292
derekwaynecarr merged 1 commit into
NVIDIA:mainfrom
derekwaynecarr:feat/db-resource-version-cas

Conversation

@derekwaynecarr

@derekwaynecarrderekwaynecarr commented May 9, 2026•
edited
Loading

Copy link
Copy Markdown
Collaborator

Summary

Add Compare-And-Swap (CAS) infrastructure for safe concurrent object mutations
and migrate critical paths to use it. This prevents lost updates in HA
deployments with multiple gateway replicas.

Core infrastructure:

  • Add resource_version field to ObjectMeta proto (uint64)
  • Add resource_version column to objects table (SQLite: INTEGER, Postgres: BIGINT)
  • Add WriteCondition enum (MustCreate, MatchResourceVersion, Unconditional)
  • Add PersistenceError::Conflict variant for version mismatch
  • Add Store::put_if() and Store::delete_if() CAS methods
  • Add Store::update_message_cas() with bounded retry for mutations
  • Implement CAS operations for both SQLite and Postgres backends
  • Hydrate resource_version on all typed reads (defaults to 1 for backfill)

Migrations:

  • Migrate policy mutations to CAS (draft operations, settings)
  • Migrate provider updates to CAS (credentials, config merging)
  • Migrate sandbox updates to CAS (phase transitions, status reconciliation)
  • Migrate compute status updates to CAS (driver watch event handling)

Database migrations backfill existing rows with resource_version = 1.
CAS updates increment atomically: resource_version = resource_version + 1.

gRPC handlers map PersistenceError::Conflict to ABORTED status code
to signal clients to retry with fresh data. Server-side retries use
bounded retry (5 attempts) with fresh reads on each iteration.

Test coverage includes concurrent update scenarios and handler-level
resource_version round-trip tests.

Related Issue

Fixes#1255

Changes

Testing

  • [x ] mise run pre-commit passes
  • [ x] Unit tests added/updated
  • [ x] E2E tests added/updated (if applicable)

Checklist

  • [ x] Follows Conventional Commits
  • [ x] Commits are signed off (DCO)
  • [ x] Architecture docs updated (if applicable)

@derekwaynecarr
derekwaynecarr requested review from a team, maxamillion and mrunalp as code ownersMay 9, 2026 13:59
@copy-pr-bot

copy-pr-botBot commented May 9, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@derekwaynecarrderekwaynecarr changed the title Feat/db resource version casfeat(db) resource version casMay 9, 2026
@derekwaynecarr
derekwaynecarrforce-pushed the feat/db-resource-version-cas branch from 30e8540 to 46c93c3CompareMay 10, 2026 01:38
johntmyers
johntmyers reviewed May 12, 2026
View reviewed changes
Comment threadcrates/openshell-server/src/grpc/provider.rs Outdated
johntmyers
johntmyers reviewed May 12, 2026
View reviewed changes
Comment threadcrates/openshell-server/src/grpc/provider.rs Outdated
johntmyers
johntmyers reviewed May 12, 2026
View reviewed changes
Comment threadcrates/openshell-server/src/grpc/policy.rs Outdated
@derekwaynecarr
derekwaynecarrforce-pushed the feat/db-resource-version-cas branch from 46c93c3 to 4931e83CompareMay 14, 2026 05:20
@derekwaynecarr

derekwaynecarr commented May 14, 2026

Copy link
Copy Markdown
CollaboratorAuthor

@johntmyers ptal. updated per prior feedback, also updated all proto paths that required CAS enablement. the recently merged draft chunk work is not covered right now, i would like to leave that as a follow-on.

johntmyers
johntmyers reviewed May 14, 2026
View reviewed changes
Comment threadcrates/openshell-server/src/grpc/provider.rs
johntmyers
johntmyers reviewed May 14, 2026
View reviewed changes
Comment threadcrates/openshell-server/src/compute/mod.rs
johntmyers
johntmyers reviewed May 14, 2026
View reviewed changes
Comment threadcrates/openshell-server/src/compute/mod.rs Outdated
@derekwaynecarr
derekwaynecarrforce-pushed the feat/db-resource-version-cas branch 2 times, most recently from 4456536 to d65c359CompareMay 16, 2026 01:21
@derekwaynecarr

derekwaynecarr commented May 16, 2026

Copy link
Copy Markdown
CollaboratorAuthor

All prior comments are resolved.

  • All create/update paths for all objects that have an ObjectMeta with resource version have been updated.
  • All list and gets should properly hydrate resource version as well.
  • Updated arch docs.

@derekwaynecarr

derekwaynecarr commented May 16, 2026

Copy link
Copy Markdown
CollaboratorAuthor

/ok to test d65c359

@derekwaynecarr

derekwaynecarr commented May 16, 2026

Copy link
Copy Markdown
CollaboratorAuthor

failure due to flaky test, fix #1417

@derekwaynecarrderekwaynecarr mentioned this pull request May 18, 2026
Closed
@derekwaynecarr
derekwaynecarrforce-pushed the feat/db-resource-version-cas branch 2 times, most recently from 76a5ab8 to afdf64fCompareMay 18, 2026 17:17
@derekwaynecarr

derekwaynecarr commented May 18, 2026

Copy link
Copy Markdown
CollaboratorAuthor

/ok to test afdf64f

@derekwaynecarr
feat(persistence): implement optimistic concurrency control with CAS
0a09cac
Add resource_version-based optimistic concurrency control to the
persistence layer. Every write now requires an explicit WriteCondition
(MustCreate or MatchResourceVersion), enforced at compile time by
gating unconditional put/put_message behind #[cfg(test)].
- Add WriteCondition enum and put_if for conditional writes
- Add update_message_cas for atomic read-modify-write operations
- Add list_messages/list_messages_with_selector helpers that hydrate
resource_version from authoritative DB rows
- Convert all production write paths to CAS-aware methods
- Gate put/put_message behind #[cfg(test)] to prevent non-CAS writes
- Use structured PersistenceError::UniqueViolation matching instead of
string matching for duplicate detection
- Hydrate resource_version from WriteResult directly on creates,
eliminating unnecessary read-after-write round trips
Signed-off-by: Derek Carr <decarr@redhat.com>
@derekwaynecarr
derekwaynecarrforce-pushed the feat/db-resource-version-cas branch from afdf64f to 0a09cacCompareMay 18, 2026 17:54
@derekwaynecarr

derekwaynecarr commented May 18, 2026

Copy link
Copy Markdown
CollaboratorAuthor

/ok to test 0a09cac

@derekwaynecarr
derekwaynecarr merged commit 7f16d60 into NVIDIA:mainMay 18, 2026
26 checks passed
@pimlockpimlock mentioned this pull request Jul 17, 2026
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@johntmyersjohntmyersjohntmyers approved these changes
@mrunalpmrunalpAwaiting requested review from mrunalpmrunalp is a code owner
@maxamillionmaxamillionAwaiting requested review from maxamillionmaxamillion is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

feat(gateway): add DB-backed resource_version CAS for stored objects

2 participants

@derekwaynecarr@johntmyers