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

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

Github user olegz commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/836#discussion_r74704726
  
    --- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/StandardProcessorNode.java
 ---
    @@ -1321,12 +1325,21 @@ public void run() {
                             this.unscheduled = true;
                         }
                         try {
    -                        if (activeThreadMonitorCallback.call()) {
    +                        if (scheduleState.isScheduled()) {
    +                            
schedulingAgent.unschedule(StandardProcessorNode.this, scheduleState);
    +                        }
    --- End diff --
    
    So, after further digging I see that invoking ```OnUnscheduled``` doesn't 
mean that the actual PN is unscheduled since we still need to invoke 
```getSchedulingAgent(procNode).unschedule(procNode, state);```.
    If so we probably have to encapsulate this better where call to 
```getSchedulingAgent(procNode).unschedule(procNode, state);``` should initiate 
call to @OnUnschedule```. 
    Also, I am not sure I like leaking ```SchedulingAgent``` into the 
ProcessNode, that is why the callback felt more appropriate. With that in mind 
and in spirit of current changes wouldn't this (see below for full 
implementation of run()) be the same?
    ```java
    scheduler.execute(new Runnable() {
                    boolean unscheduled = false;
                    @Override
                    public void run() {
                        if (!this.unscheduled){
                            
ReflectionUtils.quietlyInvokeMethodsWithAnnotation(OnUnscheduled.class, 
processor, processContext);
                            try {
                                this.unscheduled = 
activeThreadMonitorCallback.call();
                                if (this.unscheduled) {
                                    try (final NarCloseable nc = 
NarCloseable.withNarLoader()) {
                                        
ReflectionUtils.quietlyInvokeMethodsWithAnnotation(OnStopped.class, processor, 
processContext);
                                    }
                                    scheduledState.set(ScheduledState.STOPPED);
                                } else {
                                    scheduler.schedule(this, 100, 
TimeUnit.MILLISECONDS);
                                }
                            } catch (Exception e) {
                                LOG.warn("Failed while shutting down processor 
" + processor, e);
                            }
                        }
                    }
                });
    ```


> Cannot tell a processor is "STOPPING", can edit, cannot restart it
> ------------------------------------------------------------------
>
>                 Key: NIFI-2545
>                 URL: https://issues.apache.org/jira/browse/NIFI-2545
>             Project: Apache NiFi
>          Issue Type: Bug
>          Components: Core Framework, Core UI
>    Affects Versions: 1.0.0
>            Reporter: Joseph Witt
>            Assignee: Mark Payne
>             Fix For: 1.0.0
>
>
> Testing ListenSMTP processor.  It has some bad behavior whereby it will not 
> stop when told to.  But on the UI i cannot tell that at all.  So, i went to 
> edit the processor config which I was able to do.  Then went to start it and 
> I get
> {quote}
> 7667f0c7-0156-1000-8181-5b556f8544da cannot be started because it is not 
> stopped. Current state is STOPPING
> {quote}
> There should be a visual indicator of its status and we should not be able to 
> edit a processor which is not stopped.
> More details on that specific case here 
> https://issues.apache.org/jira/browse/NIFI-2519#
> But key is that a bad behaving processor can allow the user to do things they 
> should not be able to.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to