snmvaughan opened a new issue, #6508: URL: https://github.com/apache/datafusion-comet/issues/6508
**Problem.** A `CometS3CredentialProvider` returns `CometS3Credentials` with an `expirationEpochMillis`. The two native paths treat it differently, and neither matches what the docs say. - **Iceberg (opendal and reqsign).** `CometS3CredentialBridge::provide_credential` turns the expiry into reqsign's `expires_in`. reqsign checks it on every request and every retry: it reuses a credential only while it has more than 120 s left (`reqsign-aws-core` 3.1.1, `Credential::is_valid`), and it signs only with more than 10 s left (`reqsign-aws-v4` 3.3.0, `CREDENTIAL_OPERATION_HEADROOM`). An expiry of 0 or less becomes now plus 5 minutes. - **Parquet (object_store).** `CometS3CredentialBridge::get_credential` drops the expiry, since object_store's `AwsCredential` has no field for it. The provider is called on every S3 request, so nothing goes stale in a cache, but a credential with seconds left is still used. object_store signs a request once and replays that signature on every retry (`object_store` 0.13.2, `aws/client.rs` and `RetryableRequest::send` in `client/retry.rs`), and it does not retry a 403. So a retry after backoff can arrive after the session token expired, and the task fails. Other gaps: - **An unknown expiry can outlast the token.** On the Iceberg path an expiry of 0 becomes 5 minutes, which can be longer than the real token has left, and a failed write is not retried. The Spark 3.x adapter always reports 0 (`spark-3.x` `SdkCredentialExtraction`). The `IcebergRESTVendedS3Provider` reference implementation reports 0 even though Iceberg's `VendedCredentialsProvider` knows the expiry. Comments in both call the 5-minute default safe. - **Call volume.** iceberg-rust builds a new opendal operator, and so a new reqsign signer, for every `Storage` call, so reqsign's cache lasts one operation. A scan makes at least one provider call per data and delete file, even with 1-hour tokens. The Parquet path makes one per HTTP request. - **Odd values.** `Long.MAX_VALUE`, meant as "never", fails `Timestamp::from_millisecond`, and an expiry given in seconds reads as a date in 1970. Either way every Iceberg request fails with `failed to load signing credential`, while Parquet keeps working. - **Docs.** The design notes say opendal "schedules the next refresh" from `expires_in`, and the user guide says object_store's retry layer calls `get_credential()` again after a 403 from an expired session token. Neither is what the pinned versions do. A comment on the `FileIO` cache in `iceberg_common.rs` says the backend caches operators. - **Tests.** No test sends a non-zero expiry through the native bridge. **Proposal.** 1. Both paths: have each bridge reuse its credential until shortly before the expiry the provider reports, then ask again, and keep asking on every request when the expiry is unknown. That closes the Parquet gap and cuts the call volume on both paths. It reverses the documented "Why no Comet-side cache" design, though. Refusing a credential inside a safety margin would keep that design, but leave the call volume as it is. 2. Expiry values: treat `Long.MAX_VALUE` as no expiry, and treat an implausibly early value as unknown, with a warning that names it. 3. JVM side: keep Iceberg's expiry in `IcebergRESTVendedS3Provider`, correct the comments that call the default safe, and document that the Spark 3.x adapter cannot report an expiry. 4. Docs and tests: correct the refresh claims, and add a test that sends a real expiry through the bridge. **Related.** Found while working on #6478 (for #6462), which does not change how expiries are handled. -- 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]
