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-12918 Fix Stateless NullPointerException on versioned sub-process groups - 1.x support branch #8572

Closed
wants to merge 16 commits into from

Conversation

slambrose
Copy link
Contributor

Summary

NIFI-12918

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

Stephanie Ambrose slambr2 and others added 10 commits March 19, 2024 15:45
@slambrose slambrose changed the base branch from main to support/nifi-1.x March 26, 2024 18:26
@slambrose
Copy link
Contributor Author

Okay, I'm trying one more solution. What I've found is that the main branch has new code where a "registryId" is implemented, but it is always null right now because the registry-api does not currently supply a "registryId" property value. The 1.x support branch does not have this new class. I submitted the fix in the PR to the main branch, but for 1.x, I have something working in the temporary until the changes from 2.0 are pushed down to 1.x (if they are going to do that).

@slambrose
Copy link
Contributor Author

Okay, so after some back and forth here is my final conclusion to this bug:

I've submitted two PRs
https://github.com/apache/nifi/pull/8572/checks <-- for support/1.x branch
https://github.com/apache/nifi/pull/8536/checks <-- for main branch

Here is what's happening. If you run a versioned stateless flow that has sub-versioned progress group, there is a NullPointerException thrown on the build method of StandardVersionControlInformation where there is a non-null requirement on "registryId".

Code has changed in main versus support/1.x, but the bug exists on both. What I've found is the JerseyClient calls that are made to map the response from the registry-api back to the higher level VersionedProcessGroup -> VersionedFlowCoordinates both return JSON where the VersionedFlowCoordinates > registryId is always null. Now in the main branch, stateless uses some of these new "synchronizer" classes, so the best place to insert a solution is to add an else on the null check where the code runs the "determineRegistryId" method and set the value to "1" for versioned flows that do not have a registryId associated to them. Doing this passes all of the integration tests and code checks and fixes the bug. Since this class does not exist in the 1.x support branch, my best solution is to just comment out the null check (this breaks integration tests in main, but not in 1.x).

I continued to look further at this idea of a "registryId" within the registry api code. The GET method on buckets/flow/version returns the same class that the stateless code uses to map into on the JerseyClient call. I then looked at the api code to POST new versioned flows, and it also expect the json as a parameter to match the same class. Well, since there is no "non-null" requirement on the registryId, versioned flows are stored in the database (or whichever storage adapter used) with this property as null. I did not check to see if the latest NiFi Registry UI is now setting this property, but making it non-null would break any older version of registry and not be backwards compatible. Therefore, the only solution at this point to fix stateless NiFi in its current state is to just set registryId to "1" if it is null. I believe this code is still probably prototype being worked and evolving, so this will be a temporary fix until those mandates are in place on a concept of "registryId".

@slambrose slambrose closed this Apr 4, 2024
@slambrose
Copy link
Contributor Author

PR has been closed until the 2.0 fix is completed and merged (PR 8536)

@slambrose slambrose reopened this Apr 4, 2024
exceptionfactory pushed a commit that referenced this pull request Apr 7, 2024
This closes #8572

Signed-off-by: David Handermann <exceptionfactory@apache.org>
@exceptionfactory
Copy link
Contributor

Thanks @slambrose, I backported the changes from #8536 and merged the update in 0e920c4

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