danielcweeks commented on code in PR #4830:
URL: https://github.com/apache/iceberg/pull/4830#discussion_r880057643


##########
core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java:
##########
@@ -222,11 +278,61 @@ public boolean updateNamespaceMetadata(SessionContext 
context, Namespace ns,
     return !response.updated().isEmpty();
   }
 
+  private ScheduledExecutorService tokenRefreshExecutor() {
+    if (refreshExecutor == null) {
+      synchronized (this) {
+        if (refreshExecutor == null) {
+          this.refreshExecutor = ThreadPools.newScheduledPool(name() + 
"-token-refresh", 1);
+        }
+      }
+    }
+
+    return refreshExecutor;
+  }
+
+  private void scheduleTokenRefresh(
+      AuthSession session, long startTimeMillis, long expiresIn, TimeUnit 
unit) {
+    // convert expiration interval to milliseconds
+    long expiresInMillis = unit.toMillis(expiresIn);
+    // how much ahead of time to start the request to allow it to complete
+    long refreshWindowMillis = Math.min(expiresInMillis / 10, 
MAX_REFRESH_WINDOW_MILLIS);
+    // how much time to wait before expiration
+    long waitIntervalMillis = expiresInMillis - refreshWindowMillis;
+    // how much time has already elapsed since the new token was issued
+    long elapsedMillis = System.currentTimeMillis() - startTimeMillis;
+    // how much time to actually wait
+    long timeToWait = Math.max(waitIntervalMillis - elapsedMillis, 
MIN_REFRESH_WAIT_MILLIS);
+
+    tokenRefreshExecutor().schedule(
+        () -> {
+          long refreshStartTime = System.currentTimeMillis();
+          Pair<Integer, TimeUnit> expiration = session.refresh(client);
+          if (expiration != null) {
+            scheduleTokenRefresh(session, refreshStartTime, 
expiration.first(), expiration.second());

Review Comment:
   While I think it's probably ok for failing fast in initial OAuth exchanges, 
I think we should probably wrap this in a tasks for retry.  Failing to refresh 
the token due to intermittent issues would cause critical failure for 
long-running processes like Flink/Trino.



-- 
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