Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NIFI-13560 Change Parameter Provider handling to avoid storing values #9102

Merged
merged 1 commit into from
Jul 23, 2024

Conversation

exceptionfactory
Copy link
Contributor

Summary

NIFI-13560 Refactors Parameter Provider handling to avoid storing fetched Parameter Values in the persisted flow configuration.

The implementation change retains memory-based storage of fetched Parameter Values ,but stores a reference string in the serialized flow configuration. When the framework starts, the flow synchronization process fetches current values from referenced Parameter Providers.

This change requires configured Parameter Provider services to be accessible on startup, but otherwise retains existing behavior. This strategy restores centralized management of external Parameter Values to referenced external services. The nature of external vaults and secrets managers necessitates high availability, so the requirement for accessibility on application start should be reasonable for production deployments.

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

Pull Request Tracking

  • Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
  • Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000

Pull Request Formatting

  • Pull Request based on current revision of the main branch
  • Pull Request refers to a feature branch with one commit containing changes

Verification

Please indicate the verification steps performed prior to pull request creation.

Build

  • Build completed using mvn clean install -P contrib-check
    • JDK 21

Licensing

  • New dependencies are compatible with the Apache License 2.0 according to the License Policy
  • New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

  • Documentation formatting appears as expected in rendered files

- Added ParameterValueMapper for handling serialization of Parameter Values for Flow Configuration
- Added Parameter Group retrieval method for Flow Synchronizer
@bbende bbende self-requested a review July 23, 2024 13:35
Copy link
Contributor

@bbende bbende left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, I was doing some testing using the KubernetesSecretParameterProvider just as way to easily use local files and I did a test where I stopped NiFi and moved the directory with the parameter groups, started NiFi back up and it got an exception on start up as expected:

