feat(storage): enable single writer by default and harden pool timeout - #393
Merged
Conversation
- Flip singleWriterEnabled to true by default - Rebind conn.SetInterrupt(ctx.Done()) across Create, GuaranteedUpdate, and singleWriter commit/prepare phases so poolTimeout does not prematurely interrupt SQLite statements - Bump DefaultPoolTimeout from 5s to 15s to provide headroom during high contention - Avoid cross-connection deadlock in SaveContainerProfile when called inside an active transaction - Populate metaOut with full candidate object in single writer Create/GuaranteedUpdate (preserving Spec) - Properly isolate test pool in TestVulnSummaryStorageImpl_Get subtests Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…dlock Calling conn.SetInterrupt on connections managed by sqlitex.Pool causes races and deadlocks in zombiezen sqlite's cancelInterrupt (blocking on nil chan) when pool.Put concurrently cleans up or when contexts have nil Done channels. sqlitex.Pool already manages SetInterrupt internally during Take and Put. Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
|
Summary:
|
This was referenced Sep 7, 2026
matthyx
added a commit
that referenced
this pull request
Sep 7, 2026
The single-writer write path (default-on since #393) funneled every Create/GuaranteedUpdate/SaveContainerProfile commit for ALL keys through one goroutine per StorageImpl, collapsing write concurrency from the legacy per-key-lock path's ~SqlitePoolSize-way parallelism to strictly 1-way. kubescape/node-agent's CI went from 0-1 flaky failures to 18-19 of ~30 jobs failing, every one on context deadline exceeded against the storage apiserver. Correctness only ever required one in-flight committer per key: commit() re-acquires the per-key lock and does an authoritative resourceVersion compare-and-commit, and writes to different keys touch disjoint metadata rows and payload files. Route commit jobs to N=8 shard goroutines by FNV-1a over the key instead, so same-key commits stay serialized in submission order while different-key commits run in parallel. highBurstLimit's high-vs-low arbitration becomes per-shard. Jobs on different shards never competed for a goroutine's time, so there is nothing there to arbitrate; only same-shard cross-priority contention is affected. Shard count is a fixed part of what SingleWriterEnabled turns on (DefaultSingleWriterShards, 8), not independently configurable -- validated empirically against kubescape/node-agent's real CI, which showed raising it further (16, with pool size 24) doesn't reduce the residual failure rate over the default, so exposing it as a tunable wasn't worth the added config surface. Empirical validation: with this fix, node-agent's CI went from 18-19/30 failing (every one a sustained 20-minute hang against the storage apiserver) to 15/30 (fast, self-healing failures on unrelated assertions; one representative test run went from 323 failed create attempts to 6 out of 61). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LLN3CgGftcnAikV13qD6yJ Signed-off-by: Matthias Bertschy <matthias.bertschy@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
SingleWriterEnabledinpkg/config/config.goandsingleWriterEnabledinpkg/registry/file/singlewriter.gototrue.sqlitex.Pool.Take(poolCtx)ties the connection's SQLite interrupt handler topoolCtx.Done(). In both legacy write paths (Create,GuaranteedUpdate) and single-writer phases (createSingleWriter,guaranteedUpdateSingleWriter, andsingleWriter.commit), we now rebindconn.SetInterrupt(ctx.Done())(orjob.ctx.Done()) and clear it upon release. This prevents the pool checkout timeout from interrupting valid in-flight writes.DefaultPoolTimeoutfrom 5s to 15s to provide breathing room during burst connection acquisition under heavy write loads.SaveContainerProfile: WhenSaveContainerProfileis called inside an active transaction (e.g. fromContainerProfileProcessor), it preserves the active transaction connection instead of attempting to acquire a second connection through the single writer.metaOut: IncreateSingleWriterandguaranteedUpdateSingleWriter,metaOutis now populated with the full persisted candidate object rather than the stripped metadata-only object.TestVulnSummaryStorageImpl_Getsubtests.