You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "andygrove (via GitHub)" <gi...@apache.org> on 2026/09/28 13:46:53 UTC
[PR] perf: [branch-1.1] cache Iceberg FileIO per executor instead of building one per task (#6106) [datafusion-comet]
andygrove opened a new pull request, #6306:
URL: https://github.com/apache/datafusion-comet/pull/6306
Backport of #6106 to `branch-1.1`.
Cherry-picked from `786bbc8fd630abfd0b29bdaffea6168f9f228863` without conflicts. The five files it changes are identical on `branch-1.1` and on `main` just before #6106, so the diff is byte-identical to upstream.
## Which issue does this PR close?
Closes #6105 on `branch-1.1`. #6106 already closed it on `main`.
## Rationale for this change
Every native Iceberg scan or write task builds its own `FileIO`. #6105 counted 40,317 of them for a single query on a production workload. Each one rebuilds the storage factory and its parsed configuration, and for S3 with a custom access provider, the JNI bridge. #6106 merged after `branch-1.1` was cut at 36ab57c68, so 1.1.0 would ship without it.
## What changes are included in this PR?
The original change, so see #6106 for the details. No adaptations were needed. In short:
- `load_file_io` serves clones from a per-executor LRU cache of 64 entries and builds a `FileIO` only on a miss.
- The cache key holds everything that shapes the client: the access mode, the catalog name, the reference path and the catalog properties.
- `memory:///` is never cached. Neither is a read whose S3 access provider failed to initialize, so the next task retries the provider.
- `release_runtime` drains the cache.
## How are these changes tested?
Run locally on `branch-1.1` with the default profile (Spark 4.1, Scala 2.13, JDK 17), against a `libcomet` built from this branch:
- The six new tests in `iceberg_common.rs` pass, along with the rest of the core crate's lib tests, 503 in all.
- `cargo fmt --all -- --check` and `cargo clippy --all-targets --workspace -- -D warnings` pass on rustc 1.97.0.
- `CometIcebergNativeSuite` and `CometIcebergWriteActionSuite` pass: 182 tests, and 1 is canceled by its Spark 4.1 `assume` (SPARK-55626).
#6106 was also verified with the MinIO-backed `IcebergReadFromS3Suite` and the manual `CometS3CredentialBridgeSuite`, which no CI job runs. I did not re-run them here. With this PR, the branch's native tree and its Java S3 SPI are byte-identical to `main` at #6106, apart from #6181's opt-in local TopK fusion in `planner.rs` and #5322's `contains` kernel. So those runs covered the code this PR ships.
Against `branch-1.1`, the changed paths route this pull request to every suite except Spark 3.4's SQL job and the benchmark check. That includes every Spark profile, macOS and all four Iceberg versions.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: [branch-1.1] cache Iceberg FileIO per executor instead of building one per task (#6106) [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove commented on PR #6306:
URL: https://github.com/apache/datafusion-comet/pull/6306#issuecomment-5872155049
Closing this rather than taking it into 1.1.0. On `branch-1.1` the native Iceberg scan is on by default and the native writer is off, so in practice this changes the default read path, and the saving there is small. For S3, GCS and OSS the shared `FileIO` only avoids re-parsing the storage config and, with a custom credential provider, rebuilding the JNI bridge, because iceberg-rust still creates a new operator per file open. The HDFS client reuse that motivated #6106 needs #5898, which isn't in this release, and nothing in CI exercises the S3 path end to end. That's not a trade I want on the release branch.
@sunchao thanks for the review, this isn't about anything in the change itself. #6106 is already on `main`, so it ships in the next release from there.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: [branch-1.1] cache Iceberg FileIO per executor instead of building one per task (#6106) [datafusion-comet]
Posted by "andygrove (via GitHub)" <gi...@apache.org>.
andygrove closed pull request #6306: perf: [branch-1.1] cache Iceberg FileIO per executor instead of building one per task (#6106)
URL: https://github.com/apache/datafusion-comet/pull/6306
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org