Repository navigation
Tell a missing server from an unreadable one - #45
Merged
Merged
Conversation
With existing: true, start_child/3 and ensure_started_child/3 read the
child's stored state and treated any read error as a missing object, so a
throttled or failed store response, a timeout, or a dropped connection
told the caller the child didn't exist. Only the store saying the object
doesn't exist is :not_found now. A read error that may pass is retried
within the caller's deadline, and one that persists returns
{:error, {:storage_unavailable, reason}} without starting anything.
get_server_info/2 read eventually consistent and returned :not_found for
any read error. It now retries a read error that may pass, returns
{:error, {:storage_unavailable, reason}} when the state still can't be
read, and takes consistent: true for callers about to act on a miss.
A caller acts on what get_server_info says, and an eventually consistent read can miss state written moments ago. Read the consistent view unless the caller passes consistent: false, as stream_all_server_info/1 does for its listing, which is eventually consistent already.
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.
Summary
With
existing: true,start_child/3andensure_started_child/3read the child's stored state before starting it, and turned any read error into{:error, :not_found}:check_existing/4(thestart_childpath) did the same. The object-store backend tells these apart: a 404 is:not_found, every other failed response is:bad_gateway, and transport failures come back as their own reasons. But the supervisor collapsed them all. So a throttled or failed read told the caller the child doesn't exist.get_server_info/2did the same, and reads eventually consistent, so it can also miss state written moments ago. A caller that acts on that miss, for example by treating the server as gone, acts on a transient error.Sprites calls
ensure_started_child(..., existing: true)for every operation on a sprite whose owner isn't registered: after it idles down, after a deploy re-homes it, while a new owner's registration propagates. A503 SlowDownthere became a 404 "not found" for a sprite that exists. Clients treat that as "gone" rather than retrying.Change
read_stored_object/4,5, used by both paths.:not_found.:bad_gateway(which covers 408/429/5xx),:timeout,:closed,:econnrefused,:econnreset, andReq/Minttransport errors. There are 3 attempts in all, with a 100–200 ms then 200–300 ms backoff, and a retry happens only if it fits in the caller's remaining:timeout. These retries sit above Req's own default GET retries.existing: true, a read that still fails returns{:error, {:storage_unavailable, reason}}and starts nothing. An error that isn't retryable, such as an undecodable object, returns that at once.existing: true, an unreadable stored state is retried the same way and then starts as before: the child's init reads and claims its state itself, and the lock claim keeps a fresh start from replacing stored state.get_server_info/3(was/2):{:error, {:storage_unavailable, reason}}when the state can't be read, keeping{:error, :not_found}for a store miss;consistent: falseto opt back into an eventually consistent read.stream_all_server_info/1uses that for its per-entry reads, because the listing behind it is eventually consistent already.:existingis now documented on both functions, and there's a changelog entry.Testing
New
test/durable_server/existing_read_error_test.exsuses an in-memory backend that fails reads on cue. It runs these cases for bothstart_childandensure_started_childwithexisting: true::bad_gatewayread is retried, and the stored server starts with its stored count;{:error, {:storage_unavailable, reason}}after more than one read, and nothing is registered for the key;storage_unavailableafter exactly one read;:not_foundafter exactly one read.For
get_server_info, four more tests cover::bad_gateway;storage_unavailableafter reads that keep failing;consistent: falseon request, and eventually consistent reads fromstream_all_server_info/1.The error-path tests fail on main with
{:error, :not_found}, and the option test fails becauseget_server_info/3is undefined. The miss tests pass before and after.mix test: 476 passed, 37 excluded.mix format --check-formattedandmix compile --warnings-as-errorsare clean.Callers
{:error, {:storage_unavailable, reason}}is a new return value fromstart_child/3,ensure_started_child/3andget_server_info. Callers that match only{:ok, _}and{:error, :not_found}onget_server_infowill raiseCaseClauseErroruntil they handle it, so update them when bumping.