karuppayya commented on PR #4335:
URL:
https://github.com/apache/datafusion-comet/pull/4335#issuecomment-4480509432
Thanks @mbutrovich for addressing my concerns. The Rust-side TTL is a JNI
micro-optimization — if the switch between JVM and native isn't expensive, we
can leave it out.
@parthchandra — fair point on the herd. I don't think we should go with
centralized refresh though:
- A driver-scheduled refresh becomes its own bottleneck, especially under
Spark Connect where the Connect server is already a single point handling
client RPCs, planning, and catalog interactions. Pushing per-executor
credential refresh through it risks hitting networking limits before the
credential backend does.
- Iceberg REST's `VendedCredentialsProvider` already takes the distributed
approach — executors refresh against the catalog endpoint directly, independent
of the driver.
- Vendor-side caching (both server and client) collapses(ideally) N
same-scoped requests to ~1 upstream call, so total upstream request count stays
close to centralized refresh in the common case.
Happy to close this PR in favor of #4309 and continue review on the other
— we need this change soon. :slightly_smiling_face:
--
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]