2024-07-23 10:29:42,384 WARN [main] o.e.jetty.ee10.webapp.WebAppContext Failed startup of context oeje10w.WebAppContext@7c5c20ed{nifi-api,/nifi-api,b=file:///Users/bbende/Projects/nifi/nifi-assembly/target/nifi-2.0.0-SNAPSHOT-bin/nifi-2.0.0-SNAPSHOT/work/jetty/nifi-web-api-2.0.0-SNAPSHOT.war/webapp/,a=AVAILABLE,h=oeje10s.SessionHandler@350f18a6{STARTED}}{./work/nar/extensions/nifi-server-nar-2.0.0-SNAPSHOT.nar-unpacked/NAR-INF/bundled-dependencies/nifi-web-api-2.0.0-SNAPSHOT.war}
org.apache.nifi.controller.serialization.FlowSynchronizationException: java.lang.IllegalStateException: Error fetching parameters for ParameterProvider[id=dfef98ea-0190-1000-2666-d1d2efc702b2]: Cannot read the array length because "<local8>" is null
	at org.apache.nifi.controller.serialization.VersionedFlowSynchronizer.synchronizeFlow(VersionedFlowSynchronizer.java:459)
	at org.apache.nifi.controller.serialization.VersionedFlowSynchronizer.sync(VersionedFlowSynchronizer.java:224)
	at org.apache.nifi.controller.FlowController.synchronize(FlowController.java:1718)
	at org.apache.nifi.persistence.StandardFlowConfigurationDAO.load(StandardFlowConfigurationDAO.java:91)
	at org.apache.nifi.controller.StandardFlowService.loadFromBytes(StandardFlowService.java:811)
	at org.apache.nifi.controller.StandardFlowService.load(StandardFlowService.java:532)
	at org.apache.nifi.web.contextlistener.ApplicationStartupContextListener.contextInitialized(ApplicationStartupContextListener.java:67)
	at org.eclipse.jetty.ee10.servlet.ServletContextHandler.callContextInitialized(ServletContextHandler.java:1591)
	at org.eclipse.jetty.ee10.servlet.ServletContextHandler.contextInitialized(ServletContextHandler.java:497)
	at org.eclipse.jetty.ee10.servlet.ServletHandler.initialize(ServletHandler.java:670)
	at org.eclipse.jetty.ee10.servlet.ServletContextHandler.startContext(ServletContextHandler.java:1325)
	at org.eclipse.jetty.ee10.webapp.WebAppContext.startWebapp(WebAppContext.java:1342)
	at org.eclipse.jetty.ee10.webapp.WebAppContext.startContext(WebAppContext.java:1300)
	at org.eclipse.jetty.ee10.servlet.ServletContextHandler.lambda$doStart$0(ServletContextHandler.java:1047)
	at org.eclipse.jetty.server.handler.ContextHandler$ScopedContext.call(ContextHandler.java:1244)
	at org.eclipse.jetty.ee10.servlet.ServletContextHandler.doStart(ServletContextHandler.java:1044)
	at org.eclipse.jetty.ee10.webapp.WebAppContext.doStart(WebAppContext.java:499)
	at org.eclipse.jetty.util.component.AbstractLifeCycle.start(AbstractLifeCycle.java:93)
	at org.eclipse.jetty.util.component.ContainerLifeCycle.start(ContainerLifeCycle.java:169)
	at org.eclipse.jetty.util.component.ContainerLifeCycle.doStart(ContainerLifeCycle.java:120)
	at org.eclipse.jetty.server.Handler$Abstract.doStart(Handler.java:491)
	at org.eclipse.jetty.util.component.AbstractLifeCycle.start(AbstractLifeCycle.java:93)
	at org.eclipse.jetty.util.component.ContainerLifeCycle.start(ContainerLifeCycle.java:169)
	at org.eclipse.jetty.util.component.ContainerLifeCycle.doStart(ContainerLifeCycle.java:120)
	at org.eclipse.jetty.server.Handler$Abstract.doStart(Handler.java:491)
	at org.eclipse.jetty.util.component.AbstractLifeCycle.start(AbstractLifeCycle.java:93)
	at org.eclipse.jetty.util.component.ContainerLifeCycle.start(ContainerLifeCycle.java:169)
	at org.eclipse.jetty.util.component.ContainerLifeCycle.doStart(ContainerLifeCycle.java:120)
	at org.eclipse.jetty.server.Handler$Abstract.doStart(Handler.java:491)
	at org.eclipse.jetty.util.component.AbstractLifeCycle.start(AbstractLifeCycle.java:93)
	at org.eclipse.jetty.util.component.ContainerLifeCycle.start(ContainerLifeCycle.java:169)
	at org.eclipse.jetty.server.Server.start(Server.java:624)
	at org.eclipse.jetty.util.component.ContainerLifeCycle.doStart(ContainerLifeCycle.java:120)
	at org.eclipse.jetty.server.Handler$Abstract.doStart(Handler.java:491)
	at org.eclipse.jetty.server.Server.doStart(Server.java:565)
	at org.eclipse.jetty.util.component.AbstractLifeCycle.start(AbstractLifeCycle.java:93)
	at org.apache.nifi.web.server.JettyServer.start(JettyServer.java:863)
	at org.apache.nifi.NiFi.<init>(NiFi.java:172)
	at org.apache.nifi.NiFi.<init>(NiFi.java:83)
	at org.apache.nifi.NiFi.main(NiFi.java:332)
Caused by: java.lang.IllegalStateException: Error fetching parameters for ParameterProvider[id=dfef98ea-0190-1000-2666-d1d2efc702b2]: Cannot read the array length because "<local8>" is null
	at org.apache.nifi.controller.parameter.StandardParameterProviderNode.fetchParameters(StandardParameterProviderNode.java:286)
	at org.apache.nifi.controller.serialization.VersionedFlowSynchronizer.getProvidedParameters(VersionedFlowSynchronizer.java:1017)
	at org.apache.nifi.controller.serialization.VersionedFlowSynchronizer.getProvidedParameters(VersionedFlowSynchronizer.java:1010)
	at org.apache.nifi.controller.serialization.VersionedFlowSynchronizer.createParameterMap(VersionedFlowSynchronizer.java:858)
	at org.apache.nifi.controller.serialization.VersionedFlowSynchronizer.addParameterContext(VersionedFlowSynchronizer.java:808)
	at org.apache.nifi.controller.serialization.VersionedFlowSynchronizer.inheritParameterContext(VersionedFlowSynchronizer.java:796)
	at org.apache.nifi.controller.serialization.VersionedFlowSynchronizer.lambda$inheritParameterContexts$4(VersionedFlowSynchronizer.java:782)
	at org.apache.nifi.controller.flow.AbstractFlowManager.withParameterContextResolution(AbstractFlowManager.java:668)
	at org.apache.nifi.controller.serialization.VersionedFlowSynchronizer.inheritParameterContexts(VersionedFlowSynchronizer.java:774)
	at org.apache.nifi.controller.serialization.VersionedFlowSynchronizer.synchronizeFlow(VersionedFlowSynchronizer.java:405)
	... 39 common frames omitted
Caused by: java.lang.NullPointerException: Cannot read the array length because "<local8>" is null
	at org.apache.nifi.parameter.KubernetesSecretParameterProvider.getParameterGroup(KubernetesSecretParameterProvider.java:191)
	at org.apache.nifi.parameter.KubernetesSecretParameterProvider.lambda$fetchParameters$0(KubernetesSecretParameterProvider.java:133)
	at java.base/java.lang.Iterable.forEach(Iterable.java:75)
	at org.apache.nifi.parameter.KubernetesSecretParameterProvider.fetchParameters(KubernetesSecretParameterProvider.java:132)
	at org.apache.nifi.controller.parameter.StandardParameterProviderNode.fetchParameters(StandardParameterProviderNode.java:284)
	... 48 common frames omitted

The issue was that it did not fail start up, it continued on an said that JettyServer was started and printed the normal Started Server on https://localhost:8443/nifi.

It seems like we are trying to handle this case in JettyServer after calling start() we then check all the contexts to see if they have an unavailable exception, and if so then trigger shutdown, but that doesn't seem to be happening.

I realize this is somewhat unrelated to the changes in this PR, but since the expected behavior is to fail start up if unable to fetch parameters, I'm thinking maybe this should be improved. What do you think?

@exceptionfactory
Copy link
Contributor Author

Thanks for the feedback and exercising the failure behavior @bbende.

I agree that the JettyServer startup failure behavior should be adjusted. Although it is related, I think it would be better to consider that in a separate issue to consider general failure handling, to include this and other types of context initialization failures.

@bbende
Copy link
Contributor

bbende commented Jul 23, 2024

I'm fine tackling the start up issue in a separate ticket since it is really an existing problem. Everything else is good here so +1

@bbende bbende merged commit b0f419b into apache:main Jul 23, 2024
5 checks passed
@exceptionfactory
Copy link
Contributor Author

Thanks @bbende! I created NIFI-13575 to address the Jetty Server startup behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants