feat(failure): distinguish quota-exceeded from the rest of FailureTerminal - #117
Merged
Conversation
…minal Every terminal error (bad AMI, IAM denial, malformed request, and an exhausted account quota) previously looked identical to a caller: an immediate give-up with no further signal. A caller running several concurrent pkg/snipe.Snipe calls against the same account has no way to tell "this quota ceiling might free up soon (it may be MY OWN other in-flight requests)" apart from "nothing about this will ever work." New pkg/failure.IsQuotaExceeded(err) checks a quota-specific error-code set (split out of terminalErrorCodes into its own quotaErrorCodes map, checked first in ClassifyFailure so a code is listed exactly once). pkg/watcher.IsQuotaExceeded aliases it. pkg/snipe.Status gains QuotaExceeded bool, set on every Progress report that carries a LastErr. Does NOT change any default retry behavior: a quota error still classifies as FailureTerminal and Snipe still returns immediately on it, unchanged. Purely additive -- existing TestClassifyFailure/ TestSnipe_TerminalStopsImmediately etc. pass unmodified, confirming the classification itself didn't move. Fixes #116.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
pkg/failure.IsQuotaExceeded(err) bool— a finer-grained question thanClassifyFailure'sFailureTerminal, which also covers bad AMI/IAM/malformed-request errors this correctly reportsfalsefor.quotaErrorCodes(InstanceLimitExceeded,VcpuLimitExceeded,MaxSpotInstanceCountExceeded) split out ofterminalErrorCodesinto its own map, checked first inClassifyFailure— so each code is listed exactly once, not duplicated across two maps.pkg/watcher.IsQuotaExceededaliases it, matching the existing alias pattern for every otherpkg/failureexport.pkg/snipe.StatusgainsQuotaExceeded bool, set on everyOptions.Progressreport that carries aLastErr— so a caller doesn't need to separately importpkg/failureand re-classify the error itself.No default behavior changes. A quota error still classifies as
FailureTerminalandSnipestill returns immediately on it, exactly as before —TestClassifyFailure,TestSnipe_TerminalStopsImmediately, etc. all pass unmodified, confirming the classification itself didn't move. This is purely an additive signal for a caller building its own backoff/reduce-concurrency logic on top.Real-world motivation: several concurrent
pkg/snipe.Snipecalls against the same account (one per fleet shard) each getMaxSpotInstanceCountExceededand give up immediately when the account's own quota is momentarily saturated by the caller's OTHER in-flight requests — indistinguishable, before this change, from a hard account wall needing a support ticket.Fixes #116.
Test plan
TestIsQuotaExceeded(11 cases) — all 3 quota codes report true; every other terminal code, every capacity code, an unknown code, and a plain non-AWS error all report falseTestIsQuotaExceeded_StillClassifiesTerminal— explicit regression guard thatClassifyFailureis unchanged for the 3 quota codesTestSnipe_ProgressReportsQuotaExceeded/TestSnipe_ProgressDoesNotReportQuotaForOrdinaryTerminal—Status.QuotaExceededcorrectly true/false via the real retry loopwatcher.TestIsQuotaExceeded— alias reaches the real classifierTestClassifyFailure(bothpkg/failureandpkg/watchercopies) pass unmodified — proves quota codes still classify asFailureTerminalgo build ./...,go vet ./...,gofmt -lclean (root module +lambda/capacity-poller); fullgo test ./...greengolangci-lint run— 2 pre-existing issues in untouched files (pkg/watcher/notifier.go,pkg/watcher/store.go), zero in touched filesmake check-docs— no drift