From ce7633cd010d3db36806a2311096c9fb7b46223a Mon Sep 17 00:00:00 2001 From: Mark Payne Date: Tue, 6 Nov 2018 16:20:57 -0500 Subject: [PATCH 1/2] NIFI-5796: Addressed issue that caused Counters' values to show the wrong value in Status History --- .../status/history/MetricDescriptor.java | 5 + .../history/AbstractMetricDescriptor.java | 98 +++++++++++++++++++ .../history/CounterMetricDescriptor.java | 63 ++++++++++++ .../history/StandardMetricDescriptor.java | 74 ++------------ .../history/StandardStatusSnapshot.java | 6 +- .../apache/nifi/util/ComponentMetrics.java | 4 +- 6 files changed, 181 insertions(+), 69 deletions(-) create mode 100644 nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/status/history/AbstractMetricDescriptor.java create mode 100644 nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/status/history/CounterMetricDescriptor.java diff --git a/nifi-framework-api/src/main/java/org/apache/nifi/controller/status/history/MetricDescriptor.java b/nifi-framework-api/src/main/java/org/apache/nifi/controller/status/history/MetricDescriptor.java index c0c52b6af765..14887ef2f363 100644 --- a/nifi-framework-api/src/main/java/org/apache/nifi/controller/status/history/MetricDescriptor.java +++ b/nifi-framework-api/src/main/java/org/apache/nifi/controller/status/history/MetricDescriptor.java @@ -64,4 +64,9 @@ enum Formatter { * into a single Long value */ ValueReducer getValueReducer(); + + /** + * @return true if the metric is for a component Counter, false otherwise + */ + boolean isCounter(); } diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/status/history/AbstractMetricDescriptor.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/status/history/AbstractMetricDescriptor.java new file mode 100644 index 000000000000..980efdda918a --- /dev/null +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/status/history/AbstractMetricDescriptor.java @@ -0,0 +1,98 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.controller.status.history; + +import java.util.List; + +public abstract class AbstractMetricDescriptor implements MetricDescriptor { + private final IndexableMetric indexableMetric; + private final String field; + private final String label; + private final String description; + private final MetricDescriptor.Formatter formatter; + private final ValueMapper valueMapper; + private final ValueReducer reducer; + + public AbstractMetricDescriptor(final IndexableMetric indexableMetric, final String field, final String label, final String description, + final MetricDescriptor.Formatter formatter, final ValueMapper valueFunction) { + this(indexableMetric, field, label, description, formatter, valueFunction, null); + } + + public AbstractMetricDescriptor(final IndexableMetric indexableMetric, final String field, final String label, final String description, + final MetricDescriptor.Formatter formatter, final ValueMapper valueFunction, final ValueReducer reducer) { + this.indexableMetric = indexableMetric; + this.field = field; + this.label = label; + this.description = description; + this.formatter = formatter; + this.valueMapper = valueFunction; + this.reducer = reducer == null ? new SumReducer() : reducer; + } + + @Override + public int getMetricIdentifier() { + return indexableMetric.getIndex(); + } + + @Override + public String getField() { + return field; + } + + @Override + public boolean isCounter() { + return false; + } + + @Override + public String getLabel() { + return label; + } + + @Override + public String getDescription() { + return description; + } + + @Override + public MetricDescriptor.Formatter getFormatter() { + return formatter; + } + + @Override + public ValueMapper getValueFunction() { + return valueMapper; + } + + @Override + public ValueReducer getValueReducer() { + return reducer; + } + + class SumReducer implements ValueReducer { + + @Override + public Long reduce(final List values) { + long sum = 0; + for (final StatusSnapshot snapshot : values) { + sum += snapshot.getStatusMetric(AbstractMetricDescriptor.this); + } + + return sum; + } + } +} diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/status/history/CounterMetricDescriptor.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/status/history/CounterMetricDescriptor.java new file mode 100644 index 000000000000..5aa7a72a355e --- /dev/null +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/status/history/CounterMetricDescriptor.java @@ -0,0 +1,63 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.nifi.controller.status.history; + +public class CounterMetricDescriptor extends AbstractMetricDescriptor { + + private static final IndexableMetric ILLEGAL_INDEXABLE_METRIC = () -> { + throw new UnsupportedOperationException(); + }; + + public CounterMetricDescriptor(final String field, final String label, final String description, + final MetricDescriptor.Formatter formatter, final ValueMapper valueFunction) { + super(ILLEGAL_INDEXABLE_METRIC, field, label, description, formatter, valueFunction, null); + } + + public CounterMetricDescriptor(final String field, final String label, final String description, + final MetricDescriptor.Formatter formatter, final ValueMapper valueFunction, final ValueReducer reducer) { + super(ILLEGAL_INDEXABLE_METRIC, field, label, description, formatter, valueFunction, reducer); + } + + @Override + public boolean isCounter() { + return true; + } + + @Override + public String toString() { + return "StandardMetricDescriptor[" + getLabel() + "]"; + } + + @Override + public int hashCode() { + return 239891 + getFormatter().name().hashCode() + 4 * getLabel().hashCode() + 8 * getField().hashCode() + 28 * getDescription().hashCode(); + } + + @Override + public boolean equals(final Object obj) { + if (this == obj) { + return true; + } + if (!(obj instanceof CounterMetricDescriptor)) { + return false; + } + + MetricDescriptor other = (MetricDescriptor) obj; + return getField().equals(other.getField()); + } + +} diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/status/history/StandardMetricDescriptor.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/status/history/StandardMetricDescriptor.java index de5e0b55e19c..1b6f65f9ec41 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/status/history/StandardMetricDescriptor.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/status/history/StandardMetricDescriptor.java @@ -16,77 +16,31 @@ */ package org.apache.nifi.controller.status.history; -import java.util.List; - -public class StandardMetricDescriptor implements MetricDescriptor { - - private final IndexableMetric indexableMetric; - private final String field; - private final String label; - private final String description; - private final MetricDescriptor.Formatter formatter; - private final ValueMapper valueMapper; - private final ValueReducer reducer; +public class StandardMetricDescriptor extends AbstractMetricDescriptor { public StandardMetricDescriptor(final IndexableMetric indexableMetric, final String field, final String label, final String description, final MetricDescriptor.Formatter formatter, final ValueMapper valueFunction) { - this(indexableMetric, field, label, description, formatter, valueFunction, null); + super(indexableMetric, field, label, description, formatter, valueFunction); } public StandardMetricDescriptor(final IndexableMetric indexableMetric, final String field, final String label, final String description, final MetricDescriptor.Formatter formatter, final ValueMapper valueFunction, final ValueReducer reducer) { - this.indexableMetric = indexableMetric; - this.field = field; - this.label = label; - this.description = description; - this.formatter = formatter; - this.valueMapper = valueFunction; - this.reducer = reducer == null ? new SumReducer() : reducer; - } - - @Override - public int getMetricIdentifier() { - return indexableMetric.getIndex(); - } - - @Override - public String getField() { - return field; - } - - @Override - public String getLabel() { - return label; - } - - @Override - public String getDescription() { - return description; - } - - @Override - public MetricDescriptor.Formatter getFormatter() { - return formatter; - } - - @Override - public ValueMapper getValueFunction() { - return valueMapper; + super(indexableMetric, field, label, description, formatter, valueFunction, reducer); } @Override - public ValueReducer getValueReducer() { - return reducer; + public boolean isCounter() { + return false; } @Override public String toString() { - return "StandardMetricDescriptor[" + label + "]"; + return "StandardMetricDescriptor[" + getLabel() + "]"; } @Override public int hashCode() { - return 23987 + formatter.name().hashCode() + 4 * label.hashCode() + 8 * field.hashCode() + 28 * description.hashCode(); + return 23987 + getFormatter().name().hashCode() + 4 * getLabel().hashCode() + 8 * getField().hashCode() + 28 * getDescription().hashCode(); } @Override @@ -99,19 +53,7 @@ public boolean equals(final Object obj) { } MetricDescriptor other = (MetricDescriptor) obj; - return field.equals(other.getField()); + return getField().equals(other.getField()); } - class SumReducer implements ValueReducer { - - @Override - public Long reduce(final List values) { - long sum = 0; - for (final StatusSnapshot snapshot : values) { - sum += snapshot.getStatusMetric(StandardMetricDescriptor.this); - } - - return sum; - } - } } diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/status/history/StandardStatusSnapshot.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/status/history/StandardStatusSnapshot.java index fb9112b55caf..8e78c5ea01e9 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/status/history/StandardStatusSnapshot.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/status/history/StandardStatusSnapshot.java @@ -65,7 +65,11 @@ public Set> getMetricDescriptors() { @Override public Long getStatusMetric(final MetricDescriptor descriptor) { - return values[descriptor.getMetricIdentifier()]; + if (descriptor.isCounter()) { + return counterValues.get(descriptor); + } else { + return values[descriptor.getMetricIdentifier()]; + } } public void setTimestamp(final Date timestamp) { diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/util/ComponentMetrics.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/util/ComponentMetrics.java index 9c1505cd002b..7fa2278da5e1 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/util/ComponentMetrics.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/util/ComponentMetrics.java @@ -21,11 +21,11 @@ import org.apache.nifi.controller.status.ProcessorStatus; import org.apache.nifi.controller.status.RemoteProcessGroupStatus; import org.apache.nifi.controller.status.history.ConnectionStatusDescriptor; +import org.apache.nifi.controller.status.history.CounterMetricDescriptor; import org.apache.nifi.controller.status.history.MetricDescriptor; import org.apache.nifi.controller.status.history.ProcessGroupStatusDescriptor; import org.apache.nifi.controller.status.history.ProcessorStatusDescriptor; import org.apache.nifi.controller.status.history.RemoteProcessGroupStatusDescriptor; -import org.apache.nifi.controller.status.history.StandardMetricDescriptor; import org.apache.nifi.controller.status.history.StandardStatusSnapshot; import org.apache.nifi.controller.status.history.StatusSnapshot; @@ -84,7 +84,7 @@ public static StatusSnapshot createSnapshot(final ProcessorStatus status, final final String counterName = entry.getKey(); final String label = entry.getKey() + " (5 mins)"; - final MetricDescriptor metricDescriptor = new StandardMetricDescriptor<>(() -> 0, entry.getKey(), label, label, MetricDescriptor.Formatter.COUNT, + final MetricDescriptor metricDescriptor = new CounterMetricDescriptor<>(entry.getKey(), label, label, MetricDescriptor.Formatter.COUNT, s -> s.getCounters() == null ? null : s.getCounters().get(counterName)); snapshot.addCounterStatusMetric(metricDescriptor, entry.getValue()); From 14151652686792cf6f00b8a076742b67f8a1e082 Mon Sep 17 00:00:00 2001 From: Mark Payne Date: Tue, 6 Nov 2018 17:38:46 -0500 Subject: [PATCH 2/2] Addressed bug in subtract() method for keeping running total of counters for status history --- .../nifi/controller/repository/metrics/EventSumValue.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/repository/metrics/EventSumValue.java b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/repository/metrics/EventSumValue.java index 210f7ace8288..a618812bf2f4 100644 --- a/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/repository/metrics/EventSumValue.java +++ b/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-core/src/main/java/org/apache/nifi/controller/repository/metrics/EventSumValue.java @@ -180,7 +180,7 @@ public synchronized void subtract(final EventSumValue other) { final String counterName = entry.getKey(); final Long counterValue = entry.getValue(); - counters.compute(counterName, (key, value) -> value == null ? counterValue : counterValue - value); + counters.compute(counterName, (key, value) -> value == null ? -counterValue : value - counterValue); } } }