fix(ui): stop serving a cold snapshot cache as an empty sites list - #1260
Merged
Conversation
A caller that lost the build lock was handed whatever the cache already held, which on a cold cache is nil. The handler wrote that out as a zero-byte body, so a browser opening the dashboard just after lerd-ui restarted lost the race on most of the requests it fires at once: twenty five concurrent reads of /api/sites returned an empty body to twenty four of them. On the client that empty body fails to parse and the catch keeps the value the store was initialised with, so the dashboard showed no sites at all and stayed that way until some unrelated mutation published a sites payload over the websocket, which on an idle machine can be minutes. A caller holding a usable stale value still returns immediately rather than queuing behind a slow podman inspect, which is why the early return existed. A caller with nothing to show now waits for the build in flight, because answering with the cold cache is answering with a claim about the registry that nobody checked. The four kinds each carried their own copy of that pattern, so they are one type now. From the other direction, buildSites turned a failure to read the registry into an empty slice and the cache stamped it fresh, which would serve one transient failure as the truth for up to five minutes. It reports the error instead, a failed build is never cached, and the handler answers 503 rather than an empty list. Where something is already cached, a failed rebuild keeps serving it and leaves the timestamp alone so the next read tries again. Worker health still reports an empty list when its probe fails, since no unhealthy workers is a safe reading of that, unlike no sites. Nothing re-fetches the sites list on a timer, so loadSites now retries while it has never managed to load. Once a list is on screen a later failure leaves it there instead of chasing the endpoint. Closes #1248.
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.
The snapshot cache handed a caller that lost the build lock whatever it already held, and on a cold cache that is nil. The handler wrote it out as a zero-byte body, so a browser opening the dashboard just after a restart lost the race on most of the requests it fires at once, and the client's parse failure left the store at the empty array it was initialised with, with nothing scheduled to try again.
A caller holding a usable stale value still returns immediately rather than queuing behind a slow podman inspect, which is what the early return was for. A caller with nothing waits for the build already in flight instead, because answering from a cold cache is making a claim about the registry that nobody checked. All four kinds carried their own copy of that pattern with the same flaw, so they are one type now with the rule stated once.
The second path in the report is fixed the same way. A failure to read the registry is reported rather than turned into an empty slice, a failed build is never cached, and the handler answers 503 instead of a body the dashboard would render as "you have no sites". Where something is already cached, a failed rebuild keeps serving it and leaves the timestamp untouched so the next read retries. Worker health still reports an empty list when its probe fails, since no unhealthy workers is a safe reading of a failed health check in a way that no sites is not.
On the client, loadSites retries at 500ms, 1.5s and 4s while it has never managed to load. Once a list is on screen a later blip leaves it alone.
Verified against the repro in the report, on a machine with twenty sites. Restarting lerd-ui and firing twenty five concurrent reads of /api/sites returned an empty body to twenty four of them beforehand, and the full payload to all twenty five afterwards, holding at forty concurrent across repeated restarts.
Closes #1248.