You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by markap14 <gi...@git.apache.org> on 2016/07/28 18:11:52 UTC

[GitHub] nifi pull request #739: NIFI-2419: Ensure that if a node is disconnected tha...

GitHub user markap14 opened a pull request:

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

    NIFI-2419: Ensure that if a node is disconnected that we unregister f\u2026

    \u2026or 'cluster coordinator' and 'primary node' roles by updating FlowController to know that it is disconnected. Also removed dead code that was needed in the master-worker clustering paradigm but not for zero-master-clustering

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

    $ git pull https://github.com/markap14/nifi NIFI-2419

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

    https://github.com/apache/nifi/pull/739.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 #739
    
----

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi issue #739: NIFI-2419: Ensure that if a node is disconnected that we un...

Posted by YolandaMDavis <gi...@git.apache.org>.
Github user YolandaMDavis commented on the issue:

    https://github.com/apache/nifi/pull/739
  
    @markap14 was able to test with 2 node cluster removing one node from cluster, changing it's flow by adding a new processor, and attempting to add it back to the cluster.  Did confirm through debug and log review that upon the UninheritableFlowException node's clustered state was set to false and node was unregistered from both Primary Node and Cluster Coordinator roles in leader election.  Updating flow to match original flow did allow the node to rejoin the cluster without issue.
    
    +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi issue #739: NIFI-2419: Ensure that if a node is disconnected that we un...

Posted by YolandaMDavis <gi...@git.apache.org>.
Github user YolandaMDavis commented on the issue:

    https://github.com/apache/nifi/pull/739
  
    @markap14 started review however can you rebase against the latest in master? 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #739: NIFI-2419: Ensure that if a node is disconnected tha...

Posted by olegz <gi...@git.apache.org>.
Github user olegz commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/739#discussion_r72802000
  
    --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/FlowController.java ---
    @@ -3279,14 +3256,14 @@ public synchronized void onLeaderElection() {
             });
         }
     
    +
         /**
          * Sets whether this instance is clustered. Clustered means that a node is either connected or trying to connect to the cluster.
          *
          * @param clustered true if clustered
          * @param clusterInstanceId if clustered is true, indicates the InstanceID of the Cluster Manager
    -     * @param clusterManagerDn the DN of the NCM
          */
    -    public void setClustered(final boolean clustered, final String clusterInstanceId, final String clusterManagerDn) {
    +    public void setClustered(final boolean clustered, final String clusterInstanceId) {
    --- End diff --
    
    Given that this is non-public API and is only used by StandardFlowService do you mind if we make it package private? Don't need to actually do it as I can do it during merge/squash. Just want your OK.
    
    Basically since we alredy started conversation around annotation private APIs, annotation is the last resort for cases where native Java compile constructs can't protect. And I see this as a case. We can always go back to public if there is justification in the future.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #739: NIFI-2419: Ensure that if a node is disconnected tha...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #739: NIFI-2419: Ensure that if a node is disconnected tha...

Posted by markap14 <gi...@git.apache.org>.
Github user markap14 commented on a diff in the pull request:

    https://github.com/apache/nifi/pull/739#discussion_r72810212
  
    --- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/FlowController.java ---
    @@ -3279,14 +3256,14 @@ public synchronized void onLeaderElection() {
             });
         }
     
    +
         /**
          * Sets whether this instance is clustered. Clustered means that a node is either connected or trying to connect to the cluster.
          *
          * @param clustered true if clustered
          * @param clusterInstanceId if clustered is true, indicates the InstanceID of the Cluster Manager
    -     * @param clusterManagerDn the DN of the NCM
          */
    -    public void setClustered(final boolean clustered, final String clusterInstanceId, final String clusterManagerDn) {
    +    public void setClustered(final boolean clustered, final String clusterInstanceId) {
    --- End diff --
    
    @olegz - I think it is accurate that this is called only from other classes within the same package (I believe only StandardFlowService, in fact). And if that is the case then yes, I have no problem with making it package-private. Feel free to do so on merge.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi issue #739: NIFI-2419: Ensure that if a node is disconnected that we un...

Posted by markap14 <gi...@git.apache.org>.
Github user markap14 commented on the issue:

    https://github.com/apache/nifi/pull/739
  
    @YolandaMDavis thanks - I have rebased against master.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---