core: retry REST OAuth2 token refresh after transient failure - #17768
Open
waterWang wants to merge 1 commit into
Open
core: retry REST OAuth2 token refresh after transient failure#17768waterWang wants to merge 1 commit into
waterWang wants to merge 1 commit into
Conversation
One failed background token refresh permanently stopped the refresh chain: AuthSession.refresh returns null when retries and credential fallback fail, and scheduleTokenRefresh only rescheduled when a non-null expiration was returned. After the token expired, requests kept using the stale token and received 401s for the remaining lifetime of the catalog, even after the token endpoint recovered. Fix: when refresh fails but the session is still active (keepRefreshed is true), schedule a bounded retry (60s) instead of ending the chain. A closed session (stopRefreshing/close) still stops scheduling. fromAccessToken also schedules a bounded retry when the initial refresh of an expired token fails while the session stays active.
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 #17756
Problem
One failed background OAuth2 token refresh permanently stops refresh for that
AuthSession.scheduleTokenRefreshchains one-shot scheduled tasks: a successfulAuthSession.refresh(client)returns the next expiration, which schedules the following task. When refresh fails (retries exhausted + credential fallback fails),refreshreturnsnull, and the chain simply ends — no manager or request path re-arms it:authenticatekeeps attaching the stale token already stored in the session.Reproduced by the issue: a token endpoint down for ~8s at refresh time (6
Tasksattempts + 1 credential fallback, ~3.1s of sleep) permanently disabled refresh. StarRocks reports the same production failure (StarRocks/starrocks#76438).Fix
Distinguish the three outcomes the scheduling chain needs:
stopRefreshing()/close(), i.e.keepRefreshed == false) → do not schedule again.scheduleTokenRefresh: whenrefresh()returnsnullbutsession.config().keepRefreshed()is stilltrue, reschedule with a fixed 60s bounded wait instead of dropping the chain.fromAccessTokeninitial-refresh path: when the initial refresh of an already-expired token fails while the session stays active, schedule the same bounded retry instead of leavingexpiresAtMillisnull (which skipped scheduling entirely).A fixed bounded retry avoids hammering an unavailable token endpoint while still allowing recovery, and it automatically stops for closed sessions (no unconditional
null-rescheduling loop).Tests
Three new tests in
TestOAuth2Util:failedRefreshSchedulesBoundedRetryWhileSessionStaysActive— a scheduled refresh that fails (token endpoint down) reschedules with a ~60s delay.closedSessionDoesNotRescheduleAfterFailedRefresh— afterclose(), a failed refresh does not reschedule.initialRefreshFailureSchedulesBoundedRetryWhenSessionStaysActive— an already-expired token whose initial refresh fails still schedules the bounded retry.All 12 tests in
TestOAuth2Utilpass.