rdblue commented on code in PR #7270:
URL: https://github.com/apache/iceberg/pull/7270#discussion_r1156265000
##########
aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java:
##########
@@ -131,6 +135,26 @@ private ScheduledExecutorService tokenRefreshExecutor() {
return tokenRefreshExecutor;
}
+ @Value.Lazy
+ Cache<String, AuthSession> authSessionCache() {
+ long expirationIntervalMs =
+ PropertyUtil.propertyAsLong(
+ properties(),
+ CatalogProperties.AUTH_SESSION_TIMEOUT_MS,
+ CatalogProperties.AUTH_SESSION_TIMEOUT_MS_DEFAULT);
Review Comment:
Okay, after some discussion to clarify Dan, Eduard, and I think 1h is pretty
reasonable. The cache is using `expireAfterAccess` so reusing the token keeps
it alive. And the removal listener stops refreshing the token when it is
expired from the cache. The downside of too short of an interval is starting
new sessions, but only if they are already mostly inactive (more than 1 hour
between uses). The downside of too long of an interval is potentially
needlessly refreshing a token for 24 hours when it's unused. I think it is
better to have a short interval.
--
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]
