Skip to content

Fix atomic local state writes - #6350

Merged
jhrozek merged 1 commit into
mainfrom
flaky-state
Aug 19, 2026
Merged

Fix atomic local state writes#6350
jhrozek merged 1 commit into
mainfrom
flaky-state

Conversation

@jhrozek

@jhrozek jhrozek commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Local state writes (GetWriter/CreateExclusive on LocalStore) previously opened the target
file directly and wrote to it in place. A failed or aborted write (e.g. process crash or error
mid-write) could leave a truncated or partially-written state file behind, and CreateExclusive's
"exists" check and file creation were not atomic with each other.

  • LocalStore writers now write to a temp file in the same directory and publish it atomically
    on Close — via os.Rename for GetWriter, and os.Link (which fails if the target already
    exists) for CreateExclusive, closing the exclusivity race.
  • Added an Aborter interface and state.AbortWriter helper so callers can explicitly discard
    an in-progress write instead of relying on Close, which now publishes data and can itself fail.
  • Updated all callers (pkg/groups, pkg/migration, pkg/state/runconfig.go) to call
    AbortWriter in their deferred cleanup instead of Close, and to check the error from the
    final Close, since publishing can now fail (e.g. CreateExclusive conflict).

Type of change

  • Bug fix

Test plan

  • Unit tests (task test)

API Compatibility

  • This PR does not break the v1beta1 API, OR the api-break-allowed label is applied and the migration guidance is described above.

Special notes for reviewers

state.Store writers are now required to implement Aborter; both LocalStore and the
Kubernetes no-op store were updated. Any future Store implementation needs to do the same.

Generated with Claude Code

@github-actions github-actions Bot added the size/M Medium PR: 300-599 lines changed label Aug 17, 2026
@codecov

codecov Bot commented Aug 17, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 58.13953% with 36 lines in your changes missing coverage. Please review.
✅ Project coverage is 72.98%. Comparing base (406fd8d) to head (ad9a394).

Files with missing lines Patch % Lines
pkg/state/local.go 62.22% 10 Missing and 7 partials ⚠️
pkg/migration/telemetry_config.go 0.00% 8 Missing ⚠️
pkg/state/runconfig.go 25.00% 4 Missing and 2 partials ⚠️
pkg/groups/cli_manager.go 72.22% 3 Missing and 2 partials ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6350   +/-   ##
=======================================
  Coverage   72.98%   72.98%           
=======================================
  Files         744      745    +1     
  Lines       78611    78670   +59     
=======================================
+ Hits        57374    57419   +45     
- Misses      17236    17244    +8     
- Partials     4001     4007    +6     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Publish LocalStore state files only after complete writes so concurrent readers cannot decode partial JSON.

Fixes #6328
@github-actions github-actions Bot added size/M Medium PR: 300-599 lines changed and removed size/M Medium PR: 300-599 lines changed labels Aug 19, 2026
@jhrozek
jhrozek merged commit 471e5f5 into main Aug 19, 2026
75 of 78 checks passed
@jhrozek
jhrozek deleted the flaky-state branch August 19, 2026 08:42
@github-actions github-actions Bot mentioned this pull request Aug 26, 2026
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Medium PR: 300-599 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants