Skip to content

Commit

Permalink
common: fix bug in CountingHistogram index computation
Browse files Browse the repository at this point in the history
Motivation:

After deploying 4.2 to production, Fermilab public dCache began
to experience NullPointerExceptions in the RESTful pool services
resulting from missing sweeper data.  The problem of how to
avoid the NPEs is addressing in a second patch, but the root
problem seems to be from configuring of the histogram that
is embedded.

This was discovered by logging the exception that is sent back
with the message, which was like this:

Problem with retrieval of live pool data for rw-stkendca38a-2: java.lang.ArrayIndexOutOfBoundsException

Further debugging could not be added to the pools without a restart,
not desirable on production, but a look at the code revealed
what seems to be a single vulnerability at line 129 of
the configure() method in CountingHistogram.

Indeed, in the testing below, a sampling of how that algorithm
currently works reveals it can produce erroneous results for
values which are within 0.5 the bin width of the bin
count.

The reason is because the width is computed using floor,
but the binning is done using round.

Modification:

While round/floor, round/round and floor/floor all work,
floor/floor has the advantage of not contracting the plot
by doubling the bin size for a larger range of the values
that would result in the erroneous index.

Result:

Hopefully, no more ArrayIndexOOB exceptions.

Target: master
Request: 4.2
Request: 4.1
Acked-by: Paul
  • Loading branch information
alrossi committed Nov 8, 2018
1 parent ccef077 commit 30ae5dd
Showing 1 changed file with 1 addition and 1 deletion.
Expand Up @@ -126,7 +126,7 @@ public void configure() {
long[] histogram = new long[binCount];

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

data = new ArrayList<>();
Expand Down

0 comments on commit 30ae5dd

Please sign in to comment.