vincentchenjl commented on a change in pull request #5869:
URL: https://github.com/apache/incubator-pinot/pull/5869#discussion_r471774644
##########
File path:
thirdeye/thirdeye-pinot/src/test/java/org/apache/pinot/thirdeye/anomaly/views/TestCondensedAnomalyTimelinesView.java
##########
@@ -73,35 +74,46 @@ public void testFromJsonString() throws Exception{
}
}
+ /** Compression Test case 1: anomaly view could satisfy requirement after
rounding up the decimals.*/
+ @Test
+ public void testCompressWithRoundUp() throws Exception {
+ int testNum = 500;
+ CondensedAnomalyTimelinesView condensedView =
CondensedAnomalyTimelinesView.fromAnomalyTimelinesView(getTestData(testNum));
+ Assert.assertTrue(condensedView.toJsonString().length() >
CondensedAnomalyTimelinesView.DEFAULT_MAX_LENGTH);
+ CondensedAnomalyTimelinesView compressedView = condensedView.compress();
+ Assert.assertTrue(compressedView.toJsonString().length() <
CondensedAnomalyTimelinesView.DEFAULT_MAX_LENGTH);
+ Assert.assertEquals(testNum, compressedView.timeStamps.size());
+ }
+
+ /** Compression Test case 2: The anomaly view is still too large after
rounding up, and needed to be further compressed */
@Test
public void testCompress() throws Exception {
- int testNum = 1500;
+ int testNum = 600;
long minBucketMillis =
CondensedAnomalyTimelinesView.DEFAULT_MIN_BUCKET_UNIT;
CondensedAnomalyTimelinesView condensedView =
CondensedAnomalyTimelinesView.fromAnomalyTimelinesView(getTestData(testNum));
Assert.assertTrue(condensedView.toJsonString().length() >
CondensedAnomalyTimelinesView.DEFAULT_MAX_LENGTH);
-
CondensedAnomalyTimelinesView compressedView = condensedView.compress();
Assert.assertTrue(compressedView.toJsonString().length() <
CondensedAnomalyTimelinesView.DEFAULT_MAX_LENGTH);
- Assert.assertEquals(compressedView.bucketMillis.longValue(), 240l);
+ Assert.assertEquals(300, compressedView.timeStamps.size());
+ Assert.assertEquals(compressedView.bucketMillis.longValue(), 10);
DateTime date = new DateTime(2018, 1, 1, 0, 0, 0);
for (int i = 0; i < compressedView.getTimeStamps().size(); i++) {
Assert.assertEquals(compressedView.getTimeStamps().get(i).longValue(),
(date.getMillis() - condensedView.timestampOffset)/minBucketMillis);
- Assert.assertEquals(compressedView.getCurrentValues().get(i), i * 4 +
1.5, 0.000001);
- Assert.assertEquals(compressedView.getBaselineValues().get(i), i * 4 +
1.6, 0.000001);
- date = date.plusHours(4);
+ Assert.assertEquals(compressedView.getCurrentValues().get(i), i * 2 +
0.5, 0.000001);
+ Assert.assertEquals(compressedView.getBaselineValues().get(i), i * 2 +
0.833, 0.000001);
Review comment:
Could we add another check to make sure that the JSON exported also has
3 decimal points? For example
`Assert.assertEquals(OBJECT_MAPPER.writeValueAsString(compressedView.getBaselineValues().get(i)),
"2.833")`
##########
File path:
thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/anomaly/views/CondensedAnomalyTimelinesView.java
##########
@@ -182,9 +183,33 @@ public static CondensedAnomalyTimelinesView
fromAnomalyTimelinesView(AnomalyTime
* @return a compressed CondensedAnomalyTimelinesView
*/
public CondensedAnomalyTimelinesView compress() {
+ if (timeStamps.size() == 0) {
+ return this;
+ }
+ try {
+ if (this.toJsonString().length() > DEFAULT_MAX_LENGTH) {
+ // First try rounding up
+ roundUp();
Review comment:
I think that the roundup logic is better added into
`compress(DEFAULT_MAX_LENGTH)`, because people can call either these two
methods to compress and `roundup` should run in both cases.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
