Skip to content

Release v0.0.316

Choose a tag to compare

@github-actions github-actions released this 31 Aug 08:48
· 107 commits to main since this release
f34d7e5

Summary

Follow-up to #379 (merged into main, gated behind SingleWriterEnabled, default false). A review of #379 (posted after it had already merged) found two functional-breaking bugs in guaranteedUpdateSingleWriter that only surface with the flag on — this fixes both.

  • SaveContainerProfile's ignoreNotFound=true upsert against a missing key never converges. guaranteedUpdateSingleWriter always built the commit job with create:false, baseRV:0 for this case. commit()'s compare-and-commit rejected that unconditionally with errWriteConflict — a nonexistent key can never make !exists false — so the retry loop can never succeed. This isn't an edge case: it's a container's first-ever consolidation write (ConsolidateTimeSeries → loadOrInitializeProfile synthesizes a fresh ContainerProfile{} on not-found, then calls SaveContainerProfile), so with the flag on every new container's first consolidation retries for 5s, fails with a contention-timeout error, and rolls back the whole consolidation transaction — every 30s tick, forever. The legacy GuaranteedUpdateWithConn/saveObject path has always treated a missing key + ignoreNotFound as a plain upsert (StorageImpl.get returns a zero object with a nil error, not a not-found error, in that case); commit() now honors the same contract by treating baseRV==0 (i.e. nothing existed when the caller read state) as "no compare-and-commit required," matching the old path's semantics exactly.

  • Finalizer-delete updates are silently dropped. The "before" snapshot used for the no-op-update short-circuit (orig := origState.obj.DeepCopyObject()) was taken after tryUpdate ran. genericregistry.Store's finalizer-delete tryUpdate mutates its input in place (via markAsDeleting) and returns that same reference, so by the time the snapshot was taken it had already observed the mutated state — making reflect.DeepEqual(orig, ret) always true and silently no-op'ing the delete. This is exactly the bug #372 fixed on the legacy GuaranteedUpdateWithConn path; the single-writer path reintroduced it. Fixed the same way #372 did: snapshot orig before tryUpdate runs, not after.

Both bugs are regression-tested directly against guaranteedUpdateSingleWriter/GuaranteedUpdate with singleWriterEnabled=true; both new tests fail against the pre-fix code and pass after.

Since the flag defaults to false, this is not urgent for anything currently running with it off, but it should land before SingleWriterEnabled is turned on anywhere (including the existing dev/stage soak, which apparently didn't exercise a brand-new container's first consolidation or a finalizer-delete, which is why neither surfaced there).

Test plan

  • go build ./... clean
  • go test -race -skip TestFileSystemStorageWatchReturnsDistinctWatchers ./pkg/registry/file/... — 3x clean (the skipped test is the same pre-existing, unrelated flaky test #379 excluded; verified it fails identically on unmodified main)
  • go test -race ./pkg/metrics/... ./pkg/config/... clean
  • New tests (TestSingleWriter_GuaranteedUpdate_IgnoreNotFound_MissingKey_Upserts, TestSingleWriter_GuaranteedUpdate_MutateInPlacePersists) verified to fail against pre-fix singlewriter.go and pass after

🤖 Generated with Claude Code