aboueleyes opened a new issue, #17756:
URL: https://github.com/apache/iceberg/issues/17756

   ### Apache Iceberg version
   
   1.11.0 (latest release)
   
   ### Query engine
   
   None
   
   ### Please describe the bug 🐞
   
   One failed background OAuth2 token refresh permanently stops refresh for 
that `AuthSession`.
   
   After the existing token expires, REST catalog requests continue using it 
and receive 401 responses for the remaining lifetime of the catalog object. 
Restoring the token endpoint does not recover the session.
   
   I reproduced this against Iceberg 1.10.0 with a standalone HTTP stub. The 
same control flow is present
   in 1.11.0 and on `main`:
   
   
https://github.com/apache/iceberg/blob/main/core/src/main/java/org/apache/iceberg/rest/auth/OAuth2Util.java
   
   ### Cause
   
   Token refresh is implemented as a chain of one-shot scheduled tasks. Each 
successful refresh schedules the next task:
   
   ```java
   Pair<Integer, TimeUnit> expiration = session.refresh(client);
   if (expiration != null) {
     scheduleTokenRefresh(
         client,
         executor,
         session,
         refreshStartTime + expiration.second().toMillis(expiration.first()));
   }
   ```
   
   `AuthSession.refresh` returns null after its refresh attempts and credential 
fallback fail. The scheduled task then returns without scheduling another task. 
No manager or request path re-arms the catalog session:
   
   - `authenticate` continues attaching the token already stored in the session.
   - `OAuth2Manager.contextualSession` continues using or deriving from the 
existing catalog session.
   - A 401 is mapped to `NotAuthorizedException` and does not initiate token 
renewal.
   
   The current token is normally still valid when the background refresh fails, 
so the visible 401s
   begin later, after the refresh warning is no longer near the failing request 
in the log.
   
   ### Retry window
   
   The refresh uses `Tasks.retry(5)`, which gives six attempts. With the 
configured exponential backoff,
   the sleeps are approximately 100, 200, 400, 800, and 1600 milliseconds. That 
is about 3.1 seconds of
   sleep before the attempt count is exhausted. Request duration adds to that 
time.
   
   `onFailure` then attempts credential-based recovery once. If that also 
fails, refresh returns null and
   the scheduling chain ends.
   
   The 30-minute maximum retry duration does not normally bind because the 
attempt count is exhausted
   first.
   A token endpoint that rejects requests quickly only needs to be unavailable 
for a few seconds at the refresh time.
   
   ### Reproduction
   
   The standalone stub:
   
   1. Issues an access token with `expires_in: 2`.
   2. Allows one catalog listing.
   3. Returns 503 from the token endpoint for eight seconds, covering the 
refresh attempts.
   4. Restores the token endpoint.
   5. Waits another eight seconds, covering four more token lifetimes.
   
   Observed output on 1.10.0:
   
   ```text
   token request 1 -> 200, expires in 2s
   connected, token requests so far: 1
   first listing ok
   taking the token endpoint down for the refresh window
   token request 2 -> 503
   token request 3 -> 503
   token request 4 -> 503
   token request 5 -> 503
   token request 6 -> 503
   token request 7 -> 503
   token request 8 -> 503
   token endpoint back up after 8 token requests
   
   token requests during the outage : 8
   token requests after recovery    : 0
   RESULT: the refresh chain never restarted
   ```
   
   The seven failed calls consist of six `Tasks` attempts and one 
credential-based fallback. After the
   endpoint recovers, there are no further token requests.
   
   ### Expected behavior
   
   A temporary token-endpoint failure should delay refresh rather than 
permanently disable it. Once the endpoint recovers, the session should obtain a 
new token without requiring the catalog object or host process to be restarted.
   
   Permanent authentication failures should not cause aggressive retries 
forever. The retry behavior should distinguish a temporary endpoint failure 
from a session that has been closed or credentials that have been rejected.
   
   ### Production evidence
   
   StarRocks has an independently reported production-facing failure with 
Iceberg Core 1.10.0 and the same root cause. After one refresh cycle fails, 
metadata requests eventually return 401 until the catalog is rebuilt or the 
process restarts:
   
   https://github.com/StarRocks/starrocks/issues/76438
   
   StarRocks also has its own missing catalog-recovery path, but that does not 
change the Iceberg client behavior: the Iceberg `AuthSession` stops refreshing 
after the failed cycle.
   
   ### Related Iceberg issues
   
   - https://github.com/apache/iceberg/issues/12363 reports token expiry in 
Spark, but does not isolate
     the failed refresh scheduling chain.
   - https://github.com/apache/iceberg/issues/13591 discusses whether 
expired-token recovery uses the
     correct OAuth flow.
   - https://github.com/apache/iceberg/issues/12196 requests support for the 
OAuth `refresh_token` grant.
   
   These are related OAuth discussions, but none adds recovery after the 
scheduled refresh chain stops.
   
   ### Fix direction for discussion
   
   The scheduling code needs an outcome that distinguishes at least:
   
   - Refresh succeeded, schedule from the new expiration.
   - Refresh failed temporarily, schedule a bounded delayed retry.
   - Refresh is disabled, or the session was closed, do not schedule again.
   
   The current null return cannot make that distinction. It may represent a 
failed refresh, disabled refresh, a missing token, or a response without 
`expires_in`.
   
   Simply rescheduling every time `refresh` returns null would therefore be 
unsafe. Token refresh uses the process-wide `ThreadPools.authRefreshPool()`, 
and closing an `AuthSession` changes its `keepRefreshed` state but does not 
shut down that shared executor. An unconditional retry could keep rescheduling 
a closed session.
   
   A bounded backoff for temporary failures, with an explicit stopped outcome 
for closed sessions and permanent credential errors, would allow recovery 
without continuously calling an unavailable token
   endpoint.
   
   I would like maintainer guidance on the preferred outcome model and backoff 
policy before preparing a PR.
   
   
   ### Willingness to contribute
   
   - [ ] I can contribute a fix for this bug independently
   - [x] I would be willing to contribute a fix for this bug with guidance from 
the Iceberg community
   - [ ] I cannot contribute a fix for this bug at this time


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to