Skip to content

fix(config): make the service-config PUT read-merge-write atomic #1488

Closed

Description

@cristim

Found by the second review of #1487. The bug predates that PR.

Problem: the service-config PUT reads the stored row (GetServiceConfig), merges the request into it (mergeServiceConfig in internal/api/handler_config.go) and writes back the full row with an upsert. None of this runs in a transaction or takes a lock. When two admins save different fields of the same service row at the same moment, one change is lost. A concurrent test lost one update in 199 of 200 runs. #1487 narrows the window, since a save now sends only the changed keys, but doesn't close it. The global config PUT is already atomic (UpdateGlobalConfigAtomic).

Fix: a store method that does the read, merge and write in one transaction, taking a SELECT ... FOR UPDATE lock on the row. A column-level UPDATE of only the present keys would also work. Mirror UpdateGlobalConfigAtomic, update the mock and the handler tests, and add a real-Postgres test that runs two concurrent PUTs of different keys and checks both persist.

Activity

  1. IAmJSD commented on Oct 26, 2026

    @IAmJSD
    Collaborator

    Claiming: working on this

  2. self-assigned this
    on Oct 26, 2026
  3. added a commit that references this issue on Oct 26, 2026
    0463051
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions