mixermt opened a new pull request, #6106: URL: https://github.com/apache/datafusion-comet/pull/6106
## Which issue does this PR close? Closes #6105. ## Rationale for this change Every native Iceberg scan or write task builds its own `FileIO`, and since iceberg-rust creates the storage client lazily per instance, every task also opens and tears down its own storage client. On a production workload that was 40,317 client set-ups against one endpoint for a single query. Spark's Hadoop layer shares one client per executor JVM instead. Details in #6105. ## What changes are included in this PR? `load_file_io` in `iceberg_common.rs` now serves clones of a per-executor cache; the previous body becomes `build_file_io` and runs only on a miss. Clones share iceberg-rust's `Arc<OnceLock<Arc<dyn Storage>>>`, so all tasks on an executor reuse one client. The cache key is everything that shapes the client: - access mode, since read and write can be granted different access by the S3 bridge; - catalog name, the dispatch key for that bridge; - the full reference path, not just `scheme://authority`: the S3 bridge is constructed with the bucket and `url.path()` of the reference location and the JVM provider is called with exactly that pair, so a bridge built for one table must not serve another; - the whole catalog property bag, so rotated access keys or a changed endpoint produce a new entry rather than a stale client. `memory:///` is never cached; the write path assembles manifest bytes there and each task needs a private namespace. The map is cleared past 64 entries so per-query rotation cannot grow it without bound, and `release_runtime` clears it with the Tokio runtime. Running tasks keep their own clones either way. ### Storage backends and catalogs Every arm of `storage_factory_for` was checked against the key: | Backend | Client state | Covered by | |---|---|---| | `file` | none | sharing is harmless | | `memory` | in-process namespace | excluded from the cache | | `s3`, `s3a`, s3-compliant aliases with static keys | client from properties | properties in the key | | S3 with a custom access provider (JNI bridge) | bridge scoped to bucket and path | reference path and catalog name in the key; opendal's expiry-aware cache on the shared bridge now also spares the per-task JNI fetch | | S3 with REST catalog vending | per-table, rotating properties | properties in the key; a rotation is a new entry, the cap bounds growth | | `gs`, `oss` | client from properties | properties in the key | | `hdfs` (with #5898) | NameNode session | properties and path in the key; this is the case that motivated the change | Sharing a factory across tasks is only possible because `Arc<dyn StorageFactory>` requires `Send + Sync`, so the compiler already guarantees the bridge is safe to use from several tasks at once. ## How are these changes tested? - New unit test `load_file_io_is_cached_per_storage_client_configuration`: the same configuration is served once; a different access mode, a different table path, a changed property, and `memory:///` each miss or bypass the cache. - `cargo clippy --all-targets -p datafusion-comet -- -D warnings` is clean. - JVM, with the rebuilt library: `CometIcebergNativeSuite` (reads, DPP, planning metrics), `CometIcebergWriteActionSuite` (native writes, manifests through `memory:///`), and the MinIO-backed `IcebergReadFromS3Suite`. The S3 suite is the meaningful one for the key: it runs REST catalog vending with wrong access immediately before correct access against the same bucket and catalog, and the rejected client is not reused. - Not measured here: the end-to-end gain on the production workload, which needs a run with this build. ## AI Disclosure Drafted, implemented and tested with AI assistance (Claude Code); reviewed before submission. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
