Repository navigation
fix(state): durable config writes wait for another writer - #1826
Merged
Merged
Conversation
SaveDriverSecret, SaveConfiguration and ReserveVehicleWake read before they write inside a deferred transaction. SQLite does not wait out busy_timeout for a transaction that holds a read lock and then asks to write; it returns SQLITE_BUSY at once while another connection writes. On the home box that refused a rotated MyUplink refresh token. Take the write lock with the first statement, so the 5 s busy timeout applies. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
frahlg
added a commit
to srcfl/device-drivers
that referenced
this pull request
Oct 25, 2026
Once Core waits out a busy database (srcfl/ftw#1826), one failed store can take 5 s. Stored before the read, that pushed a poll with one timed-out read to 67.9 s between readings, past the 60 s watchdog. The token is now stored after the read on every return path, and a rotation during sign-in is only marked unsaved; the same case gives 57.9 s. The harness now also checks that, after rotations while the host refuses the store, the newest token is the one stored. Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
frahlg
added a commit
to srcfl/device-drivers
that referenced
this pull request
Oct 25, 2026
….4) (#193) * fix(myuplink): retry a failed read soon and keep a rotated token (1.2.4) One request that hits the host's 15 s HTTP timeout left 75 s without a reading (30 s wait, 15 s timeout, 30 s wait), past Core's 60 s watchdog, so the heat pump was marked offline on every slow request. Retry a failed read or sign-in once after 2 s; a second failure in a row waits the full 30 s, so a real outage still shows as offline. A good read asks for 30 s, the wait Core already applied. A rotated refresh_token that the host could not store (SQLITE_BUSY) was held only in memory until the next rotation. Retry the store on every poll until it succeeds, but not in driver_cleanup, where a reconnect may just have stored a newer token. Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(myuplink): store a rotated token after the reading, not before Once Core waits out a busy database (srcfl/ftw#1826), one failed store can take 5 s. Stored before the read, that pushed a poll with one timed-out read to 67.9 s between readings, past the 60 s watchdog. The token is now stored after the read on every return path, and a rotation during sign-in is only marked unsaved; the same case gives 57.9 s. The harness now also checks that, after rotations while the host refuses the store, the newest token is the one stored. Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Signed-off-by: Fredrik Ahlgren <fredrik@sourceful-labs.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 25, 2026
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
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.
Problem
On Fredrik's home box (v0.155.1-beta.1), on 25 October at 06:45:
The same night
mpc: persist diagnostic failedandforecast archive: retrying issuehit the same error.The state database sets
busy_timeout(5000), so a writer should wait up to 5 s for another one.SaveDriverSecretdid not wait. It runs insidedurableConfigWriteContext, which opens a deferred transaction, and it reads the settings row before it writes. SQLite does not wait out the busy timeout for a transaction that already holds a read lock and then asks to write, because waiting there could deadlock. It returns SQLITE_BUSY at once.SaveConfiguration(every settings save) andReserveVehicleWakeread first too.A rotated OAuth token that is not stored is held only in the driver's memory. After a reload or restart the driver starts from the replaced token, and if the vendor no longer accepts it, the driver cannot sign in until the owner connects again.
Evidence
A test that holds the write lock on another connection for 300 ms:
SaveDriverSecret(reads, then writes)SaveConfig(writes only)With a 2 s hold,
SaveDriverSecretstored after 2.06 s.Change
durableConfigWriteContexttakes the write lock with its first statement,UPDATE config SET value = value WHERE 0, before the caller's function reads anything. That statement changes no row and fires no trigger; the existing trigger test still passes. The existing 5 s busy timeout and the 5 s context then apply to every durable config write.Tests
TestDurableConfigWriteWaitsForAnotherWriter(new):SaveDriverSecretandSaveConfigurationwhile another connection holds the write lock. Before the fix it fails with the box's exact error.go test ./internal/state: pass.make verify: every package passes exceptinternal/energyreportandinternal/statsrelay(the relay report tests). Both fail the same way onmasterwithout this change; test(stats): pin the relay client's clock to the fake gateway's #1825 pins those tests' clock.Pairing
This PR goes with srcfl/device-drivers#193 (myuplink 1.2.4). That PR makes the driver retry a failed store on every poll, so it no longer depends on this one, and this one helps every driver that rotates a token. Merge either first.
🤖 Generated with Claude Code