NIFI-15791 - Add per-secret TTL cache to ParameterProviderSecretsManager#11100
NIFI-15791 - Add per-secret TTL cache to ParameterProviderSecretsManager#11100exceptionfactory merged 2 commits intoapache:mainfrom
Conversation
exceptionfactory
left a comment
exceptionfactory
left a comment
There was a problem hiding this comment.
Thanks for putting this together @pvillard31.
On initial review, it would be helpful to update the Administrator's Guide with documentation for the new application property.
Regarding the property name, what do you think about cache.duration or cache.expiration instead of cache.ttl? Using a word instead of the acronym seems a bit clearer, although TTL is more common than some acronyms.
|
Thanks for the initial review. There is no documentation yet for Connectors but I added a section for the configuration of the Secrets Manager. |
exceptionfactory
left a comment
exceptionfactory
left a comment
There was a problem hiding this comment.
Thanks for the renaming @pvillard31, I noted a few more more recommendations, and then this should be ready to go.
| } | ||
|
|
||
| private boolean isExpired(final CachedSecret cached) { | ||
| return Duration.ofNanos(System.nanoTime() - cached.timestampNanos()).compareTo(cacheDuration) >= 0; |
There was a problem hiding this comment.
Recommend breaking this out to multiple lines to declare intermediate variables for easier reading. It should also be possible to compare on Duration to another.
| * @param key the property key | ||
| * @return the property value, or {@code null} | ||
| */ | ||
| String getProperty(String key); |
There was a problem hiding this comment.
Recommend naming this getApplicationProperty() for clarity, since it sources from NiFi application properties.
exceptionfactory
left a comment
exceptionfactory
left a comment
There was a problem hiding this comment.
Thanks for making the updates @pvillard31, the latest version looks good. +1 merging
2 participants
Summary
NIFI-15791 - Add per-secret TTL cache to ParameterProviderSecretsManager
Adds a TTL-based cache for resolved secret values in ParameterProviderSecretsManager to avoid redundant calls to ParameterProvider.fetchParameterValues() on every secret lookup. Each secret is cached independently by its fully qualified name and expires after a configurable TTL (default 5 minutes). Caching can be disabled by setting the TTL to 0 sec.
A follow-up improvement will be done to expose an API to invalidate the cache in case a user wants to force the refresh on the secrets while configuring a connector.
Changes
SecretsManager— AddedinvalidateCache()to support programmatic cache clearing.SecretsManagerInitializationContext— AddedgetProperty(String)to receive configuration properties without coupling the API toNiFiProperties.NiFiProperties— Addednifi.secrets.manager.cache.ttlproperty key.StandardSecretsManagerInitializationContext— Extended to accept and expose an immutable properties map, implementing the newgetProperty()method.FlowController.createSecretsManager()— Reads the cache TTL fromNiFiPropertiesand passes it through the initialization context.ParameterProviderSecretsManager— Core caching implementation:ConcurrentHashMapcache keyed by FQN with individual timestamps.0disables all cache reads and writes, preserving existing behavior.getSecret()checks cache before calling the provider;getSecrets()partitions references into cache hits and misses, batch-fetching only the misses.findProvider()refactored to accept a pre-fetched provider set to avoid redundantgetSecretProviders()calls.invalidateCache()clears all cached entries.Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000NIFI-00000VerifiedstatusPull Request Formatting
mainbranchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
./mvnw clean install -P contrib-checkLicensing
LICENSEandNOTICEfilesDocumentation