siddharthteotia commented on a change in pull request #6625: URL: https://github.com/apache/incubator-pinot/pull/6625#discussion_r591786053
########## File path: pinot-controller/src/test/java/org/apache/pinot/controller/recommender/realtime/provisioning/MemoryEstimatorTest.java ########## @@ -41,20 +43,20 @@ public void testSegmentGenerator() throws Exception { runTest("memory_estimation/schema-with-metadata.json", metadata -> { assertEquals(extract(metadata, "segment.total.docs = (\\d+)"), "100000"); - assertEquals(extract(metadata, "column.colInt.cardinality = (\\d+)"), "100"); - assertEquals(extract(metadata, "column.colIntMV.cardinality = (\\d+)"), "150"); - assertEquals(extract(metadata, "column.colFloat.cardinality = (\\d+)"), "200"); - assertEquals(extract(metadata, "column.colFloatMV.cardinality = (\\d+)"), "250"); - assertEquals(extract(metadata, "column.colString.cardinality = (\\d+)"), "300"); - assertEquals(extract(metadata, "column.colStringMV.cardinality = (\\d+)"), "350"); - assertEquals(extract(metadata, "column.colBytes.cardinality = (\\d+)"), "400"); - assertEquals(extract(metadata, "column.colLong.cardinality = (\\d+)"), "500"); - assertEquals(extract(metadata, "column.colLongMV.cardinality = (\\d+)"), "550"); - assertEquals(extract(metadata, "column.colDouble.cardinality = (\\d+)"), "600"); - assertEquals(extract(metadata, "column.colDoubleMV.cardinality = (\\d+)"), "650"); - assertEquals(extract(metadata, "column.colDoubleMetric.cardinality = (\\d+)"), "700"); - assertEquals(extract(metadata, "column.colFloatMetric.cardinality = (\\d+)"), "800"); - assertEquals(extract(metadata, "column.colTime.cardinality = (\\d+)"), "900"); + assertEquals(extract(metadata, "column.colInt.cardinality = (\\d+)"), "10"); Review comment: As discussed offline, this is a good way to make test deterministic. Smaller cardinality might ensure that random generator will generate as many unique values as we want but it is still not deterministic. Secondly, smaller cardinality is not very meaningful for the test. So let's fix the generation code to ensure it generates as many unique values as we want in a deterministic style. ########## File path: pinot-controller/src/test/java/org/apache/pinot/controller/recommender/realtime/provisioning/MemoryEstimatorTest.java ########## @@ -41,20 +43,20 @@ public void testSegmentGenerator() throws Exception { runTest("memory_estimation/schema-with-metadata.json", metadata -> { assertEquals(extract(metadata, "segment.total.docs = (\\d+)"), "100000"); - assertEquals(extract(metadata, "column.colInt.cardinality = (\\d+)"), "100"); - assertEquals(extract(metadata, "column.colIntMV.cardinality = (\\d+)"), "150"); - assertEquals(extract(metadata, "column.colFloat.cardinality = (\\d+)"), "200"); - assertEquals(extract(metadata, "column.colFloatMV.cardinality = (\\d+)"), "250"); - assertEquals(extract(metadata, "column.colString.cardinality = (\\d+)"), "300"); - assertEquals(extract(metadata, "column.colStringMV.cardinality = (\\d+)"), "350"); - assertEquals(extract(metadata, "column.colBytes.cardinality = (\\d+)"), "400"); - assertEquals(extract(metadata, "column.colLong.cardinality = (\\d+)"), "500"); - assertEquals(extract(metadata, "column.colLongMV.cardinality = (\\d+)"), "550"); - assertEquals(extract(metadata, "column.colDouble.cardinality = (\\d+)"), "600"); - assertEquals(extract(metadata, "column.colDoubleMV.cardinality = (\\d+)"), "650"); - assertEquals(extract(metadata, "column.colDoubleMetric.cardinality = (\\d+)"), "700"); - assertEquals(extract(metadata, "column.colFloatMetric.cardinality = (\\d+)"), "800"); - assertEquals(extract(metadata, "column.colTime.cardinality = (\\d+)"), "900"); + assertEquals(extract(metadata, "column.colInt.cardinality = (\\d+)"), "10"); Review comment: As discussed offline, this is not a good way to make test deterministic. Smaller cardinality might ensure that random generator will generate as many unique values as we want but it is still not deterministic. Secondly, smaller cardinality is not very meaningful for the test. So let's fix the generation code to ensure it generates as many unique values as we want in a deterministic style. ---------------------------------------------------------------- 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