Repository navigation
fix(microvm-worker): wait at least 30 s after a suspend refused for now - #799
Draft
ParidelPooya wants to merge 7 commits into
Draft
ParidelPooya wants to merge 7 commits into
ParidelPooya wants to merge 7 commits into
Conversation
The docs said that a first-attempt throttle or a clock offset is the normal case for a suspend refused for now. A probe of the default client showed otherwise. 1. The default client uses botocore's standard retry mode with 3 attempts. 2. That mode retries ThrottlingException. A 429 on every attempt ends with RetryAttempts 2. 3. An error after a retry is an unknown outcome. So a throttle from the default client usually ends as unknown, with the 30-second refusal. 4. botocore retries no signature or token error, also when the server's Date header is 10 minutes ahead. These errors end with RetryAttempts 0, so they stay refused for now. The README and the docstrings now state this. The create_microvms_client doc states that a client that retries must report RetryAttempts on its error. Without it, a retried throttle counts as a first attempt. A new test pins the default client's retries for these four errors.
After a suspend refused for now, the worker started the idle time again at once. Some of these errors repeat on every call. A wrong secret key gives a signature error on every call. A static session token gives ExpiredTokenException on every call after it expires. So an idle MicroVM made one SuspendMicrovm call per idle time, without a bound. With a 0.0001-second idle time, a probe counted 15 calls and 30 log lines in 3 seconds. The restart after a refusal for now now waits max(idle time, 30 s). The 30 s is SUSPEND_GRACE_SECONDS. The probe now counts 1 call in 3 seconds for each idle time. 1. The refused branch asks for the idle time with after_refusal=True. 2. The call is still counted as running at that point. So the request waits in _idle_after_call, and _idle_after_call_floor records the floor. 3. Each later request during the same call overwrites the floor. So the latest request decides. 4. A resume hook during the call comes before the refused outcome. So the floor applies. A job that ends after the outcome writes False. So the normal idle time applies. 5. Every other start, such as a job end or a resume hook after the refusal, uses the normal idle time. The warning is still logged once per refused call. The JavaScript worker (#1040) is being changed to the same rule.
The warning for a suspend refused for now said that the worker tries again after the next idle time, "at least 30 seconds". That promise was false in two cases. 1. A suspend hook can arrive during the call. The hook's refusal of jobs then stays, and the worker sets no 30-second floor. The worker still logged the same warning. 2. A job that ends after the refusal, or a resume hook after it, starts the normal idle time. So the next call can come sooner than 30 s. The worker now checks the suspend hook count first, and then logs one of two warnings. - Without a suspend hook during the call, the warning says that the next idle time lasts at least 30 seconds, unless a job or a resume hook starts the idle time again. - With a suspend hook during the call, the warning says that the hook's refusal stays. That refusal ends when the resume hook arrives, or 30 seconds after the suspend hook. The idle time then starts again. The tests now assert both texts in full. The JavaScript worker (#1040) is being changed to the same wording.
Two paths had no test. A probe in review showed the right behavior for both. 1. A resume hook arrives during the call, and a job starts. The call ends refused for now while the job runs. The job ends after the call. The worker makes no call while the job runs. It makes one call one normal idle time after the job ends. 2. A resume hook arrives during the call. A job starts and ends during the call. The call then ends refused for now. The refused outcome is the later request to start the idle time. So the 30-second floor applies.
1. README item 7 said that the JavaScript worker decides these cases by HTTP status. The JavaScript worker checks the error name first, as this worker does. The two differ in the default client. The JavaScript client retries a clock-skew error. So there, a clock-skew error usually ends as an unknown outcome. 2. README item 7 said that the worker makes at most one call per 30 seconds while the error lasts. A job that ends restarts the normal idle time. So the limit holds only while no job ends and no lifecycle hook arrives. The sentence now states that condition. 3. README item 7 said that only a resume hook during the call keeps the floor. A job that ends during the call keeps it too, because the refused outcome arrives after the job's end. The sentence now names both. 4. The create_microvms_client docstring said that each error reports its retries in ResponseMetadata.RetryAttempts. Only a ClientError carries ResponseMetadata. The docstring now says ClientError.
Item 7 of the listener docstring named only a resume hook during the refused call. The README also names a job that ends during the call. Both keep the 30-second wait, because the refused outcome arrives after them. So the docstring now names both.
…fused-for-now-floor
This branch has not been deployed
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.
Fixes #797.
Summary
After a suspend that the service refused for now, the MicroVM worker now waits at least 30 seconds before its next SuspendMicrovm call. The docs now say which errors reach "refused for now" with the default client. The JavaScript worker made the same change in aws/aws-durable-execution-sdk-js#1040.
Why
ExpiredTokenExceptioneach time after it expires.autoSuspendIdleSeconds. So an idle MicroVM made one call per idle time, with no lower bound, and wrote two log lines per call.ThrottlingException. A probe showed that a 429 on every attempt ends withRetryAttempts2. So the error is classified unknown.Changes
The 30-second minimum
_start_idle_time(after_refusal=True)uses max(autoSuspendIdleSeconds, 30 s) for the next idle time._suspend_calls. So the request waits in_idle_after_call, and_idle_after_call_floorrecords the minimum._suspend_call_endedapplies it.resumehook or a job that ends during the call does not shorten the wait, because the refused outcome arrives after it.resumehook after the refusal starts the normal idle time.Warnings
The refused-for-now warning now has two texts. The worker chooses the text under the lock, from the same decision that sets the minimum.
suspendhook during the call: the worker tries again after the next idle time. That idle time lasts at least 30 seconds, unless a job or aresumehook starts the idle time again.suspendhook during the call: the hook's refusal of jobs stays. It ends when theresumehook arrives, or 30 seconds after thesuspendhook. The idle time then starts again.Docs
SuspendOutcome,classify_suspend_error,create_microvms_clientand the listener say that the default client retries a throttle. So a throttle usually ends as unknown. "Refused for now" covers expired credentials and a signature error, with or without a clock offset. A throttle counts only from a client that does not retry it.create_microvms_clientoption doc says that a caller client that retries must report its retries inResponseMetadata.RetryAttemptson eachClientError. Otherwise a retried throttle counts as a first attempt.Testing
worker.pyandself_suspend.pyare at 100 % line and branch coverage. Typecheck,hatch fmt --checkand the commit lint pass.resumehook that follows the refusal;resumehook, or a job that starts and ends, during the call;