Skip to content

Commit

Permalink
dcache-history: avoid NaN stack trace in histogram conversion
Browse files Browse the repository at this point in the history
Motivation:

See GH #6978 NaN error in history service
#6978

The stack trace comes when the Gson writer
attempts to serialize a double NaN.

Modification:

While it has been difficult to reproduce
the conditions giving rise to this error,
there are a number of possible places where
NaNs can occur in the histogram pipeline.
These all involve division where either numerator
or denominator are a floating point value and they
both are zero.  (a NaN can also occur from the
modulo operation if done using a floating point zero
value, but there are no instances of % + fp that
I could find).

To prevent this, those places are wrapped in
a utility method that converts the
NaN to a zero.

As extra prevention, we do the same when
adding a double value to a timeseries
histogram.

Result:

The code should not generate the reported exception.

Target: master
Request: 8.2
Request: 8.1
Request: 8.0
Request: 7.2
Requires-notes: yes
Patch: https://rb.dcache.org/r/13867/
Closes: #6978
Acked-by: Dmitry
  • Loading branch information
alrossi authored and lemora committed Feb 2, 2023
1 parent dcfcc59 commit 0f1d2d9
Show file tree
Hide file tree
Showing 5 changed files with 28 additions and 12 deletions.
12 changes: 12 additions & 0 deletions modules/common/src/main/java/org/dcache/util/MathUtils.java
Expand Up @@ -72,4 +72,16 @@ public static long subWithInfinity(long a, long b) {

return (a < 0) ? Long.MIN_VALUE : Long.MAX_VALUE;
}

/**
* For applications where this is appropriate (like a Histogram value),
* convert NaN to 0.0.
*
* @param value the double result of the computation.
* @return 0.0 if the value is NaN; otherwise, the value.
*/
public static Double nanToZero(Double value) {
return Double.isNaN(value) ? 0.0 : value;
}

}
Expand Up @@ -60,6 +60,7 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
package org.dcache.util.histograms;

import static java.util.Objects.requireNonNull;
import static org.dcache.util.MathUtils.nanToZero;

import com.google.common.base.Preconditions;
import java.util.ArrayList;
Expand Down Expand Up @@ -126,8 +127,8 @@ public void configure() {
*/
Double min = FastMath.min(0.0D, metadata.getMinValue().orElse(0.0D));

double maxIndex = max == null ? binCount - 1 : FastMath.floor((max - min) / binSize);
binWidth = (int) FastMath.ceil(maxIndex / (binCount - 1));
double maxIndex = max == null ? binCount - 1 : FastMath.floor(nanToZero((max - min) / binSize));
binWidth = (int) FastMath.ceil(nanToZero(maxIndex / (binCount - 1)));

/**
* Re-adjust size on the basis of the adjusted width.
Expand All @@ -145,7 +146,7 @@ public void configure() {
binSize, binCount, min, max, maxIndex);

for (Double d : data) {
++histogram[(int) FastMath.floor((d - min) / binSize)];
++histogram[(int) FastMath.floor(nanToZero((d - min) / binSize))];
}

data = new ArrayList<>();
Expand All @@ -158,7 +159,7 @@ public void configure() {
setHighestFromLowest();
int computedCount = (int) binSize == 0 ?
binCount :
(int) FastMath.ceil((highestBin - lowestBin) / binSize) + 1;
(int) FastMath.ceil(nanToZero((highestBin - lowestBin) / binSize)) + 1;

if (computedCount > binCount) {
throw new RuntimeException("bin count " + binCount
Expand Down
Expand Up @@ -60,6 +60,7 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
package org.dcache.util.histograms;

import static java.util.Objects.requireNonNull;
import static org.dcache.util.MathUtils.nanToZero;

import com.google.common.annotations.VisibleForTesting;
import java.io.Serializable;
Expand Down Expand Up @@ -251,8 +252,8 @@ public double standardDeviation() {
return 0.0;
}

double variance = (sumOfSquares / count)
- FastMath.pow(sum / count, 2);
double variance = nanToZero((sumOfSquares / count)
- FastMath.pow(sum / count, 2));

return FastMath.sqrt(variance);
}
Expand Down
Expand Up @@ -60,6 +60,7 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
package org.dcache.util.histograms;

import static java.util.Objects.requireNonNull;
import static org.dcache.util.MathUtils.nanToZero;

import com.google.common.base.Preconditions;
import java.util.ArrayList;
Expand Down Expand Up @@ -88,12 +89,12 @@ public TimeseriesHistogram(TimeseriesHistogram copy) {

@Override
public void add(Double value, Long timestamp) {
update(value, UpdateOperation.SUM, timestamp);
update(nanToZero(value), UpdateOperation.SUM, timestamp);
}

@Override
public void average(Double value, Long timestamp) {
update(value, UpdateOperation.AVERAGE, timestamp);
update(nanToZero(value), UpdateOperation.AVERAGE, timestamp);
}

@Override
Expand Down Expand Up @@ -150,7 +151,7 @@ public void configure() {

@Override
public void replace(Double value, Long timestamp) {
update(value, UpdateOperation.REPLACE, timestamp);
update(nanToZero(value), UpdateOperation.REPLACE, timestamp);
}

/**
Expand Down Expand Up @@ -180,7 +181,7 @@ public void withTimeFrame(TimeFrame timeFrame) {
}

private int findTimebinIndex(long timestamp) {
return (int) FastMath.floor((timestamp - lowestBin) / binSize);
return (int) FastMath.floor(nanToZero((timestamp - lowestBin) / binSize));
}

private int rotateBuffer(int binIndex) {
Expand Down
Expand Up @@ -60,6 +60,7 @@ LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING
package org.dcache.util.collector.pools;

import static org.dcache.services.history.pools.PoolListingService.ALL;
import static org.dcache.util.MathUtils.nanToZero;

import diskCacheV111.poolManager.PoolSelectionUnit;
import diskCacheV111.poolManager.PoolSelectionUnit.SelectionPool;
Expand Down Expand Up @@ -352,8 +353,8 @@ public static CountingHistogram mergeLastAccess(List<PoolInfoWrapper> pools) {
double currentBinSize = h.getBinSize();
int numBins = currentData.size();
for (int bin = 0; bin < numBins; ++bin) {
int groupBin = (int) FastMath.floor(
(bin * currentBinSize) / binSize);
int groupBin = (int) FastMath.floor(nanToZero(
(bin * currentBinSize) / binSize));
dataArray[groupBin] += currentData.get(bin);
}
}
Expand Down

0 comments on commit 0f1d2d9

Please sign in to comment.