NIFI-14245 Enable caching in DatabaseUserGroupProvider and DatabaseAccessPolicyProvider to improve performance

[yoshiata]

Summary

NIFI-14245

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

[yoshiata]

I’d really appreciate it if you could review this.

[yoshiata]

I noticed the changes to the unit tests were incorrect, so I’ve fixed them again.
8c1f5d3

This should allow us to test the same behavior as before the patch, meaning we can confirm that enabling the cache doesn’t actually change anything.

[exceptionfactory]

Thanks for proposing this improvement @yoshiata. I have not reviewed all of the details, but I'm concerned about the potential complexity introduced through method synchronization and calls to refresh the entire cache of information. The Jira issue description mentions improving the speed of listing users, but this by itself does not seem like sufficient reason to introduce the complexity of caching. If there are additional benefits, are there are some performance comparisons, that may help determine the relative value of introducing the complexity.

[yoshiata]

@exceptionfactory Thanks for replying! And sorry I didn’t give enough info about the performance.
I left a comment on Jira issue with the performance comparison results.

https://issues.apache.org/jira/browse/NIFI-14245?focusedCommentId=17940552&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-17940552

I implemented caching modeled after FileUserGroupProvider class and FileAccessPolicyProvider class as follows:

• On startup, retrieve all records and store them in the cache
• On updates, write to the database first, then retrieve all records again to rebuild the cache

[exceptionfactory]

Thanks for the reply and performance comparison details on the Jira issue @yoshiata! Although 1000 users is probably on the higher end, that is quite a performance hit.

The reference to the File-based implementations is helpful. I was considering recommending a different approach based on Caffeine Cache, but noting a similar existing approach is useful.

In that light, the strategy seems reasonable in general. I may be able to take a closer look at these changes if others aren't able to review them before me, but I have removed my current request for changes flag.

[yoshiata]

Thanks for removing the request for changes @exceptionfactory! I’ll wait to see if anyone else has feedback.
And let me know if you think of anything else—happy to update!