Skip to content

Commit 376628e

Browse files
vjkoskelavjkoskela
authored
Upgrade histogram bucket counts to 64-bit. (#181)
1 parent 2433d8a commit 376628e

File tree

7 files changed

+51
-50
lines changed

7 files changed

+51
-50
lines changed

‎pom.xml‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@
7979
<aspectjrt.version>1.9.2</aspectjrt.version>
8080
<asynchttpclient.version>2.6.0</asynchttpclient.version>
8181
<cglib.version>3.2.10</cglib.version>
82-
<client.protocol.version>0.11.1</client.protocol.version>
82+
<client.protocol.version>0.11.2</client.protocol.version>
8383
<fastutil.version>8.2.2</fastutil.version>
8484
<guava.version>25.1-jre</guava.version>
8585
<guice.version>4.2.2</guice.version>
@@ -91,7 +91,7 @@
9191
<logback.steno.version>1.18.2</logback.steno.version>
9292
<log4j.over.slf4j.version>1.7.25</log4j.over.slf4j.version>
9393
<metrics.aggregator.protocol.prometheus.version>1.0.0</metrics.aggregator.protocol.prometheus.version>
94-
<metrics.aggregator.protocol.version>1.0.6</metrics.aggregator.protocol.version>
94+
<metrics.aggregator.protocol.version>1.0.8</metrics.aggregator.protocol.version>
9595
<metrics.client.version>0.11.0</metrics.client.version>
9696
<metrics.client.http.version>0.11.0</metrics.client.http.version>
9797
<metrics.client.incubator.version>0.11.0</metrics.client.incubator.version>

‎src/main/java/com/arpnetworking/metrics/mad/model/statistics/HistogramStatistic.java‎

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@
2020
import com.arpnetworking.metrics.mad.model.Quantity;
2121
import com.arpnetworking.metrics.mad.model.Unit;
2222
import com.arpnetworking.tsdcore.model.CalculatedValue;
23-
import it.unimi.dsi.fastutil.doubles.Double2IntAVLTreeMap;
24-
import it.unimi.dsi.fastutil.doubles.Double2IntMap;
25-
import it.unimi.dsi.fastutil.doubles.Double2IntSortedMap;
23+
import it.unimi.dsi.fastutil.doubles.Double2LongAVLTreeMap;
24+
import it.unimi.dsi.fastutil.doubles.Double2LongMap;
25+
import it.unimi.dsi.fastutil.doubles.Double2LongSortedMap;
2626
import it.unimi.dsi.fastutil.objects.ObjectSortedSet;
2727
import net.sf.oval.constraint.NotNull;
2828

@@ -182,7 +182,7 @@ public HistogramSnapshot getHistogramSnapshot() {
182182
public HistogramSupportingData toUnit(final Unit newUnit) {
183183
if (_unit.isPresent()) {
184184
final Histogram newHistogram = new Histogram();
185-
for (final Map.Entry<Double, Integer> entry : _histogramSnapshot.getValues()) {
185+
for (final Map.Entry<Double, Long> entry : _histogramSnapshot.getValues()) {
186186
final Double newBucket = newUnit.convert(entry.getKey(), _unit.get());
187187
newHistogram.recordValue(newBucket, entry.getValue());
188188
}
@@ -259,8 +259,8 @@ public static final class Histogram {
259259
* @param value The value of the entry.
260260
* @param count The number of entries at this value.
261261
*/
262-
public void recordValue(final double value, final int count) {
263-
_data.merge(truncateToDouble(value), count, Integer::sum);
262+
public void recordValue(final double value, final long count) {
263+
_data.merge(truncateToDouble(value), count, Long::sum);
264264
_entriesCount += count;
265265
}
266266

@@ -280,7 +280,7 @@ public void recordValue(final double value) {
280280
* @param packed The packed bucket key.
281281
* @param count The number of entries at this value.
282282
*/
283-
public void recordPacked(final long packed, final int count) {
283+
public void recordPacked(final long packed, final long count) {
284284
recordValue(unpack(packed), count);
285285
}
286286

@@ -290,8 +290,8 @@ public void recordPacked(final long packed, final int count) {
290290
* @param histogramSnapshot The histogram snapshot to add to this one.
291291
*/
292292
public void add(final HistogramSnapshot histogramSnapshot) {
293-
for (final Double2IntMap.Entry entry : histogramSnapshot._data.double2IntEntrySet()) {
294-
_data.merge(entry.getDoubleKey(), entry.getIntValue(), Integer::sum);
293+
for (final Double2LongMap.Entry entry : histogramSnapshot._data.double2LongEntrySet()) {
294+
_data.merge(entry.getDoubleKey(), entry.getLongValue(), Long::sum);
295295
}
296296
_entriesCount += histogramSnapshot._entriesCount;
297297
}
@@ -341,8 +341,8 @@ public Histogram(final int precision) {
341341
_packMask = (1 << (_precision + EXPONENT_BITS + 1)) - 1;
342342
}
343343

344-
private int _entriesCount = 0;
345-
private final Double2IntSortedMap _data = new Double2IntAVLTreeMap();
344+
private long _entriesCount = 0;
345+
private final Double2LongSortedMap _data = new Double2LongAVLTreeMap();
346346
private final int _precision;
347347
private final long _truncateMask;
348348
private final int _packMask;
@@ -358,7 +358,7 @@ public Histogram(final int precision) {
358358
* @author Brandon Arp (brandon dot arp at inscopemetrics dot io)
359359
*/
360360
public static final class HistogramSnapshot {
361-
private HistogramSnapshot(final Double2IntSortedMap data, final int entriesCount, final int precision) {
361+
private HistogramSnapshot(final Double2LongSortedMap data, final long entriesCount, final int precision) {
362362
_precision = precision;
363363
_entriesCount = entriesCount;
364364
_data.putAll(data);
@@ -375,9 +375,10 @@ public Double getValueAtPercentile(final double percentile) {
375375
// The Math.min is for the case where the computation may be just
376376
// slightly larger than the _entriesCount and prevents an index out of range.
377377
final int target = (int) Math.min(Math.ceil(_entriesCount * percentile / 100.0D), _entriesCount);
378-
int accumulated = 0;
379-
for (final Double2IntMap.Entry next : _data.double2IntEntrySet()) {
380-
accumulated += next.getIntValue();
378+
// TODO(ville): Is it sufficient for this to be a long?
379+
long accumulated = 0;
380+
for (final Double2LongMap.Entry next : _data.double2LongEntrySet()) {
381+
accumulated += next.getLongValue();
381382
if (accumulated >= target) {
382383
return next.getDoubleKey();
383384
}
@@ -389,7 +390,7 @@ public int getPrecision() {
389390
return _precision;
390391
}
391392

392-
public int getEntriesCount() {
393+
public long getEntriesCount() {
393394
return _entriesCount;
394395
}
395396

@@ -398,8 +399,8 @@ public int getEntriesCount() {
398399
*
399400
* @return the histogram bucket count entries
400401
*/
401-
public ObjectSortedSet<Double2IntMap.Entry> getValues() {
402-
return _data.double2IntEntrySet();
402+
public ObjectSortedSet<Double2LongMap.Entry> getValues() {
403+
return _data.double2LongEntrySet();
403404
}
404405

405406
/**
@@ -408,12 +409,12 @@ public ObjectSortedSet<Double2IntMap.Entry> getValues() {
408409
* @param key the histogram bucket key
409410
* @return the count in the bucket
410411
*/
411-
public int getValue(final double key) {
412+
public long getValue(final double key) {
412413
return _data.get(key);
413414
}
414415

415-
private int _entriesCount;
416-
private final Double2IntSortedMap _data = new Double2IntAVLTreeMap();
416+
private long _entriesCount;
417+
private final Double2LongSortedMap _data = new Double2LongAVLTreeMap();
417418
private final int _precision;
418419
}
419420
}

‎src/main/java/com/arpnetworking/metrics/mad/parsers/ProtobufV3ToRecordParser.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ public List<Record> parse(final HttpRequest data) throws ParsingException {
8787
for (final ClientV3.MetricDataEntry metricDataEntry : record.getDataList()) {
8888
final String metricName = decodeRequiredIdentifier(metricDataEntry.getName());
8989
switch (metricDataEntry.getDataCase()) {
90-
case NUMERICALDATA:
90+
case NUMERICAL_DATA:
9191
final ClientV3.NumericalData numericalData = metricDataEntry.getNumericalData();
9292
if (!numericalData.getSamplesList().isEmpty()) {
9393
parseNumericalSamples(metricName, metricData, numericalData.getSamplesList());
@@ -153,7 +153,7 @@ private void parseNumericalStatistics(
153153

154154
final Map<Statistic, CalculatedValue<?>> statistics = Maps.newHashMap();
155155
final long populationSize = augmentedHistogram.getEntriesList().stream()
156-
.map(e -> (long) e.getCount())
156+
.map(ClientV3.SparseHistogramEntry::getCount)
157157
.reduce(Long::sum)
158158
.orElse(0L);
159159

@@ -240,7 +240,7 @@ private String decodeRequiredIdentifier(final ClientV3.Identifier identifier) {
240240
@Nullable
241241
private String decodeIdentifier(final ClientV3.Identifier identifier) {
242242
switch (identifier.getValueCase()) {
243-
case STRINGVALUE:
243+
case STRING_VALUE:
244244
return identifier.getStringValue();
245245
case VALUE_NOT_SET:
246246
return null;

‎src/main/java/com/arpnetworking/tsdcore/sinks/AggregationServerHttpSink.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ private ByteString serializeSupportingData(final AggregatedData datum) {
125125
}
126126
builder.setUnit(unit);
127127

128-
for (final Map.Entry<Double, Integer> entry : histogram.getValues()) {
128+
for (final Map.Entry<Double, Long> entry : histogram.getValues()) {
129129
builder.addEntriesBuilder()
130130
.setBucket(entry.getKey())
131131
.setCount(entry.getValue())

‎src/main/java/com/arpnetworking/tsdcore/sinks/AggregationServerSink.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ private ByteString serializeSupportingData(final AggregatedData datum) {
125125
}
126126
builder.setUnit(unit);
127127

128-
for (final Map.Entry<Double, Integer> entry : histogram.getValues()) {
128+
for (final Map.Entry<Double, Long> entry : histogram.getValues()) {
129129
builder.addEntriesBuilder()
130130
.setBucket(entry.getKey())
131131
.setCount(entry.getValue())

‎src/test/java/com/arpnetworking/metrics/mad/model/statistics/HistogramStatisticTest.java‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ public void histogramAccumulateQuantities() {
4040
final CalculatedValue<HistogramStatistic.HistogramSupportingData> value = accumulator.calculate(Collections.emptyMap());
4141
final HistogramStatistic.HistogramSupportingData supportingData = value.getData();
4242
final HistogramStatistic.HistogramSnapshot histogram = supportingData.getHistogramSnapshot();
43-
for (final Map.Entry<Double, Integer> entry : histogram.getValues()) {
44-
Assert.assertEquals(entry.getValue(), (Integer) 1);
43+
for (final Map.Entry<Double, Long> entry : histogram.getValues()) {
44+
Assert.assertEquals(entry.getValue(), (Long) 1L);
4545
}
4646
}
4747

@@ -59,8 +59,8 @@ public void histogramAccumulateHistogram() {
5959
final CalculatedValue<HistogramStatistic.HistogramSupportingData> value = merged.calculate(Collections.emptyMap());
6060
final HistogramStatistic.HistogramSupportingData supportingData = value.getData();
6161
final HistogramStatistic.HistogramSnapshot histogram = supportingData.getHistogramSnapshot();
62-
for (final Map.Entry<Double, Integer> entry : histogram.getValues()) {
63-
Assert.assertEquals(entry.getValue(), (Integer) 1);
62+
for (final Map.Entry<Double, Long> entry : histogram.getValues()) {
63+
Assert.assertEquals(entry.getValue(), (Long) 1L);
6464
}
6565
}
6666

@@ -90,14 +90,14 @@ public void histogramAccumulateMultipleHistogram() {
9090
final CalculatedValue<HistogramStatistic.HistogramSupportingData> value = merged.calculate(Collections.emptyMap());
9191
final HistogramStatistic.HistogramSupportingData supportingData = value.getData();
9292
final HistogramStatistic.HistogramSnapshot histogram = supportingData.getHistogramSnapshot();
93-
for (final Map.Entry<Double, Integer> entry : histogram.getValues()) {
93+
for (final Map.Entry<Double, Long> entry : histogram.getValues()) {
9494
final int val = entry.getKey().intValue();
9595
if (val < 50) {
96-
Assert.assertEquals("incorrect value for key " + val, (Integer) 1, entry.getValue());
96+
Assert.assertEquals("incorrect value for key " + val, (Long) 1L, entry.getValue());
9797
} else if (val <= 100) {
98-
Assert.assertEquals("incorrect value for key " + val, (Integer) 2, entry.getValue());
98+
Assert.assertEquals("incorrect value for key " + val, (Long) 2L, entry.getValue());
9999
} else { // val > 100
100-
Assert.assertEquals("incorrect value for key " + val, (Integer) 1, entry.getValue());
100+
Assert.assertEquals("incorrect value for key " + val, (Long) 1L, entry.getValue());
101101
}
102102
}
103103

@@ -113,7 +113,7 @@ public void histogramQuantityInvalidConversion() {
113113
final CalculatedValue<HistogramStatistic.HistogramSupportingData> value = accumulator.calculate(Collections.emptyMap());
114114
final HistogramStatistic.HistogramSupportingData supportingData = value.getData();
115115
final HistogramStatistic.HistogramSnapshot histogram = supportingData.getHistogramSnapshot();
116-
Assert.assertEquals(2, histogram.getEntriesCount());
116+
Assert.assertEquals(2L, histogram.getEntriesCount());
117117
Assert.assertEquals(1, histogram.getValues().size());
118118
}
119119

‎src/test/java/com/arpnetworking/metrics/mad/parsers/ProtobufV3ToRecordParserTest.java‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -127,13 +127,13 @@ public void testParseSingleRecord() throws ParsingException, IOException {
127127
supportingData = (HistogramStatistic.HistogramSupportingData) calculatedValue.getData();
128128
histogramSnapshot = supportingData.getHistogramSnapshot();
129129
Assert.assertFalse(supportingData.getUnit().isPresent());
130-
Assert.assertEquals(9, histogramSnapshot.getEntriesCount());
130+
Assert.assertEquals(9L, histogramSnapshot.getEntriesCount());
131131
Assert.assertEquals(7, histogramSnapshot.getPrecision());
132-
Assert.assertEquals(1, histogramSnapshot.getValue(1.0));
133-
Assert.assertEquals(2, histogramSnapshot.getValue(2.0));
134-
Assert.assertEquals(3, histogramSnapshot.getValue(3.0));
135-
Assert.assertEquals(2, histogramSnapshot.getValue(4.0));
136-
Assert.assertEquals(1, histogramSnapshot.getValue(5.0));
132+
Assert.assertEquals(1L, histogramSnapshot.getValue(1.0));
133+
Assert.assertEquals(2L, histogramSnapshot.getValue(2.0));
134+
Assert.assertEquals(3L, histogramSnapshot.getValue(3.0));
135+
Assert.assertEquals(2L, histogramSnapshot.getValue(4.0));
136+
Assert.assertEquals(1L, histogramSnapshot.getValue(5.0));
137137

138138
final Metric combined = metrics.get("combined1");
139139
Assert.assertNotNull(combined);
@@ -170,12 +170,12 @@ public void testParseSingleRecord() throws ParsingException, IOException {
170170
supportingData = (HistogramStatistic.HistogramSupportingData) calculatedValue.getData();
171171
histogramSnapshot = supportingData.getHistogramSnapshot();
172172
Assert.assertFalse(supportingData.getUnit().isPresent());
173-
Assert.assertEquals(4, histogramSnapshot.getEntriesCount());
173+
Assert.assertEquals(4L, histogramSnapshot.getEntriesCount());
174174
Assert.assertEquals(7, histogramSnapshot.getPrecision());
175-
Assert.assertEquals(1, histogramSnapshot.getValue(2.0));
176-
Assert.assertEquals(1, histogramSnapshot.getValue(3.0));
177-
Assert.assertEquals(1, histogramSnapshot.getValue(4.0));
178-
Assert.assertEquals(1, histogramSnapshot.getValue(5.0));
175+
Assert.assertEquals(1L, histogramSnapshot.getValue(2.0));
176+
Assert.assertEquals(1L, histogramSnapshot.getValue(3.0));
177+
Assert.assertEquals(1L, histogramSnapshot.getValue(4.0));
178+
Assert.assertEquals(1L, histogramSnapshot.getValue(5.0));
179179
// Min
180180
calculatedValue = combined.getStatistics().get(STATISTIC_FACTORY.getStatistic("min")).get(1);
181181
Assert.assertEquals(new DefaultQuantity.Builder().setValue(3.0).build(), calculatedValue.getValue());
@@ -199,9 +199,9 @@ public void testParseSingleRecord() throws ParsingException, IOException {
199199
supportingData = (HistogramStatistic.HistogramSupportingData) calculatedValue.getData();
200200
histogramSnapshot = supportingData.getHistogramSnapshot();
201201
Assert.assertFalse(supportingData.getUnit().isPresent());
202-
Assert.assertEquals(1, histogramSnapshot.getEntriesCount());
202+
Assert.assertEquals(1L, histogramSnapshot.getEntriesCount());
203203
Assert.assertEquals(7, histogramSnapshot.getPrecision());
204-
Assert.assertEquals(1, histogramSnapshot.getValue(3.0));
204+
Assert.assertEquals(1L, histogramSnapshot.getValue(3.0));
205205
}
206206
// CHECKSTYLE.ON: MethodLength
207207

0 commit comments

Comments
 (0)