nastra opened a new pull request, #7251: URL: https://github.com/apache/iceberg/pull/7251
This removes token refresh scheduling from the `S3V4RestSignerClient` as we were previously resource leaking, because we were scheduling token refreshes for tokens that might not be in use anymore. The other problem we have here is that there's no good way of detecting when the `ScheduledExecutorService` can be properly shut down. When we create an `AuthSession` from the given token, a refresh will be performed inside `AuthSession.fromAccessToken(..) if the token expired. This refresh relies on the expiration being set on the token itself as can be seen [here](https://github.com/apache/iceberg/blob/c07fa0f9c208ecb2e0824b63ebd1ffc9e260f2bb/core/src/main/java/org/apache/iceberg/rest/auth/OAuth2Util.java#L542). I have removed the fallback to reading the token expiration from `TOKEN_EXPIRES_IN_MS` because we were only using this property to schedule a token refresh. One issue that might happen is that the token might expire between checking whether it expired and sending a request to the server. In https://github.com/apache/iceberg/blob/c07fa0f9c208ecb2e0824b63ebd1ffc9e260f2bb/core/src/main/java/org/apache/iceberg/rest/auth/OAuth2Util.java#L541 we could try and refresh the token if we know it will expire in the next X seconds. /cc @rdblue @danielcweeks -- 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]
