laxmanmamididd opened a new pull request, #7464:
URL: https://github.com/apache/datafusion-comet/pull/7464
## Which issue does this PR close?
Closes #7390.
Split out of #7363 at a reviewer's request, so the lifecycle change can be
reviewed and reverted on its own. #7363 is rebased on this branch.
## Rationale for this change
The S3 credential dispatcher keys provider instances by `(FQCN, dispatchKey,
catalogProperties)` and keeps each one until JVM shutdown. A REST catalog that
re-vends credentials gives a table new FileIO properties, so every vended
credential registers a new provider. A provider that holds an HTTP client or an
auth session, as a refreshing one does, then accumulates for the life of the
executor.
## What changes are included in this PR?
- **The dispatcher counts references.**
- `ensureInitialized` takes a reference to the instance it returns.
- A native `CometS3CredentialBridge` holds that reference and shares it
with the bridges it derives (`for_path`, `for_location`). The last of them to
be dropped gives it back through the new `release` and its JNI binding.
- The last release closes the provider and forgets its key, so a later
`ensureInitialized` creates a new instance.
- **The FileIO cache retires superseded entries.** Inserting a table's new
`FileIO` evicts the entries it supersedes. An entry is superseded when it has
the same access mode, catalog and table root (the reference path up to
`/metadata/`) and different properties.
- When both entries carry `s3.session-token-expires-at-ms`, only the later
expiry supersedes. Two sessions holding different tokens for one table then
don't evict each other on every insert.
- A commit that keeps the properties supersedes nothing, so reads of
neighboring snapshots share one `FileIO`.
- The 64-entry LRU bound limits everything else.
- **Docs.** `s3-credential-provider-design.md` and
`s3-credential-providers.md` describe the reference counts and when Comet calls
`close()`.
The Parquet path's object store cache never evicts, so its providers still
live until shutdown.
## How are these changes tested?
Run locally with `-Pspark-4.0` on JDK 17:
- `CometS3CredentialDispatcherTest` (21 tests) includes 5 lifecycle tests:
- release closes a provider only after its last reference;
- a retired key gets a new provider;
- extra and unknown releases are ignored;
- three credential snapshots for one dispatch key close as they are
released;
- concurrent acquire and release never use a closed provider.
- `iceberg_common` Rust tests (24) cover eviction on re-vended credentials,
no eviction for a commit with unchanged properties, eviction only for a later
token expiry, and that other tables, catalogs and access modes keep their
entries.
- `CometS3CredentialBridgeSuite` (10 tests, MinIO) adds `a table's
superseded FileIO releases its CometS3CredentialProvider`. It reads one table
from two sessions whose catalogs give it different FileIO properties, and
checks the dispatcher's `registeredCount()` doesn't grow. With eviction
disabled it fails with 2 registered providers instead of 1.
🤖 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]
