NIFI-13713 Moving initializtion of NAR Manager to after Jetty start i…

[bbende]

…s done

Summary

NIFI-13713

Tracking

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

Issue Tracking

• Apache NiFi Jira issue created

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

[exceptionfactory]

Thanks for addressing this issue @bbende.

On initial review, it seems like it would be worth considering alternative approaches, such as some time of asynchronous loading in Jetty Server itself. Although Jetty Server already retrieves several objects from the Spring Application Context, coupling the initialization of a particular object like the NarManager seems a bit brittle. It may be the best option, but worth evaluating other possibilities for signaling.

[bbende]

@exceptionfactory thanks for the review! I refactored the approach so that the original loadExtensionsUi(Set<Bundle>) method now adds the bundles to a queue, and then a background task polls the queue to process the bundles, and the background task is not started until after Jetty has been started to avoid any issues with the Spring WebApplicationContext not being available yet.

[exceptionfactory]

Thanks for reworking the approach @bbende, the polling strategy looks good.

On the implementation, what do you think about using a ScheduledExecutorService? With that approach, the thread could continue running on the scheduled interval, the Executor could be initialized earlier, and executor shutdown could be a bit cleaner.

[bbende]

@exceptionfactory I actually implemented that way at first and had it working like that, but then I convinced myself that it wasn't really taking advantage of having a blocking queue and by polling the blocking queue we get almost immediate response when something is submitted, as opposed to waiting til the next time the scheduled task runs. Since the interval was only 5 seconds it doesn't really make a big difference so I can go back to the executor if you think it is cleaner. In that case it doesn't really poll the queue, it just calls drainTo to take everything at the time the task runs.

[exceptionfactory]

Thanks for the reply @bbende, that's a good point about the faster response time in the current approach.

That part sounds good. The only other change I would consider then is instantiating the Task in the declaration. That way it will never be null, just in case of an earlier startup failure.

[exceptionfactory]

Thanks again for working through the feedback @bbende, the latest version looks good! +1 merging