andygrove opened a new issue, #2534:
URL: https://github.com/apache/datafusion-ballista/issues/2534

   **Is your feature request related to a problem or challenge? Please describe 
what you are trying to do.**
   
   `CustomObjectStoreRegistry::get_store` builds a new store on every call. For 
`s3://` it runs `AmazonS3Builder::from_env()...build()` each time, and for 
`http(s)://` it builds a new `HttpBuilder` store each time 
([object_store.rs](https://github.com/apache/datafusion-ballista/blob/e25c2a55d70d9692c2982c1d07db90f5b9f61897/ballista/core/src/object_store.rs#L146-L167)).
 Nothing is cached, and `register_store` is `unimplemented!`.
   
   Two calls on the same registry for the same URL return different stores 
(`Arc::ptr_eq` is false).
   
   DataFusion resolves the store once per partition when a file scan opens 
(`FileScanConfig::open_with_args` calls `runtime_env().object_store(..)`), and 
again for listing and writes. Each `AmazonS3` instance owns its HTTP client and 
its credential provider, so with the current registry:
   
   - **No connection reuse across partitions.** Every scan partition gets its 
own connection pool, so each one pays for new TCP and TLS setup to S3.
   - **No credential reuse.** `object_store` caches temporary credentials 
inside the provider it builds per store (`TokenCredentialProvider` and 
friends). With IRSA, each store makes its own `AssumeRoleWithWebIdentity` call, 
and with ECS/EKS container credentials or IMDS, each makes its own metadata 
call. A stage with many partitions starting at once turns into a burst of STS 
calls. Comet hit STS throttling from this same pattern 
(apache/datafusion-comet#6025).
   
   The executor's session runtime cache (#1995) shares the registry across a 
session's tasks, but that doesn't help here because the registry itself holds 
no stores.
   
   I have confirmed the behavior (a new store per call) but have not measured 
the cost on a real cluster.
   
   **Describe the solution you'd like**
   
   Cache stores in the registry, keyed by the URL (scheme and bucket) plus the 
S3 options the store was built from, so that:
   
   - scans of the same bucket with the same config share one client and one 
credential cache
   - a change to the options produces a new store rather than reusing a stale 
one
   
   A process-wide cache would also let different sessions with identical config 
share a store, which the per-session registry can't do.
   
   **Describe alternatives you've considered**
   
   Leaving it to DataFusion's `DefaultObjectStoreRegistry` and registering 
stores up front. That doesn't fit the current design, where stores are built on 
demand from session-level `S3Options`.
   
   **Additional context**
   
   The key needs to include the options, or this would make #2533 worse. Found 
while working on #2532, which proposes the same store cache as part of 
pluggable credential providers. This issue is the part that stands on its own.
   


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