Github user markap14 commented on the issue:
https://github.com/apache/nifi/pull/2668
@bbende thanks for addressing these issues! I can see that we've tackled a
handful of spots that could be leaking the references, and I've seen the heap
dumps showing that they are no longer concerns. The changes to the Abstract
Hadoop Processors are less desirable than I'd prefer, but I agree that the API
doesn't really expose what it would need to expose in order to do this more
effectively, so it's a reasonable workaround. The LogRepositoryFactory change
in FlowController's create* methods is good - i hadn't realized that we were
doing that there, within the nar loader.
With this particular scenario, I don't think it's really replicable in a
unit test. All that could be done in unit tests would be to verify that very
trivial functionality like "removeLogger() calls remove() on the underlying
map" which make for bad unit testing IMO. You could attempt to create some very
involved integration tests, but they would involve mocking out so much that it
would be hard to know what is really being verified. On top of that, you still
would not be able to verify that the appropriate objects are reclaimable via
garbage collection.
This all looks good to me, though. Can verify behavior and code changes
look good. +1 will merge to master.
---
