Skip to content

Commit

Permalink
CKMS Quantiles: Fix test and disallow corner cases
Browse files Browse the repository at this point in the history 
Signed-off-by: Fabian Stäber <[email protected]>
  • Loading branch information
@fstab
fstab committed Jan 30, 2022
1 parent 787eef3 commit 3d6f699
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 51 deletions.
12 changes: 4 additions & 8 deletions simpleclient/src/main/java/io/prometheus/client/CKMSQuantiles.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,14 +129,10 @@ final class CKMSQuantiles {
* @param quantiles The targeted quantiles, can be empty.
*/
CKMSQuantiles(Quantile[] quantiles) {
// hard-coded epsilon of 0.1% to determine the batch size, and default epsilon in case of empty quantiles
double pointOnePercent = 0.001;
if (quantiles.length == 0) { // we need at least one for this algorithm to work
this.quantiles = new Quantile[1];
this.quantiles[0] = new Quantile(0.5, pointOnePercent / 2);
} else {
this.quantiles = quantiles;
throw new IllegalArgumentException("quantiles cannot be empty");
}
this.quantiles = quantiles;

// section 5.1 Methods - Batch.
// This is hardcoded to 500, which corresponds to an epsilon of 0.1%.
Expand Down Expand Up @@ -431,8 +427,8 @@ static class Quantile {
* @param epsilon the desired error for this quantile, between 0 and 1.
*/
Quantile(double quantile, double epsilon) {
if (quantile < 0 || quantile > 1.0) throw new IllegalArgumentException("Quantile must be between 0 and 1");
if (epsilon < 0 || epsilon > 1.0) throw new IllegalArgumentException("Epsilon must be between 0 and 1");
if (quantile <= 0 || quantile >= 1.0) throw new IllegalArgumentException("Quantile must be between 0 and 1");
if (epsilon <= 0 || epsilon >= 1.0) throw new IllegalArgumentException("Epsilon must be between 0 and 1");

this.quantile = quantile;
this.epsilon = epsilon;
Expand Down
4 changes: 2 additions & 2 deletions simpleclient/src/main/java/io/prometheus/client/Summary.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,10 @@ public static class Builder extends SimpleCollector.Builder<Builder, Summary> {
private int ageBuckets = 5;

public Builder quantile(double quantile, double error) {
if (quantile < 0.0 || quantile > 1.0) {
if (quantile <= 0.0 || quantile >= 1.0) {
throw new IllegalArgumentException("Quantile " + quantile + " invalid: Expected number between 0.0 and 1.0.");
}
if (error < 0.0 || error > 1.0) {
if (error <= 0.0 || error >= 1.0) {
throw new IllegalArgumentException("Error " + error + " invalid: Expected number between 0.0 and 1.0.");
}
quantiles.add(new Quantile(quantile, error));
Expand Down
57 changes: 16 additions & 41 deletions simpleclient/src/test/java/io/prometheus/client/CKMSQuantilesTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,37 +25,6 @@ public void testGetOnEmptyValues() {
assertEquals(Double.NaN, ckms.get(0), 0);
}

@Test
public void testGetWhenNoQuantilesAreDefined() {
CKMSQuantiles ckms = new CKMSQuantiles(new Quantile[]{});
assertEquals(Double.NaN, ckms.get(0), 0);
}

@Test
public void testInsertWhenNoQuantilesAreDefined() {
CKMSQuantiles ckms = new CKMSQuantiles(new Quantile[]{});
ckms.insert(1.0);
ckms.insert(2.0);
ckms.insert(3.0);
assertEquals(1.0, ckms.get(0), 0);
assertEquals(2.0, ckms.get(0.5), 0);
assertEquals(3.0, ckms.get(1), 0);
}

@Test
public void testCompressWhenBufferSize500Reached() {
CKMSQuantiles ckms = new CKMSQuantiles(new Quantile[]{});
List<Double> input = makeSequence(1, 499);

for (double v : input) {
ckms.insert(v);
}
assertEquals("No compress should be triggered", 0, ckms.samples.size());

ckms.insert(500);
assertEquals(500, ckms.samples.size());
}

@Test
public void testGet() {
List<Quantile> quantiles = new ArrayList<Quantile>();
Expand All @@ -80,20 +49,19 @@ public void testGet() {
@Test
public void testGetWithAMillionElements() {
List<Quantile> quantiles = new ArrayList<Quantile>();
quantiles.add(new Quantile(0.0, 0.01));
quantiles.add(new Quantile(0.01, 0.001));
quantiles.add(new Quantile(0.10, 0.01));
quantiles.add(new Quantile(0.90, 0.001));
quantiles.add(new Quantile(0.95, 0.02));
quantiles.add(new Quantile(0.99, 0.001));

final int elemCount = 1000000;
double[] shuffle = new double[elemCount];
for (int i = 0; i < shuffle.length; i++) {
shuffle[i] = i + 1;
List<Double> shuffle = new ArrayList<Double>(elemCount);
for (int i = 0; i < elemCount; i++) {
shuffle.add(i+1.0);
}
Random rand = new Random(0);

Collections.shuffle(Arrays.asList(shuffle), rand);
Collections.shuffle(shuffle, rand);

CKMSQuantiles ckms = new CKMSQuantiles(
quantiles.toArray(new Quantile[]{}));
Expand All @@ -102,14 +70,21 @@ public void testGetWithAMillionElements() {
ckms.insert(v);
}
// given the linear distribution, we set the delta equal to the εn value for this quantile
assertEquals(0.1 * elemCount, ckms.get(0.1), 0.01 * elemCount);
assertEquals(0.9 * elemCount, ckms.get(0.9), 0.001 * elemCount);
assertEquals(0.95 * elemCount, ckms.get(0.95), 0.02 * elemCount);
assertEquals(0.99 * elemCount, ckms.get(0.99), 0.001 * elemCount);
assertRank(elemCount, ckms.get(0.01), 0.01, 0.001);
assertRank(elemCount, ckms.get(0.1), 0.1, 0.01);
assertRank(elemCount, ckms.get(0.9), 0.9, 0.001);
assertRank(elemCount, ckms.get(0.95), 0.95, 0.02);
assertRank(elemCount, ckms.get(0.99), 0.99, 0.001);

assertTrue("sample size should be way below 1_000_000", ckms.samples.size() < 1000);
}

private void assertRank(int elemCount, double actual, double quantile, double epsilon) {
double lowerBound = elemCount * (quantile - epsilon);
double upperBound = elemCount * (quantile + epsilon);
assertTrue("quantile=" + quantile + ", actual=" + actual + ", lowerBound=" + lowerBound, actual >= lowerBound);
assertTrue("quantile=" + quantile + ", actual=" + actual + ", upperBound=" + upperBound, actual <= upperBound);
}

@Test
public void testGetGaussian() {
Expand Down

0 comments on commit 3d6f699

Please sign in to comment.