[GitHub] nifi pull request #900: NIFI-2605: Fixing a regression bug where nodes would...

[asfgit]

Github user asfgit closed the pull request at:

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


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #900: NIFI-2605: Fixing a regression bug where nodes would...

[olegz]

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

https://github.com/apache/nifi/pull/900#discussion_r75872798
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/leader/election/CuratorLeaderElectionManager.java
 ---
@@ -218,14 +227,92 @@ public String getLeader(final String roleName) {
 return participantId;
 }
 
+
+/**
+ * Determines whether or not leader election has already begun for the 
role with the given name
+ *
+ * @param roleName the role of interest
+ * @return true if leader election has already begun, 
false if it has not or if unable to determine this.
+ */
+@Override
+public boolean isLeaderElected(final String roleName) {
+final String leaderAddress = determineLeaderExternal(roleName);
+return !StringUtils.isEmpty(leaderAddress);
+}
+
+
+/**
+ * Use a new Curator client to determine which node is the elected 
leader for the given role.
+ *
+ * @param roleName the name of the role
+ * @return the id of the elected leader, or null if no 
leader has been selected or if unable to determine
+ * the leader from ZooKeeper
+ */
+private String determineLeaderExternal(final String roleName) {
+final CuratorFramework client = createClient();
+try {
+final LeaderSelectorListener electionListener = new 
LeaderSelectorListener() {
+@Override
+public void stateChanged(CuratorFramework client, 
ConnectionState newState) {
+}
+
+@Override
+public void takeLeadership(CuratorFramework client) throws 
Exception {
+}
+};
+
+final String electionPath = getElectionPath(roleName);
+
+// Note that we intentionally do not auto-requeue here, and we 
do not start the selector. We do not
+// want to join the leader election. We simply want to observe.
+final LeaderSelector selector = new LeaderSelector(client, 
electionPath, electionListener);
+
+try {
+final Participant leader = selector.getLeader();
+return leader == null ? null : leader.getId();
+} catch (final KeeperException.NoNodeException nne) {
+// If there is no ZNode, then there is no elected leader.
+return null;
+} catch (final Exception e) {
+logger.warn("Unable to determine the Elected Leader for 
role '{}' due to {}; assuming no leader has been elected", roleName, 
e.toString());
+if (logger.isDebugEnabled()) {
+logger.warn("", e);
+}
+
+return null;
+}
+} finally {
+client.close();
+}
+}
+
+private CuratorFramework createClient() {
+// Create a new client because we don't want to try indefinitely 
for this to occur.
+final RetryPolicy retryPolicy = new RetryNTimes(10, 500);
--- End diff --

got it


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #900: NIFI-2605: Fixing a regression bug where nodes would...

[markap14]

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

https://github.com/apache/nifi/pull/900#discussion_r75871373
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/leader/election/CuratorLeaderElectionManager.java
 ---
@@ -218,14 +227,92 @@ public String getLeader(final String roleName) {
 return participantId;
 }
 
+
+/**
+ * Determines whether or not leader election has already begun for the 
role with the given name
+ *
+ * @param roleName the role of interest
+ * @return true if leader election has already begun, 
false if it has not or if unable to determine this.
+ */
+@Override
+public boolean isLeaderElected(final String roleName) {
+final String leaderAddress = determineLeaderExternal(roleName);
+return !StringUtils.isEmpty(leaderAddress);
+}
+
+
+/**
+ * Use a new Curator client to determine which node is the elected 
leader for the given role.
+ *
+ * @param roleName the name of the role
+ * @return the id of the elected leader, or null if no 
leader has been selected or if unable to determine
+ * the leader from ZooKeeper
+ */
+private String determineLeaderExternal(final String roleName) {
+final CuratorFramework client = createClient();
+try {
+final LeaderSelectorListener electionListener = new 
LeaderSelectorListener() {
+@Override
+public void stateChanged(CuratorFramework client, 
ConnectionState newState) {
+}
+
+@Override
+public void takeLeadership(CuratorFramework client) throws 
Exception {
+}
+};
+
+final String electionPath = getElectionPath(roleName);
+
+// Note that we intentionally do not auto-requeue here, and we 
do not start the selector. We do not
+// want to join the leader election. We simply want to observe.
+final LeaderSelector selector = new LeaderSelector(client, 
electionPath, electionListener);
+
+try {
+final Participant leader = selector.getLeader();
+return leader == null ? null : leader.getId();
+} catch (final KeeperException.NoNodeException nne) {
+// If there is no ZNode, then there is no elected leader.
+return null;
+} catch (final Exception e) {
+logger.warn("Unable to determine the Elected Leader for 
role '{}' due to {}; assuming no leader has been elected", roleName, 
e.toString());
+if (logger.isDebugEnabled()) {
+logger.warn("", e);
+}
+
+return null;
+}
+} finally {
+client.close();
+}
+}
+
+private CuratorFramework createClient() {
+// Create a new client because we don't want to try indefinitely 
for this to occur.
+final RetryPolicy retryPolicy = new RetryNTimes(10, 500);
--- End diff --

After 10 retries it will thrown an Exception, which will drop into the 
catch clause above, returning null.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #900: NIFI-2605: Fixing a regression bug where nodes would...

[markap14]

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

https://github.com/apache/nifi/pull/900#discussion_r75871103
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/leader/election/CuratorLeaderElectionManager.java
 ---
@@ -218,14 +227,92 @@ public String getLeader(final String roleName) {
 return participantId;
 }
 
+
+/**
+ * Determines whether or not leader election has already begun for the 
role with the given name
+ *
+ * @param roleName the role of interest
+ * @return true if leader election has already begun, 
false if it has not or if unable to determine this.
+ */
+@Override
+public boolean isLeaderElected(final String roleName) {
+final String leaderAddress = determineLeaderExternal(roleName);
+return !StringUtils.isEmpty(leaderAddress);
+}
+
+
+/**
+ * Use a new Curator client to determine which node is the elected 
leader for the given role.
+ *
+ * @param roleName the name of the role
+ * @return the id of the elected leader, or null if no 
leader has been selected or if unable to determine
+ * the leader from ZooKeeper
+ */
+private String determineLeaderExternal(final String roleName) {
+final CuratorFramework client = createClient();
+try {
+final LeaderSelectorListener electionListener = new 
LeaderSelectorListener() {
+@Override
+public void stateChanged(CuratorFramework client, 
ConnectionState newState) {
+}
+
+@Override
+public void takeLeadership(CuratorFramework client) throws 
Exception {
+}
+};
+
+final String electionPath = getElectionPath(roleName);
+
+// Note that we intentionally do not auto-requeue here, and we 
do not start the selector. We do not
+// want to join the leader election. We simply want to observe.
+final LeaderSelector selector = new LeaderSelector(client, 
electionPath, electionListener);
+
+try {
+final Participant leader = selector.getLeader();
+return leader == null ? null : leader.getId();
+} catch (final KeeperException.NoNodeException nne) {
+// If there is no ZNode, then there is no elected leader.
+return null;
+} catch (final Exception e) {
+logger.warn("Unable to determine the Elected Leader for 
role '{}' due to {}; assuming no leader has been elected", roleName, 
e.toString());
+if (logger.isDebugEnabled()) {
+logger.warn("", e);
--- End diff --

@olegz no - this is the pattern that we follow in many places throughout 
the codebase. It prevents us from dumping tons of stack traces in the logs but 
still provides the ability to get the stack trace by turning on debug logging


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #900: NIFI-2605: Fixing a regression bug where nodes would...

[olegz]

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

https://github.com/apache/nifi/pull/900#discussion_r75858766
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/leader/election/CuratorLeaderElectionManager.java
 ---
@@ -218,14 +227,92 @@ public String getLeader(final String roleName) {
 return participantId;
 }
 
+
+/**
+ * Determines whether or not leader election has already begun for the 
role with the given name
+ *
+ * @param roleName the role of interest
+ * @return true if leader election has already begun, 
false if it has not or if unable to determine this.
+ */
+@Override
+public boolean isLeaderElected(final String roleName) {
+final String leaderAddress = determineLeaderExternal(roleName);
+return !StringUtils.isEmpty(leaderAddress);
+}
+
+
+/**
+ * Use a new Curator client to determine which node is the elected 
leader for the given role.
+ *
+ * @param roleName the name of the role
+ * @return the id of the elected leader, or null if no 
leader has been selected or if unable to determine
+ * the leader from ZooKeeper
+ */
+private String determineLeaderExternal(final String roleName) {
+final CuratorFramework client = createClient();
+try {
+final LeaderSelectorListener electionListener = new 
LeaderSelectorListener() {
+@Override
+public void stateChanged(CuratorFramework client, 
ConnectionState newState) {
+}
+
+@Override
+public void takeLeadership(CuratorFramework client) throws 
Exception {
+}
+};
+
+final String electionPath = getElectionPath(roleName);
+
+// Note that we intentionally do not auto-requeue here, and we 
do not start the selector. We do not
+// want to join the leader election. We simply want to observe.
+final LeaderSelector selector = new LeaderSelector(client, 
electionPath, electionListener);
+
+try {
+final Participant leader = selector.getLeader();
+return leader == null ? null : leader.getId();
+} catch (final KeeperException.NoNodeException nne) {
+// If there is no ZNode, then there is no elected leader.
+return null;
+} catch (final Exception e) {
+logger.warn("Unable to determine the Elected Leader for 
role '{}' due to {}; assuming no leader has been elected", roleName, 
e.toString());
+if (logger.isDebugEnabled()) {
+logger.warn("", e);
+}
+
+return null;
+}
+} finally {
+client.close();
+}
+}
+
+private CuratorFramework createClient() {
+// Create a new client because we don't want to try indefinitely 
for this to occur.
+final RetryPolicy retryPolicy = new RetryNTimes(10, 500);
--- End diff --

Wonder what will happen IF nothing is produced after 10 tries. Basically 
wondering if the above ```Participant leader = selector.getLeader();``` can 
return null.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #900: NIFI-2605: Fixing a regression bug where nodes would...

[olegz]

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

https://github.com/apache/nifi/pull/900#discussion_r75858215
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/leader/election/CuratorLeaderElectionManager.java
 ---
@@ -218,14 +227,92 @@ public String getLeader(final String roleName) {
 return participantId;
 }
 
+
+/**
+ * Determines whether or not leader election has already begun for the 
role with the given name
+ *
+ * @param roleName the role of interest
+ * @return true if leader election has already begun, 
false if it has not or if unable to determine this.
+ */
+@Override
+public boolean isLeaderElected(final String roleName) {
+final String leaderAddress = determineLeaderExternal(roleName);
+return !StringUtils.isEmpty(leaderAddress);
+}
+
+
+/**
+ * Use a new Curator client to determine which node is the elected 
leader for the given role.
+ *
+ * @param roleName the name of the role
+ * @return the id of the elected leader, or null if no 
leader has been selected or if unable to determine
+ * the leader from ZooKeeper
+ */
+private String determineLeaderExternal(final String roleName) {
+final CuratorFramework client = createClient();
+try {
+final LeaderSelectorListener electionListener = new 
LeaderSelectorListener() {
+@Override
+public void stateChanged(CuratorFramework client, 
ConnectionState newState) {
+}
+
+@Override
+public void takeLeadership(CuratorFramework client) throws 
Exception {
+}
+};
+
+final String electionPath = getElectionPath(roleName);
+
+// Note that we intentionally do not auto-requeue here, and we 
do not start the selector. We do not
+// want to join the leader election. We simply want to observe.
+final LeaderSelector selector = new LeaderSelector(client, 
electionPath, electionListener);
+
+try {
+final Participant leader = selector.getLeader();
+return leader == null ? null : leader.getId();
+} catch (final KeeperException.NoNodeException nne) {
+// If there is no ZNode, then there is no elected leader.
+return null;
+} catch (final Exception e) {
+logger.warn("Unable to determine the Elected Leader for 
role '{}' due to {}; assuming no leader has been elected", roleName, 
e.toString());
+if (logger.isDebugEnabled()) {
+logger.warn("", e);
--- End diff --

Did you mean ```logger.debug```? Feels confusing ```if debug then warn```


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #900: NIFI-2605: Fixing a regression bug where nodes would...

[markap14]

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

https://github.com/apache/nifi/pull/900#discussion_r75789301
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/test/java/org/apache/nifi/cluster/integration/Node.java
 ---
@@ -311,6 +320,11 @@ public boolean hasRole(final String roleName) {
 return leaderAddress.equals(getClusterAddress());
 }
 
+public void addProcessor(final Processor example) throws 
ProcessorInstantiationException {
--- End diff --

You're right - it shouldn't be there. I started to add some additional 
methods to improve the power of the testing by allowing it to change the flow 
so that we could easily test how the cluster behaves when the flows differ 
between nodes. But then I removed the methods because they didn't belong on 
this PR and it looks like I forgot to remove that method.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #900: NIFI-2605: Fixing a regression bug where nodes would...

[markap14]

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

https://github.com/apache/nifi/pull/900#discussion_r75788681
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/leader/election/LeaderElectionManager.java
 ---
@@ -40,7 +35,8 @@
 void register(String roleName, LeaderElectionStateChangeListener 
listener);
 
 /**
- * Adds a new role for which a leader is required, providing the given 
value for this node as the Participant ID
+ * Adds a new role for which a leader is required, providing the given 
value for this node as the Participant ID. If the Participant ID
+ * is null, this node will never be elected leader but 
will passively observe changes to the leadership.
--- End diff --

@JPercivall that means the node will never been elected leader... unless it 
later re-registers, providing a Participant ID. I.e., the node can register as 
an observer and later re-register as a participant.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #900: NIFI-2605: Fixing a regression bug where nodes would...

[JPercivall]

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

https://github.com/apache/nifi/pull/900#discussion_r75781210
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/leader/election/LeaderElectionManager.java
 ---
@@ -40,7 +35,8 @@
 void register(String roleName, LeaderElectionStateChangeListener 
listener);
 
 /**
- * Adds a new role for which a leader is required, providing the given 
value for this node as the Participant ID
+ * Adds a new role for which a leader is required, providing the given 
value for this node as the Participant ID. If the Participant ID
+ * is null, this node will never be elected leader but 
will passively observe changes to the leadership.
--- End diff --

Does this mean that in the future this node could never become the leader?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #900: NIFI-2605: Fixing a regression bug where nodes would...

[JPercivall]

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

https://github.com/apache/nifi/pull/900#discussion_r75701992
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/test/java/org/apache/nifi/cluster/integration/Node.java
 ---
@@ -311,6 +320,11 @@ public boolean hasRole(final String roleName) {
 return leaderAddress.equals(getClusterAddress());
 }
 
+public void addProcessor(final Processor example) throws 
ProcessorInstantiationException {
--- End diff --

Why is there a "addProcessor" method being added Node.java? There aren't 
any other "addX" methods and it seems weird to be "adding" it to a node.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #900: NIFI-2605: Fixing a regression bug where nodes would...

[JPercivall]

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

https://github.com/apache/nifi/pull/900#discussion_r75702141
  
--- Diff: 
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-cluster/src/test/java/org/apache/nifi/cluster/integration/Node.java
 ---
@@ -311,6 +320,11 @@ public boolean hasRole(final String roleName) {
 return leaderAddress.equals(getClusterAddress());
 }
 
+public void addProcessor(final Processor example) throws 
ProcessorInstantiationException {
--- End diff --

Intellij also says it's not used


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] nifi pull request #900: NIFI-2605: Fixing a regression bug where nodes would...

[markap14]

GitHub user markap14 opened a pull request:

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

NIFI-2605: Fixing a regression bug where nodes would potentially be e…

…lected leader for Cluster Coordinator role when they do not have the 
correct flow

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

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

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

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


commit d05fa581595aff6d1529da2716cada114ad9
Author: Mark Payne 
Date:   2016-08-19T17:41:58Z

NIFI-2605: Fixing a regression bug where nodes would potentially be elected 
leader for Cluster Coordinator role when they do not have the correct flow




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---