[GitHub] nifi pull request #328: NIFI-1690 Changed MonitorMemory to use allowable val...
Github user asfgit closed the pull request at: https://github.com/apache/nifi/pull/328 --- 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 #328: NIFI-1690 Changed MonitorMemory to use allowable val...
Github user alopresto commented on a diff in the pull request:
https://github.com/apache/nifi/pull/328#discussion_r67210183
--- Diff:
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-reporting-tasks/src/main/java/org/apache/nifi/controller/MonitorMemory.java
---
@@ -154,30 +164,21 @@ public void onConfigured(final ConfigurationContext
config) throws Initializatio
}
final List memoryPoolBeans =
ManagementFactory.getMemoryPoolMXBeans();
-for (final MemoryPoolMXBean bean : memoryPoolBeans) {
-final String memoryPoolName = bean.getName();
+for (int i = 0; i < memoryPoolBeans.size() && monitoredBean ==
null; i++) {
+MemoryPoolMXBean memoryPoolBean = memoryPoolBeans.get(i);
+String memoryPoolName = memoryPoolBean.getName();
if (desiredMemoryPoolName.equals(memoryPoolName)) {
-monitoredBean = bean;
-if (DATA_SIZE_PATTERN.matcher(thresholdValue).matches()) {
-final long bytes =
DataUnit.parseDataSize(thresholdValue, DataUnit.B).longValue();
--- End diff --
Sorry, meant to put it on line 178 below. Same code is still present.
---
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 #328: NIFI-1690 Changed MonitorMemory to use allowable val...
Github user olegz commented on a diff in the pull request:
https://github.com/apache/nifi/pull/328#discussion_r67150175
--- Diff:
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-reporting-tasks/src/main/java/org/apache/nifi/controller/MonitorMemory.java
---
@@ -154,30 +164,21 @@ public void onConfigured(final ConfigurationContext
config) throws Initializatio
}
final List memoryPoolBeans =
ManagementFactory.getMemoryPoolMXBeans();
-for (final MemoryPoolMXBean bean : memoryPoolBeans) {
-final String memoryPoolName = bean.getName();
+for (int i = 0; i < memoryPoolBeans.size() && monitoredBean ==
null; i++) {
+MemoryPoolMXBean memoryPoolBean = memoryPoolBeans.get(i);
+String memoryPoolName = memoryPoolBean.getName();
if (desiredMemoryPoolName.equals(memoryPoolName)) {
-monitoredBean = bean;
-if (DATA_SIZE_PATTERN.matcher(thresholdValue).matches()) {
-final long bytes =
DataUnit.parseDataSize(thresholdValue, DataUnit.B).longValue();
-if (bean.isCollectionUsageThresholdSupported()) {
-bean.setCollectionUsageThreshold(bytes);
-}
-} else {
-final String percentage = thresholdValue.substring(0,
thresholdValue.length() - 1);
-final double pct = Double.parseDouble(percentage) /
100D;
-final long calculatedThreshold = (long)
(bean.getUsage().getMax() * pct);
-if (bean.isCollectionUsageThresholdSupported()) {
-
bean.setCollectionUsageThreshold(calculatedThreshold);
+monitoredBean = memoryPoolBean;
+if (memoryPoolBean.isCollectionUsageThresholdSupported()) {
+long calculatedThreshold;
+if
(DATA_SIZE_PATTERN.matcher(thresholdValue).matches()) {
+calculatedThreshold =
DataUnit.parseDataSize(thresholdValue, DataUnit.B).longValue();
+} else {
+final String percentage =
thresholdValue.substring(0, thresholdValue.length() - 1);
+final double pct = Double.parseDouble(percentage)
/ 100D;
--- End diff --
Yes, it uses custom _ThresholdValidator_ (line 236) where those checks are
performed
---
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 #328: NIFI-1690 Changed MonitorMemory to use allowable val...
Github user olegz commented on a diff in the pull request:
https://github.com/apache/nifi/pull/328#discussion_r67149972
--- Diff:
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-reporting-tasks/src/main/java/org/apache/nifi/controller/MonitorMemory.java
---
@@ -154,30 +164,21 @@ public void onConfigured(final ConfigurationContext
config) throws Initializatio
}
final List memoryPoolBeans =
ManagementFactory.getMemoryPoolMXBeans();
-for (final MemoryPoolMXBean bean : memoryPoolBeans) {
-final String memoryPoolName = bean.getName();
+for (int i = 0; i < memoryPoolBeans.size() && monitoredBean ==
null; i++) {
+MemoryPoolMXBean memoryPoolBean = memoryPoolBeans.get(i);
+String memoryPoolName = memoryPoolBean.getName();
if (desiredMemoryPoolName.equals(memoryPoolName)) {
-monitoredBean = bean;
-if (DATA_SIZE_PATTERN.matcher(thresholdValue).matches()) {
-final long bytes =
DataUnit.parseDataSize(thresholdValue, DataUnit.B).longValue();
--- End diff --
Andy, just realized, your comment is on the removed code, so no longer
valid, correct?
---
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 #328: NIFI-1690 Changed MonitorMemory to use allowable val...
Github user olegz commented on a diff in the pull request:
https://github.com/apache/nifi/pull/328#discussion_r67149454
--- Diff:
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-reporting-tasks/src/main/java/org/apache/nifi/controller/MonitorMemory.java
---
@@ -91,12 +92,21 @@
+ " that the memory pool is exceeding this threshold.")
public class MonitorMemory extends AbstractReportingTask {
+private static final AllowableValue[] memPoolAllowableValues;
+
+static {
+List memoryPoolBeans =
ManagementFactory.getMemoryPoolMXBeans();
+memPoolAllowableValues = new
AllowableValue[memoryPoolBeans.size()];
+for (int i = 0; i < memPoolAllowableValues.length; i++) {
+memPoolAllowableValues[i] = new
AllowableValue(memoryPoolBeans.get(i).getName());
+}
+}
+
public static final PropertyDescriptor MEMORY_POOL_PROPERTY = new
PropertyDescriptor.Builder()
.name("Memory Pool")
.description("The name of the JVM Memory Pool to monitor")
--- End diff --
One thing you and I always agreed on, so yeah, why not?
---
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 #328: NIFI-1690 Changed MonitorMemory to use allowable val...
Github user olegz commented on a diff in the pull request:
https://github.com/apache/nifi/pull/328#discussion_r67149312
--- Diff:
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-reporting-tasks/src/main/java/org/apache/nifi/controller/MonitorMemory.java
---
@@ -154,30 +164,21 @@ public void onConfigured(final ConfigurationContext
config) throws Initializatio
}
final List memoryPoolBeans =
ManagementFactory.getMemoryPoolMXBeans();
-for (final MemoryPoolMXBean bean : memoryPoolBeans) {
-final String memoryPoolName = bean.getName();
+for (int i = 0; i < memoryPoolBeans.size() && monitoredBean ==
null; i++) {
+MemoryPoolMXBean memoryPoolBean = memoryPoolBeans.get(i);
+String memoryPoolName = memoryPoolBean.getName();
if (desiredMemoryPoolName.equals(memoryPoolName)) {
-monitoredBean = bean;
-if (DATA_SIZE_PATTERN.matcher(thresholdValue).matches()) {
-final long bytes =
DataUnit.parseDataSize(thresholdValue, DataUnit.B).longValue();
--- End diff --
Apparently yes, since these are old code bits as well, but looking
---
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 #328: NIFI-1690 Changed MonitorMemory to use allowable val...
Github user olegz commented on a diff in the pull request:
https://github.com/apache/nifi/pull/328#discussion_r67149210
--- Diff:
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-reporting-tasks/src/main/java/org/apache/nifi/controller/MonitorMemory.java
---
@@ -154,30 +164,21 @@ public void onConfigured(final ConfigurationContext
config) throws Initializatio
}
final List memoryPoolBeans =
ManagementFactory.getMemoryPoolMXBeans();
-for (final MemoryPoolMXBean bean : memoryPoolBeans) {
-final String memoryPoolName = bean.getName();
+for (int i = 0; i < memoryPoolBeans.size() && monitoredBean ==
null; i++) {
+MemoryPoolMXBean memoryPoolBean = memoryPoolBeans.get(i);
+String memoryPoolName = memoryPoolBean.getName();
if (desiredMemoryPoolName.equals(memoryPoolName)) {
-monitoredBean = bean;
-if (DATA_SIZE_PATTERN.matcher(thresholdValue).matches()) {
-final long bytes =
DataUnit.parseDataSize(thresholdValue, DataUnit.B).longValue();
-if (bean.isCollectionUsageThresholdSupported()) {
-bean.setCollectionUsageThreshold(bytes);
-}
-} else {
-final String percentage = thresholdValue.substring(0,
thresholdValue.length() - 1);
-final double pct = Double.parseDouble(percentage) /
100D;
-final long calculatedThreshold = (long)
(bean.getUsage().getMax() * pct);
-if (bean.isCollectionUsageThresholdSupported()) {
-
bean.setCollectionUsageThreshold(calculatedThreshold);
+monitoredBean = memoryPoolBean;
+if (memoryPoolBean.isCollectionUsageThresholdSupported()) {
+long calculatedThreshold;
+if
(DATA_SIZE_PATTERN.matcher(thresholdValue).matches()) {
+calculatedThreshold =
DataUnit.parseDataSize(thresholdValue, DataUnit.B).longValue();
+} else {
+final String percentage =
thresholdValue.substring(0, thresholdValue.length() - 1);
+final double pct = Double.parseDouble(percentage)
/ 100D;
--- End diff --
As you can see those are old code bits (just moved around slightly), but
let me see, since your point is still valid.
---
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 #328: NIFI-1690 Changed MonitorMemory to use allowable val...
Github user alopresto commented on a diff in the pull request:
https://github.com/apache/nifi/pull/328#discussion_r67093804
--- Diff:
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-reporting-tasks/src/main/java/org/apache/nifi/controller/MonitorMemory.java
---
@@ -154,30 +164,21 @@ public void onConfigured(final ConfigurationContext
config) throws Initializatio
}
final List memoryPoolBeans =
ManagementFactory.getMemoryPoolMXBeans();
-for (final MemoryPoolMXBean bean : memoryPoolBeans) {
-final String memoryPoolName = bean.getName();
+for (int i = 0; i < memoryPoolBeans.size() && monitoredBean ==
null; i++) {
+MemoryPoolMXBean memoryPoolBean = memoryPoolBeans.get(i);
+String memoryPoolName = memoryPoolBean.getName();
if (desiredMemoryPoolName.equals(memoryPoolName)) {
-monitoredBean = bean;
-if (DATA_SIZE_PATTERN.matcher(thresholdValue).matches()) {
-final long bytes =
DataUnit.parseDataSize(thresholdValue, DataUnit.B).longValue();
-if (bean.isCollectionUsageThresholdSupported()) {
-bean.setCollectionUsageThreshold(bytes);
-}
-} else {
-final String percentage = thresholdValue.substring(0,
thresholdValue.length() - 1);
-final double pct = Double.parseDouble(percentage) /
100D;
-final long calculatedThreshold = (long)
(bean.getUsage().getMax() * pct);
-if (bean.isCollectionUsageThresholdSupported()) {
-
bean.setCollectionUsageThreshold(calculatedThreshold);
+monitoredBean = memoryPoolBean;
+if (memoryPoolBean.isCollectionUsageThresholdSupported()) {
+long calculatedThreshold;
+if
(DATA_SIZE_PATTERN.matcher(thresholdValue).matches()) {
+calculatedThreshold =
DataUnit.parseDataSize(thresholdValue, DataUnit.B).longValue();
+} else {
+final String percentage =
thresholdValue.substring(0, thresholdValue.length() - 1);
+final double pct = Double.parseDouble(percentage)
/ 100D;
--- End diff --
Can this ever throw a `NumberFormatException` or an
`IndexOutOfBoundsException`? It looks like it's reading from a string that may
be empty or have a unit abbreviation that is more than one character.
---
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 #328: NIFI-1690 Changed MonitorMemory to use allowable val...
Github user alopresto commented on a diff in the pull request:
https://github.com/apache/nifi/pull/328#discussion_r67079817
--- Diff:
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-reporting-tasks/src/main/java/org/apache/nifi/controller/MonitorMemory.java
---
@@ -91,12 +92,21 @@
+ " that the memory pool is exceeding this threshold.")
public class MonitorMemory extends AbstractReportingTask {
+private static final AllowableValue[] memPoolAllowableValues;
+
+static {
+List memoryPoolBeans =
ManagementFactory.getMemoryPoolMXBeans();
+memPoolAllowableValues = new
AllowableValue[memoryPoolBeans.size()];
+for (int i = 0; i < memPoolAllowableValues.length; i++) {
+memPoolAllowableValues[i] = new
AllowableValue(memoryPoolBeans.get(i).getName());
+}
+}
+
public static final PropertyDescriptor MEMORY_POOL_PROPERTY = new
PropertyDescriptor.Builder()
.name("Memory Pool")
.description("The name of the JVM Memory Pool to monitor")
--- End diff --
Is this a good time to add a `displayName` attribute for future-proofing?
---
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 #328: NIFI-1690 Changed MonitorMemory to use allowable val...
Github user alopresto commented on a diff in the pull request:
https://github.com/apache/nifi/pull/328#discussion_r67076846
--- Diff:
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-reporting-tasks/pom.xml ---
@@ -1,60 +1,69 @@
-http://maven.apache.org/POM/4.0.0;
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0
http://maven.apache.org/xsd/maven-4.0.0.xsd;>
-
-4.0.0
-
-org.apache.nifi
-nifi-standard-bundle
-1.0.0-SNAPSHOT
-
-nifi-standard-reporting-tasks
-jar
-
-
-org.apache.nifi
-nifi-api
-
-
-org.apache.nifi
-nifi-utils
-
-
-org.apache.nifi
-nifi-properties
-
-
-org.apache.nifi
-nifi-processor-utils
-
-
-com.yammer.metrics
-metrics-ganglia
-
-
-org.slf4j
-slf4j-api
-
-
-org.apache.nifi
-nifi-mock
-test
-
-
-org.mockito
-mockito-all
-test
-
-
+http://maven.apache.org/POM/4.0.0;
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance;
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0
http://maven.apache.org/xsd/maven-4.0.0.xsd;>
+
+ 4.0.0
+
+ org.apache.nifi
+ nifi-standard-bundle
+ 1.0.0-SNAPSHOT
+
+ nifi-standard-reporting-tasks
+ jar
+
+
+ org.apache.nifi
+ nifi-framework-core
+ ${project.version}
+ test
+
+
+ org.apache.nifi
+ nifi-nar-utils
+ test
+
+
+ org.apache.nifi
+ nifi-api
+
+
+ org.apache.nifi
+ nifi-utils
+
+
+ org.apache.nifi
+ nifi-properties
+
+
+ org.apache.nifi
+ nifi-processor-utils
+
+
+ com.yammer.metrics
+ metrics-ganglia
+
+
+ org.slf4j
+ slf4j-api
+
+
+ org.apache.nifi
+ nifi-mock
+ test
+
+
+ org.mockito
+ mockito-all
+ test
+
+
+
--- End diff --
Looks like there are some formatting changes here that is throwing the diff
out of whack. I saw that there was only one significant difference -- the
following block was added:
```
org.apache.nifi
nifi-framework-core
${project.version}
test
org.apache.nifi
nifi-nar-utils
test
```
Can you revert and just insert these lines?
---
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.
---
