vbhanuchander-lang commented on issue #17518:
URL: https://github.com/apache/iceberg/issues/17518#issuecomment-5518413415
@anoopvs the 401s you are seeing are real, but I do not think
`expireAfterAccess` is the cause, and
switching to `expireAfterWrite` would hide it rather than fix it.
**`expireAfterAccess` is consistent with the intended design.** An
`AuthSession` is meant to refresh
itself in the background: `OAuth2Util.scheduleTokenRefresh` chains one-shot
tasks that fire shortly
before each token expires, so a session's cached token is supposed to stay
valid for as long as the
session is in use. On that assumption the cache TTL is about *idleness* —
evict sessions nobody is
using — and has nothing to do with token lifetime. So yes, it is intentional.
**What actually breaks is the refresh chain giving up permanently.** There
are two places where a
session ends up with no scheduled refresh at all:
1. `scheduleTokenRefresh` re-arms itself only when `session.refresh(client)`
returns non-null. When
a refresh fails — retries exhausted and the credential fallback also
fails — it returns null and
the chain simply ends. Nothing re-arms it.
2. `fromAccessToken`, for a token that is already expired at session
creation, does
`expiresAtMillis = null` on a failed refresh with the comment *"token
refresh failed, don't
reattempt with the original expiration"*, and the `scheduleTokenRefresh`
call below is guarded by
`null != expiresAtMillis` — so refresh is never armed in the first place.
In both cases `authenticate` keeps attaching the stale token, and because
the session is in active
use `expireAfterAccess` keeps renewing its TTL, so it is never evicted
either. That combination is
your symptom: 401s that persist for the life of the catalog object even
after the token endpoint
recovers. The eviction policy is what makes it *durable*, but the refresh
path is what makes it
*wrong*.
That is #17756, and @waterWang's #17768 patches both sites with a bounded
retry. If your 401s
survive that change, the diagnosis here is wrong and it is worth reopening
the question.
**Why I would not switch to `expireAfterWrite` regardless.** It does not fix
either site — the
session still holds a dead token, you just get a periodic forced re-auth on
top. And what recovery
it buys depends on the session type: a credential-based session would fetch
a new token when
reloaded, but a session configured with a static `token` reloads to exactly
the same token, so
eviction changes nothing for it. It would also force re-authentication on
healthy long-lived
sessions that have no problem, which is the cost the current policy is
deliberately avoiding.
--
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]
