[jira] [Commented] (NIFI-4994) Reload instance class loader at every component restart

["ASF GitHub Bot (JIRA)" <ji...@apache.org>]

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

ASF GitHub Bot commented on NIFI-4994:
--------------------------------------

GitHub user ijokarumawak opened a pull request:

    https://github.com/apache/nifi/pull/2568

    WIP NIFI-4994: Enable instance classloader restart without condition

    ## This PR is Work In Progress to gather opinions
    
    Not ready for fully review cycle, but any feedback is appreciated!
    
    ---
    
    Thank you for submitting a contribution to Apache NiFi.
    
    In order to streamline the review of the contribution we ask you
    to ensure the following steps have been taken:
    
    ### For all changes:
    - [x] Is there a JIRA ticket associated with this PR? Is it referenced 
         in the commit message?
    
    - [x] Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
    
    - [x] Has your PR been rebased against the latest commit within the target branch (typically master)?
    
    - [x] Is your initial contribution a single, squashed commit?
    
    ### For code changes:
    - [ ] Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder?
    - [ ] Have you written or updated unit tests to verify your changes?
    - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? 
    - [ ] If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly?
    - [ ] If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly?
    - [ ] If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties?
    
    ### For documentation related changes:
    - [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
    
    ### Note:
    Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/ijokarumawak/nifi nifi-4994

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/nifi/pull/2568.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #2568
    
----
commit 8165d871e64778a4bab7e75877cd93d662f26b77
Author: Koji Kawamura <ij...@...>
Date:   2018-03-20T07:54:17Z

    WIP NIFI-4994: Enable instance classloader restart without condition

----


> Reload instance class loader at every component restart
> -------------------------------------------------------
>
>                 Key: NIFI-4994
>                 URL: https://issues.apache.org/jira/browse/NIFI-4994
>             Project: Apache NiFi
>          Issue Type: Improvement
>          Components: Core Framework
>            Reporter: Koji Kawamura
>            Assignee: Koji Kawamura
>            Priority: Major
>
> h2. Current Class loader reload mechanism
> NiFi framework provides RequiresInstanceClassLoading to annotate components (Processor, Controller Service or Reporting Task) to use per instance class loader. Instance class loaders are reloaded when associated components are restarted, if any component property flagged as 'dynamicallyModifiesClasspath' is updated. Such component property is used to let user specify additional class pass resource URLs. E.g. PutHDFS 'Additional Classpath Resources'.
> h2. Limitation of current mechanism
> Current mechanism requires component properties to trigger reloading to represent class path resource URLs. That works well in scenarios for users to add/del/mod additional class path resources.
> However there is different needs for reloading instance class loader. That is re-initializing static state in dependencies.
> For example, ReportLineageToAtlas reporting task uses Atlas client library, and the library has static code blocks to initialize its dependencies, such as Kafka client with parameters passed by NiFi reporting task based on user inputs. Once the initialization is done, there is no way to re-initialize those objects, meaning even if NiFi user changes reporting task configuration, those will not be able to reflected. 
> https://github.com/apache/atlas/blob/master/notification/src/main/java/org/apache/atlas/hook/AtlasHook.java#L63
> The only possible operations to reset such static state are, creating another instance, configure the instance again, remove the old one and start new one. Or restart NiFi process. This limitation affects UX negatively.
> h2. Improvement design
> Possible approaches are:
> # Enhance RequiresInstanceClassLoading annotation so that custom components can specify whether it needs to be reloaded on every component restart without condition.
> # Add new PropertyDescriptor instance field to denote that instance class loader needs to be reloaded if a property is changed. Similar to 'dynamicallyModifiesClasspath' but for configurations other than URI. Then, include those property value to AbstractConfiguredComponent.additionalResourcesFingerprint
> No.1 is more naive approach, but require less framework code change. No.2 looks redundant, also could be error-prone, e.g. by forgetting to mark some properties.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)