LuciferYang commented on PR #12423: URL: https://github.com/apache/gravitino/pull/12423#issuecomment-5265862653
Thanks! I went through all ~16 other `ServiceLoader.load` sites. Grouped by whether the same refinement fits: **Init-once already (per-call caching adds nothing)** — scanned once at construction, a guarded singleton init, or a one-shot CLI command, not on a per-request path: - `LakehouseTableDelegatorFactory` — guarded static holder, memoized. - `FileSystemUtils` / `BaseGVFSOperations` — loaded once into an `ImmutableMap` field at catalog-ops init / ctor. - `KmsClientRegistry` — constructed once from the `GravitinoEnv` singleton. - `SqlSessionFactoryHelper` — double-checked `init()`, baked into the singleton `SqlSessionFactory`. - `WebIdentityTokenSources` — loaded once in the credential generator's `initialize`. - optimizer `ProviderUtils` / `InstanceLoaderUtils` — resolved in optimizer object ctors / short-lived CLI commands. **Per-classloader by design (a static type→impl cache would be *wrong*)** — they pass an isolated or thread-context classloader so different catalogs/plugins resolve different impls: `CatalogManager`, `BaseAuthorization`, `AuxiliaryServiceManager`, `CredentialProviderFactory` (resolves against the catalog's isolated CL that `IsolatedClassLoader.withClassLoader` installs), Trino's `GravitinoConnectorPluginManager` (fresh per-plugin `PluginClassLoader`), and `BuiltInJobTemplateEventListener` (a throwaway `URLClassLoader` that's *closed* right after — caching across it would be actively unsafe). A global cache would bind whichever impl loaded first. **The one genuine analog:** `GravitinoCatalogStore` (flink-connector) re-scans the Flink `Factory` SPI on every `storeCatalog`/`getCatalog`, no cache. It's cacheable, but it uses single-arg `ServiceLoader.load(Factory.class)` (thread-context CL, which Flink may swap to a user-code CL), so it'd want a per-classloader cache rather than a plain static map — and the win is smaller than the per-request credential path. Happy to do it as a separate follow-up if you think it's worth it. -- 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]
